From ff15b405e2a89e4fb58113d88e625d36358859ff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 18:59:28 +0000 Subject: [PATCH 1/9] test(verify): one coverage case per committed S-1 fixture The single body parsed four fixtures (the largest 7 MB, ~5 s together on an idle machine) and crossed the 30 s test timeout under full-suite load. Split with it.each, no case comes near the limit, and a timeout or a loss names the fixture it came from. Fixes #391 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- src/verify/coverage.test.ts | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/verify/coverage.test.ts b/src/verify/coverage.test.ts index 147b924d..326b6791 100644 --- a/src/verify/coverage.test.ts +++ b/src/verify/coverage.test.ts @@ -90,21 +90,23 @@ describe("buildParseTrace", () => { * furniture rules moves it without anything being lost. `lostChars` is the * defect signal on its own, and it is 0 across all 44 committed fixtures, so * the assertion can be the property rather than a number to maintain. + * + * One case per fixture rather than one body over all of them: the largest + * is 7 MB and the four together took ~5 s idle, so under full-suite load + * the single body crossed the 30 s test timeout while no one fixture comes + * near it. Split, a timeout or a loss also names the fixture it came from. */ - it("loses no content on committed fixtures", () => { - const fixtures = [ - "s1_1563568_000143774926013504.htm", - "s1_1849470_000110465921035696.htm", - "s1_1925283_000162828026027260.htm", - "s1_2049662_000110465926079324.htm", - ]; - const losses = fixtures.flatMap((name) => { - const { coverage } = buildParseTrace(readFileSync(join(fixtureRoot, name), "utf8"), name); - return coverage.lostChars === 0 - ? [] - : [`${name}: ${coverage.lostChars} chars in ${coverage.lostRuns} runs`]; + it.each([ + "s1_1563568_000143774926013504.htm", + "s1_1849470_000110465921035696.htm", + "s1_1925283_000162828026027260.htm", + "s1_2049662_000110465926079324.htm", + ])("loses no content on committed fixture %s", (name) => { + const { coverage } = buildParseTrace(readFileSync(join(fixtureRoot, name), "utf8"), name); + expect({ lostChars: coverage.lostChars, lostRuns: coverage.lostRuns }).toEqual({ + lostChars: 0, + lostRuns: 0, }); - expect(losses).toEqual([]); }); /** From 3bb86ef2c61a501b8379490690cf788a6c869b96 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:02:12 +0000 Subject: [PATCH 2/9] ci: run the release gate list on every push, on a pinned Bun CI ran five of the six release gates and never inspected the tarball or started the built binaries; a widened `files` or a binary that dies on --help was found only at release time. `scripts/releaseChecks.ts` is now the one gate list both `release-checks` and CI run: format-check, lint, typecheck, build, prepack-check, smoke-bins, test. - smoke-bins starts each built binary with no SEC env, no .env.local and an empty cwd, and requires --help and --version to exit 0. It covers dist/sec.js; dist/libs-cli.js still dies there and joins the list when its boot is made lazy. - prepack-check refuses any src/ path, not only mock_data, so a source leak too small for the size ceiling still fails. - Bun is pinned once, packageManager bun@1.4.3, and setup-bun reads it from package.json instead of installing latest: typecheck is bun check, so a Bun release could turn an unchanged commit red. engines.bun and @types/bun are checked against it by toolchainPins.test.ts. - bun-latest.yml runs the same gates weekly on the newest Bun, gating nothing, so drift is seen before the pin moves. Fixes #389 Fixes #390 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- .claude/CLAUDE.md | 16 ++++++- .github/workflows/bun-latest.yml | 31 +++++++++++++ .github/workflows/test.yml | 18 ++++---- README.md | 11 +++-- package.json | 4 +- scripts/checkPackedContents.ts | 7 ++- scripts/releaseChecks.ts | 72 +++++++++++++++++++++++++++++ scripts/smokeBinaries.ts | 59 ++++++++++++++++++++++++ src/config/toolchainPins.test.ts | 78 ++++++++++++++++++++++++++++++++ 9 files changed, 279 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/bun-latest.yml create mode 100644 scripts/releaseChecks.ts create mode 100644 scripts/smokeBinaries.ts create mode 100644 src/config/toolchainPins.test.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 75dfd73a..7b3ae859 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -42,16 +42,28 @@ bun run lint # oxlint + tsgolint type-aware rules; CI runs this bun run format # oxfmt write — run before pushing ``` -CI runs `format-check` → `lint` → `typecheck` → `build` → `test`, cheapest first. +CI and `bun run release-checks` run one gate list, `GATES` in +`scripts/releaseChecks.ts`: `format-check` → `lint` → `typecheck` → `build` → +`prepack-check` → `smoke-bins` → `test`, cheapest first. Add a gate there, never +as a workflow step of its own — `toolchainPins.test.ts` fails on a restated one. +`smoke-bins` starts each built binary unconfigured and requires `--help` and +`--version` to exit 0. **Tests are vitest with `pool: "forks"` and `isolate: true`.** Run them with `bunx vitest run`, never `bun test`: the Bun shim does not give each file a fresh module registry, so `globalServiceRegistry` leaks between files. -**Node 24, Bun 1.4+.** `node:sqlite` backs the SQLite storage and is neither stable +**Node 24, Bun 1.4.3.** `node:sqlite` backs the SQLite storage and is neither stable nor unflagged below those. A subprocess CLI test on an older Bun fails with what reads like a storage bug. +**Bun is pinned once, in `packageManager` (`bun@1.4.3`)**, and CI installs exactly +that (`setup-bun` reads it from `package.json`). `typecheck` is `bun check`, so a +Bun release could otherwise turn an unchanged commit red. `engines.bun` is the +same version as a floor and `@types/bun` is the same version exactly; +`toolchainPins.test.ts` fails when they disagree, so bump all three in one +commit. `bun-latest.yml` runs the gates on the newest Bun weekly, gating nothing. + There is one tsconfig covering src, scripts and tests. It emits nothing — the binaries bundle — so there is no declaration build to keep tests out of. diff --git a/.github/workflows/bun-latest.yml b/.github/workflows/bun-latest.yml new file mode 100644 index 00000000..e358f6f4 --- /dev/null +++ b/.github/workflows/bun-latest.yml @@ -0,0 +1,31 @@ +name: Bun latest + +# The pinned Bun is what gates trunk (`test.yml`). This runs the same gates on +# the newest Bun once a week so a release that changes `bun check` or the test +# runner shows up here, as a red run nobody is blocked by, before the pin is +# bumped onto it. + +on: + schedule: + - cron: "17 6 * * 1" + workflow_dispatch: + +env: + CI: "true" + DO_NOT_TRACK: "1" + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v6 + with: + node-version: "24" + - uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + - run: bun --version + - run: bun i + - run: bun run release-checks diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e1252812..e828c387 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,15 +25,15 @@ jobs: - uses: actions/setup-node@v6 with: node-version: "24" + # The Bun pinned by `packageManager`, not `latest`: `typecheck` is + # `bun check`, so a Bun release can change a run's result on a commit + # that did not change. `bun-latest.yml` watches for that drift. - uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version-file: package.json - run: bun i - - run: bun run format-check - # oxlint + tsgolint. Needs no build of its own: the type-aware rules - # resolve `@workglow/*` through the published `dist/*.d.ts` that `bun i` - # already put in node_modules. - - run: bun run lint - - run: bun run typecheck - - run: bun run build - - run: bun run test + # The gate list `release` runs, from the one script both call + # (`scripts/releaseChecks.ts`); each gate is its own log group. lint needs + # no build of its own: the type-aware rules resolve `@workglow/*` through + # the published `dist/*.d.ts` that `bun i` already put in node_modules. + - run: bun run release-checks diff --git a/README.md b/README.md index e8a13358..a8939bd1 100644 --- a/README.md +++ b/README.md @@ -123,9 +123,12 @@ bun run lint # oxlint + type-aware rules bun run format # oxfmt ``` -Node 24 and Bun 1.4+, declared as `engines` so a lower Node fails at install +Node 24 and Bun 1.4.3, declared as `engines` so a lower Node fails at install rather than later inside `node:sqlite`. `node:sqlite` is what backs the SQLite -storage, and it is neither stable nor unflagged below those. +storage, and it is neither stable nor unflagged below those. CI runs exactly +Bun 1.4.3, read from `packageManager` in `package.json` — the one place the +version is written, since `typecheck` is `bun check` and a new Bun can change +its answer on an unchanged commit. ### The package is two binaries, not a library @@ -139,7 +142,9 @@ the decision. ### Cutting a release `bun run release-checks` is the gate set (format, lint, typecheck, build, packed -contents). One script runs it and then bumps: +contents, a start of each built binary with no configuration, tests) — the same +list CI runs on every push, from `scripts/releaseChecks.ts`. One script runs it +and then bumps: ```sh bun run release diff --git a/package.json b/package.json index a1eb4660..21811464 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,10 @@ "homepage": "https://workglow.dev", "description": "A worked example of the Workglow libraries: SEC EDGAR and Form ADV ingestion, filing-to-markdown conversion, and retrieval-grounded question answering over what filings say.", "scripts": { - "release-checks": "bun run format-check && bun run lint && bun run typecheck && bun run build && bun run test && bun run prepack-check", + "release-checks": "bun ./scripts/releaseChecks.ts", "release": "bun run release-checks && bunset --auto --push --commit --tag", "prepack-check": "bun ./scripts/checkPackedContents.ts", + "smoke-bins": "bun ./scripts/smokeBinaries.ts", "dev": "concurrently -c 'auto' -n 'sec:' 'bun:dev-*'", "dev-sec": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/sec.js ./src/sec.ts", "dev-sec-base": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/libs-cli.js ./src/libs-cli.ts", @@ -46,6 +47,7 @@ "dist", "!**/*.map" ], + "packageManager": "bun@1.4.3", "engines": { "node": ">=24", "bun": ">=1.4.3" diff --git a/scripts/checkPackedContents.ts b/scripts/checkPackedContents.ts index 4d90975f..f5cee883 100644 --- a/scripts/checkPackedContents.ts +++ b/scripts/checkPackedContents.ts @@ -35,7 +35,10 @@ import { findSourceStubs } from "./sourceStubs"; // not when src sneaks back in. const MAX_UNPACKED_BYTES = 8_000_000; // 8 MB const FORBIDDEN_SUFFIXES = [".test.ts", ".map"] as const; -const FORBIDDEN_PREFIXES = ["src/sec/html/mock_data/"] as const; +// Any `src/` path is a leak: `files` ships `dist` alone, so a source file in +// the listing means `files` or `.npmignore` widened. The size ceiling only +// catches a leak big enough to cross it. +const FORBIDDEN_PREFIXES = ["src/"] as const; const FORBIDDEN_SUBSTRINGS = ["/mock_data/"] as const; /** @@ -214,7 +217,7 @@ async function main(): Promise { const extra = forbidden.length > 10 ? `\n ... and ${forbidden.length - 10} more` : ""; violations.push( `${forbidden.length} forbidden file(s) would be published ` + - `(tests / mock_data are not part of the shipped package):\n ${sample}${extra}` + `(source, tests and mock_data are not part of the shipped package):\n ${sample}${extra}` ); } diff --git a/scripts/releaseChecks.ts b/scripts/releaseChecks.ts new file mode 100644 index 00000000..f47ebb4d --- /dev/null +++ b/scripts/releaseChecks.ts @@ -0,0 +1,72 @@ +#!/usr/bin/env bun +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * The gate list, once. `bun run release-checks` and CI both run this script, + * so what a release is checked against and what every push is checked against + * cannot drift apart — they did: CI ran five of the six, and the one it left + * out (what the tarball holds) was then found only on the maintainer's machine + * at release time. + * + * Cheapest first, stopping at the first failure. Under GitHub Actions each gate + * is a collapsible log group and a failure is an annotation naming the gate, + * which is what separate workflow steps used to give. + * + * `bun ./scripts/releaseChecks.ts --list` prints the gate names. + */ +import { spawnSync } from "node:child_process"; + +interface Gate { + readonly name: string; + readonly command: readonly string[]; +} + +export const GATES: readonly Gate[] = [ + { name: "format-check", command: ["bun", "run", "format-check"] }, + { name: "lint", command: ["bun", "run", "lint"] }, + { name: "typecheck", command: ["bun", "run", "typecheck"] }, + { name: "build", command: ["bun", "run", "build"] }, + // Both read `dist`, so they follow `build`; both take seconds, so they go + // ahead of the suite rather than after it. + { name: "prepack-check", command: ["bun", "run", "prepack-check"] }, + { name: "smoke-bins", command: ["bun", "run", "smoke-bins"] }, + { name: "test", command: ["bun", "run", "test"] }, +]; + +const inActions = process.env.GITHUB_ACTIONS === "true"; + +function runGate(gate: Gate): boolean { + const started = performance.now(); + if (inActions) console.log(`::group::${gate.name}`); + else console.log(`\n== ${gate.name} ==`); + const [command, ...args] = gate.command; + const result = spawnSync(command, args, { stdio: "inherit" }); + if (inActions) console.log("::endgroup::"); + const seconds = ((performance.now() - started) / 1000).toFixed(1); + const ok = result.status === 0; + if (ok) { + console.log(`ok ${gate.name} (${seconds}s)`); + } else { + const why = result.error?.message ?? `exit ${result.status ?? result.signal}`; + console.log( + inActions + ? `::error title=${gate.name}::${gate.name} failed (${why})` + : `FAILED ${gate.name} (${why})` + ); + } + return ok; +} + +if (import.meta.main) { + if (process.argv.includes("--list")) { + for (const gate of GATES) console.log(gate.name); + } else { + for (const gate of GATES) { + if (!runGate(gate)) process.exit(1); + } + } +} diff --git a/scripts/smokeBinaries.ts b/scripts/smokeBinaries.ts new file mode 100644 index 00000000..28f557a6 --- /dev/null +++ b/scripts/smokeBinaries.ts @@ -0,0 +1,59 @@ +#!/usr/bin/env bun +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Starts each built binary the way a fresh install meets it: no `SEC_*` + * environment, no `.env.local`, a working directory holding nothing. `--help` + * and `--version` must exit 0 there. + * + * `checkPackedContents.ts` proves the files ship; nothing else proves they + * start. A binary that boots its runtime before commander parses dies on + * `--help` with a configuration error, and every unit test still passes, + * because none of them run `dist`. + */ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +/** Each must print its help and version unconfigured. */ +const BINARIES = ["dist/sec.js"] as const; +const ARGS = [["--help"], ["--version"]] as const; + +const root = resolve(import.meta.dirname, ".."); +const scratch = mkdtempSync(join(tmpdir(), "sec-smoke-")); +const failures: string[] = []; + +try { + for (const binary of BINARIES) { + for (const args of ARGS) { + const line = `${binary} ${args.join(" ")}`; + const result = spawnSync(process.execPath, [join(root, binary), ...args], { + cwd: scratch, + // PATH alone: the runtime under test is this one, and everything a + // configured machine would add is what the check exists to leave out. + env: { PATH: process.env.PATH ?? "", HOME: scratch }, + encoding: "utf8", + timeout: 60_000, + }); + if (result.status === 0) { + console.log(`ok ${line}`); + continue; + } + const why = result.error?.message ?? `exit ${result.status ?? result.signal}`; + const output = `${result.stderr}${result.stdout}`.trim().split("\n").slice(0, 15).join("\n"); + failures.push(`${line}: ${why}\n${output}`); + } + } +} finally { + rmSync(scratch, { recursive: true, force: true }); +} + +if (failures.length > 0) { + console.error(`smoke-bins FAILED:\n\n${failures.join("\n\n")}`); + process.exit(1); +} diff --git a/src/config/toolchainPins.test.ts b/src/config/toolchainPins.test.ts new file mode 100644 index 00000000..d96791e1 --- /dev/null +++ b/src/config/toolchainPins.test.ts @@ -0,0 +1,78 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { GATES } from "../../scripts/releaseChecks"; + +const root = join(import.meta.dirname, "../.."); +const pkg = JSON.parse(readFileSync(join(root, "package.json"), "utf8")) as { + readonly packageManager?: string; + readonly engines?: { readonly bun?: string }; + readonly devDependencies?: Record; + readonly scripts?: Record; +}; +const workflow = readFileSync(join(root, ".github/workflows/test.yml"), "utf8"); + +/** + * `typecheck` is `bun check`, so the Bun that runs CI decides a run's result + * as much as any pinned dependency does. It is pinned in one place, + * `packageManager`, which `setup-bun` reads; everything else that names a Bun + * version is checked against it here rather than kept in step by hand. + */ +describe("the pinned Bun", () => { + const pinned = /^bun@(\d+\.\d+\.\d+)$/.exec(pkg.packageManager ?? "")?.[1]; + + it("is an exact version in packageManager", () => { + expect(pkg.packageManager).toMatch(/^bun@\d+\.\d+\.\d+$/); + }); + + it("is the engines floor", () => { + expect(pkg.engines?.bun).toBe(`>=${pinned}`); + }); + + it("is the version @types/bun describes", () => { + expect(pkg.devDependencies?.["@types/bun"]).toBe(pinned); + }); + + it("is what CI installs, read from package.json rather than restated", () => { + expect(workflow).toMatch(/bun-version-file:\s*package\.json/); + expect(workflow).not.toMatch(/bun-version:/); + }); +}); + +/** + * CI and `release` run one gate list. Separate lists drifted: CI ran five of + * the six release gates and left out the one inspecting the tarball. + */ +describe("the gate list", () => { + it("is what release-checks runs", () => { + expect(pkg.scripts?.["release-checks"]).toBe("bun ./scripts/releaseChecks.ts"); + }); + + it("is what CI runs, with no gate restated as a step of its own", () => { + expect(workflow).toMatch(/run:\s*bun run release-checks/); + for (const gate of GATES) { + expect(workflow).not.toMatch(new RegExp(`run:\\s*bun run ${gate.name}\\b`)); + } + }); + + it("names only scripts package.json defines", () => { + for (const gate of GATES) { + const [runner, verb, script] = gate.command; + expect([runner, verb]).toEqual(["bun", "run"]); + expect(pkg.scripts?.[script ?? ""], gate.name).toBeDefined(); + } + }); + + it("inspects the tarball and starts the binaries after build, ahead of the suite", () => { + const names = GATES.map((gate) => gate.name); + const build = names.indexOf("build"); + expect(names.indexOf("prepack-check")).toBeGreaterThan(build); + expect(names.indexOf("smoke-bins")).toBeGreaterThan(build); + expect(names.indexOf("test")).toBeGreaterThan(names.indexOf("smoke-bins")); + }); +}); From 64011bfbd5d3cdccbfeee5c3aaa54e100e2f4658 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:07:55 +0000 Subject: [PATCH 3/9] fix: buffer run diagnostics instead of writing under the live frame 43 console.log/warn/error calls under src/task, src/sec, src/storage and src/config ran while the Ink progress frame was up. The frame erases by line count, so a write underneath it shifted the count and the tail of the message was overwritten; a failing CIK during `sec update` left a garbled frame and an unreadable line. - src/util/diagnostics.ts (reportDiagnostic / reportDryRun): buffered inside withDeferredDiagnostics and flushed once the run settles, capped at 200 lines keeping both ends; console.warn outside a run. - runWorkflowCli runs every graph inside withDeferredDiagnostics. - Every site converted: failures and summaries to reportDiagnostic, "Would ..." lines to reportDryRun, bulk-download progress to the task's updateProgress. The console.error before a rethrow is gone; setup's "Wrote .env.local" lines are printed by the command from the task's output. unzip's per-member listing no longer goes to the terminal; its stderr is kept for the error. - The Postgres cik_names writer reports one total of collapsed duplicate CIKs instead of a line per 30,000-row slice. - consoleDiagnostics.test.ts pins console writes per file over every tree but src/cli (the sink is the one entry), exact in both directions. - runWorkflow.diagnostics.test.ts drives a failing CIK through fetchAndStoreSubmission inside runWorkflowCli and asserts the console saw nothing while the frame was up and the CIK's message after it. Fixes #388 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- .claude/CLAUDE.md | 12 ++ src/cli/groups/init.ts | 15 +- src/cli/runWorkflow.diagnostics.test.ts | 85 ++++++++ src/cli/runWorkflow.ts | 7 +- src/config/addMissingColumns.ts | 7 +- src/config/alignPostgresColumnTypes.ts | 5 +- src/config/dropStaleCheckConstraints.ts | 3 +- src/config/registerProviders.ts | 5 +- src/config/resetAllDatabases.ts | 5 +- .../submissions/EnititySubmissionSchema.ts | 3 +- src/storage/entity/cikNameBulkWriter.ts | 15 +- src/storage/xbrl/XbrlFactRepo.ts | 3 +- .../bootstrap/BootstrapAccessionDocsTask.ts | 13 +- src/task/bootstrap/BootstrapDownloadTask.ts | 26 ++- .../document/ConvertFilingDocumentTask.ts | 3 +- src/task/facts/BootstrapCompanyFactsTask.ts | 5 +- src/task/facts/UpdateAllCompanyFactsTask.ts | 5 +- src/task/facts/fetchAndStoreCompanyFacts.ts | 3 +- src/task/fetch/SecCachedFetchTask.ts | 3 +- src/task/fetch/SecFetchFileOutputCache.ts | 3 +- src/task/fixtures/FetchFixturesTask.ts | 3 +- src/task/fixtures/FetchS1FixturesTask.ts | 3 +- src/task/fixtures/GoldenFixturesTask.ts | 3 +- src/task/fixtures/fetchFixtures.ts | 3 +- src/task/fixtures/fetchS1Fixtures.ts | 3 +- src/task/index/CatchUpDailyIndexTask.test.ts | 12 +- src/task/index/CatchUpDailyIndexTask.ts | 3 +- src/task/index/FetchQuarterlyFormIdxTask.ts | 3 +- src/task/index/FetchQuarterlyIndexTask.ts | 3 +- src/task/init/InitApplyTask.ts | 8 +- src/task/kb/IndexFilingSectionsTask.ts | 3 +- .../submissions/BackfillNameHistoryTask.ts | 5 +- .../submissions/BootstrapSubmissionsTask.ts | 5 +- src/task/submissions/FetchSubmissionsTask.ts | 15 +- src/task/submissions/StoreSubmissionsTask.ts | 3 +- .../submissions/UpdateAllSubmissionsTask.ts | 5 +- .../submissions/fetchAndStoreSubmission.ts | 3 +- src/util/consoleDiagnosticBudget.ts | 24 +++ src/util/consoleDiagnostics.test.ts | 105 +++++++++ src/util/diagnostics.test.ts | 204 ++++++++++++++++++ src/util/diagnostics.ts | 163 ++++++++++++++ 41 files changed, 728 insertions(+), 77 deletions(-) create mode 100644 src/cli/runWorkflow.diagnostics.test.ts create mode 100644 src/util/consoleDiagnosticBudget.ts create mode 100644 src/util/consoleDiagnostics.test.ts create mode 100644 src/util/diagnostics.test.ts create mode 100644 src/util/diagnostics.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 7b3ae859..f83cc232 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -151,6 +151,18 @@ everything else as its primary document. 8-K is unconditional because its primar document is routinely four sentences pointing at the EX-99.1 that carries the news — for that form the exhibits *are* the filing, and only the `.txt` has them. +**Diagnostics are buffered, not printed.** Anything a task wants to tell an +operator — a CIK that failed, a skipped row, a closing summary, what a dry run +would have done — goes through `reportDiagnostic` / `reportDryRun` +(`src/util/diagnostics.ts`), never `console.*`. `runWorkflowCli` holds them until +the run settles and prints them after the frame closes; outside a run they fall +through to `console.warn`. A console write underneath the live Ink frame shifts +the lines its erase is counting, so the tail of the message is what gets +overwritten. Progress text goes through the task's `updateProgress`. The count is +pinned per file in `CONSOLE_DIAGNOSTIC_BUDGET`, exact in both directions, over +every tree but `src/cli/` (commands render their answer after the run); +`consoleDiagnostics.test.ts` fails on a new write. + **`FILING_CONVERTER_VERSION` is the only version knob left.** Bump it by hand after a parser change to re-select already-converted filings; never truncate, since a half-finished re-run then leaves the old rows readable. diff --git a/src/cli/groups/init.ts b/src/cli/groups/init.ts index e81b2191..d698cc9e 100644 --- a/src/cli/groups/init.ts +++ b/src/cli/groups/init.ts @@ -5,7 +5,12 @@ import { resolve } from "path"; import { createInterface } from "readline"; import { globalServiceRegistry } from "workglow"; import { SEC_DRY_RUN, SEC_JSON_OUTPUT } from "../../config/tokens"; -import { buildEnvConfig, InitApplyTask, type InitConfig } from "../../task/init/InitApplyTask"; +import { + buildEnvConfig, + InitApplyTask, + type InitApplyTaskOutput, + type InitConfig, +} from "../../task/init/InitApplyTask"; import { parseGlobalOptions } from "../GlobalOptions"; import { runCommand } from "../runCommand"; import { runWorkflowCli } from "../runWorkflow"; @@ -93,7 +98,13 @@ export function addInitCommand(parent: Command): void { return; } - await runWorkflowCli([new InitApplyTask({ defaults: { ...config, envPath } })]); + const applied = await runWorkflowCli([ + new InitApplyTask({ defaults: { ...config, envPath } }), + ]); + console.log(`Wrote ${applied.envPath}`); + console.log(`Created directory: ${applied.dbFolder}`); + console.log(`Created directory: ${applied.rawDataFolder}`); + console.log("Database tables created."); console.log("\n Setup complete."); suggest( diff --git a/src/cli/runWorkflow.diagnostics.test.ts b/src/cli/runWorkflow.diagnostics.test.ts new file mode 100644 index 00000000..7278bee1 --- /dev/null +++ b/src/cli/runWorkflow.diagnostics.test.ts @@ -0,0 +1,85 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { Type } from "typebox"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { type IExecuteContext, Task } from "workglow"; +import { setupAllDatabases } from "../config/setupAllDatabases"; +import { resetDependencyInjectionsForTesting } from "../config/TestingDI"; +import { fetchAndStoreSubmission } from "../task/submissions/fetchAndStoreSubmission"; +import { captureDiagnosticsForTesting, resetDiagnosticsForTesting } from "../util/diagnostics"; + +/** + * Stands in for the Ink frame: what matters is the moment it mounts and the + * moment it closes, and what the console had been handed in between. + */ +const frame: string[] = []; +let capture: ReturnType | undefined; + +vi.mock("@workglow/cli", () => ({ + withCli: vi.fn((wf: { run: (input?: unknown) => Promise }) => ({ + run: async (input?: unknown) => { + frame.push(`mounted, console had ${capture?.lines.length ?? 0} line(s)`); + try { + return await wf.run(input); + } finally { + frame.push(`closed, console had ${capture?.lines.length ?? 0} line(s)`); + } + }, + })), +})); + +/** One CIK of `sec update submissions`, whose EDGAR fetch fails. */ +class FailingCikTask extends Task, { readonly success: boolean }> { + static readonly type = "RunWorkflowDiagnosticsFailingCikTask"; + static readonly title = "Failing CIK"; + + public static inputSchema() { + return Type.Object({}); + } + + public static outputSchema() { + return Type.Object({ success: Type.Boolean() }); + } + + async execute(_input: Record, context: IExecuteContext) { + const failingFetch = { + run: async (): Promise => { + throw new Error("HTTP 503 from data.sec.gov"); + }, + }; + const ctx = { ...context, own: () => failingFetch } as unknown as IExecuteContext; + return fetchAndStoreSubmission({ cik: 320193 }, ctx); + } +} + +describe("runWorkflowCli and a failing CIK", () => { + beforeEach(async () => { + resetDependencyInjectionsForTesting(); + resetDiagnosticsForTesting(); + await setupAllDatabases(); + frame.length = 0; + capture = captureDiagnosticsForTesting(); + }); + + afterEach(() => { + capture?.restore(); + capture = undefined; + resetDiagnosticsForTesting(); + }); + + it("says nothing while the frame is up, and names the CIK once it has closed", async () => { + const { runWorkflowCli } = await import("./runWorkflow"); + + const out = await runWorkflowCli<{ readonly success: boolean }>([new FailingCikTask()]); + + // One CIK failing never fails the sweep. + expect(out.success).toBe(true); + expect(frame).toEqual(["mounted, console had 0 line(s)", "closed, console had 0 line(s)"]); + expect(capture?.lines).toEqual([ + "Failed to fetch/store submission for CIK 320193: HTTP 503 from data.sec.gov", + ]); + }); +}); diff --git a/src/cli/runWorkflow.ts b/src/cli/runWorkflow.ts index 4110df0c..df13aa25 100644 --- a/src/cli/runWorkflow.ts +++ b/src/cli/runWorkflow.ts @@ -6,6 +6,7 @@ import { withCli } from "@workglow/cli"; import { OutputTask, Workflow, type DataPorts, type ITask } from "workglow"; +import { withDeferredDiagnostics } from "../util/diagnostics"; import { isJsonOutput } from "./isJsonOutput"; /** @@ -26,6 +27,10 @@ import { isJsonOutput } from "./isJsonOutput"; * string) rather than throwing: on a TTY the workflow renderer intercepts a * thrown error with `process.exit(1)`, bypassing the calling command's error * handling and the CLI's teardown; only unexpected failures should throw. + * + * Anything a task reports through `reportDiagnostic` during the run is held + * until the run settles and printed then, clear of the live frame — whose + * erase counts lines, so a write underneath it overwrites the message's tail. */ export async function runWorkflowCli( tasks: readonly ITask[], @@ -57,6 +62,6 @@ export async function runWorkflowCli( // runs plainly by itself, which is what a redirected shard needs — the Ink UI // would write control codes into the logfile and grab raw mode, and a // backgrounded process that cannot own the TTY is stopped by SIGTTIN. - await withCli(wf, { interactive: !isJsonOutput() }).run(input); + await withDeferredDiagnostics(() => withCli(wf, { interactive: !isJsonOutput() }).run(input)); return sink.runOutputData as T; } diff --git a/src/config/addMissingColumns.ts b/src/config/addMissingColumns.ts index 2c8b303f..a77e7d41 100644 --- a/src/config/addMissingColumns.ts +++ b/src/config/addMissingColumns.ts @@ -6,6 +6,7 @@ import { globalServiceRegistry, Sqlite } from "workglow"; import { isDryRun } from "../cli/isDryRun"; +import { reportDiagnostic } from "../util/diagnostics"; import { getPgPool } from "../util/pg"; import { currentSchemaName, quote } from "../util/pgIdentifiers"; import { @@ -220,7 +221,7 @@ export function planMissingColumns( /** The warning an executor prints for a plan entry it cannot apply. */ function warnUnsupported(step: MissingColumn): void { - console.warn( + reportDiagnostic( `db setup: cannot add missing column ${step.table}.${step.column} — ${step.unsupported}. ` + `Add a hand-written migration, or recreate the database with \`sec db reset\` && ` + `\`sec db setup\` if it holds nothing you need.` @@ -253,7 +254,7 @@ export function addMissingColumnsSqlite(db: Sqlite.Database): void { // Never abort `db setup` over one column: everything after this call — // the view DDL, the resolver seeding, the component-version rows — is // unrelated to it, and the failure is reported rather than swallowed. - console.warn( + reportDiagnostic( `db setup: skipped adding ${step.table}.${step.column} — ${ err instanceof Error ? err.message : String(err) }. Resolve it and re-run \`sec db setup\`.` @@ -300,7 +301,7 @@ export async function addMissingColumnsPostgres(): Promise { try { await pool.query(sql); } catch (err) { - console.warn( + reportDiagnostic( `db setup: skipped \`${sql}\` — ${ err instanceof Error ? err.message : String(err) }. Resolve it and re-run \`sec db setup\`.` diff --git a/src/config/alignPostgresColumnTypes.ts b/src/config/alignPostgresColumnTypes.ts index eca736e8..05f093c4 100644 --- a/src/config/alignPostgresColumnTypes.ts +++ b/src/config/alignPostgresColumnTypes.ts @@ -6,6 +6,7 @@ import { globalServiceRegistry } from "workglow"; import { isDryRun } from "../cli/isDryRun"; +import { reportDiagnostic } from "../util/diagnostics"; import { getPgPool } from "../util/pg"; import { currentSchemaName, quote } from "../util/pgIdentifiers"; import { listRegisteredTables, type RegisteredTable } from "./tableRegistry"; @@ -331,7 +332,7 @@ export async function alignPostgresColumnTypes(): Promise { // operator to have, and aborting here would turn recoverable schema // drift into a hard `db setup` outage — and would also skip every // other alignment, including the ones nothing is blocking. - console.warn( + reportDiagnostic( `db setup: skipped \`${step.sql}\` — dependent view(s) ${blockers .map((v) => `"${v}"`) .join(", ")} reference ${step.table}.${step.column}. ` + @@ -349,7 +350,7 @@ export async function alignPostgresColumnTypes(): Promise { // `setupAllDatabases()` and skip everything that follows it — the view // DDL, the resolver seeding, the rate-limiter tables — over one column // nothing else depends on. - console.warn( + reportDiagnostic( `db setup: skipped \`${step.sql}\` — ${ err instanceof Error ? err.message : String(err) }. Resolve it and re-run \`sec db setup\`.` diff --git a/src/config/dropStaleCheckConstraints.ts b/src/config/dropStaleCheckConstraints.ts index bad0fd21..38497440 100644 --- a/src/config/dropStaleCheckConstraints.ts +++ b/src/config/dropStaleCheckConstraints.ts @@ -6,6 +6,7 @@ import { globalServiceRegistry } from "workglow"; import { isDryRun } from "../cli/isDryRun"; +import { reportDiagnostic } from "../util/diagnostics"; import { getPgPool } from "../util/pg"; import { currentSchemaName, quote } from "../util/pgIdentifiers"; import { nonNullType, type ObjectSchema, type PropertySchema } from "./alignPostgresColumnTypes"; @@ -198,7 +199,7 @@ export async function dropStaleCheckConstraints(): Promise { // adds capability; this one removes a guarantee the database has been // enforcing, and an operator reading the log afterwards should be able to // see that it happened without diffing the catalog. - console.warn( + reportDiagnostic( `db setup: dropping stale CHECK "${step.name}" on ${step.table}.${step.column} — ` + `the schema no longer bounds this column at >= 0.` ); diff --git a/src/config/registerProviders.ts b/src/config/registerProviders.ts index 2c9aa10d..ebb1a241 100644 --- a/src/config/registerProviders.ts +++ b/src/config/registerProviders.ts @@ -42,6 +42,9 @@ * it never aborts the CLI or the other providers. Registration is idempotent * enough for repeat bootstraps — the provider registry keeps the last registrant. */ + +import { reportDiagnostic } from "../util/diagnostics"; + /** * Where the worker entry actually is, which differs between the two ways this * module runs. From source `import.meta.url` is this file under `src/config/` @@ -112,5 +115,5 @@ async function registerHft(): Promise { function warn(provider: string, err: unknown): void { const msg = err instanceof Error ? err.message : String(err); - console.warn(`AI provider "${provider}" not registered: ${msg}`); + reportDiagnostic(`AI provider "${provider}" not registered: ${msg}`); } diff --git a/src/config/resetAllDatabases.ts b/src/config/resetAllDatabases.ts index ceca13f6..91032d8c 100644 --- a/src/config/resetAllDatabases.ts +++ b/src/config/resetAllDatabases.ts @@ -10,6 +10,7 @@ import { SEC_KB_TABLE_NAMES } from "../kb/secKbTables"; import { secFetchRateLimiterTableNames } from "../task/fetch/secFetchRateLimiterConfig"; import { secFetchRateLimiterLedgerComponents } from "../task/fetch/SecJobQueue"; import { getDb } from "../util/db"; +import { reportDiagnostic } from "../util/diagnostics"; import { getPgPool } from "../util/pg"; import { currentSchemaName, quote } from "../util/pgIdentifiers"; import { SEC_STORAGE_REGISTRY } from "./storageRegistry"; @@ -200,7 +201,7 @@ function resetSqlite(options: ResetAllDatabasesOptions): void { // scoped path below is already what either flag would degrade to. Say so // rather than accepting the flag and silently doing something else. if (options.cascade || options.dropSchema) { - console.warn( + reportDiagnostic( "db reset: --cascade / --drop-schema are Postgres-only; the SQLite reset drops the " + "tables sec owns either way." ); @@ -243,7 +244,7 @@ function reportUnownedTables(present: ReadonlyArray, owned: ReadonlyArra const ownedSet = new Set(owned); const unowned = present.filter((name) => !ownedSet.has(name)).sort(); if (unowned.length > 0) { - console.warn( + reportDiagnostic( `db reset: left ${unowned.length} table(s) in place that sec does not own: ` + `${unowned.join(", ")}. Review before dropping any by hand — ${MIGRATIONS_TABLE} is ` + `shared infrastructure that must survive, not an orphan of a removed repository.` diff --git a/src/sec/submissions/EnititySubmissionSchema.ts b/src/sec/submissions/EnititySubmissionSchema.ts index df0ea4a0..283fb41c 100644 --- a/src/sec/submissions/EnititySubmissionSchema.ts +++ b/src/sec/submissions/EnititySubmissionSchema.ts @@ -7,6 +7,7 @@ import { Static, Type } from "typebox"; import { Format } from "typebox/format"; import { ArrayToObject } from "workglow"; +import { reportDiagnostic } from "../../util/diagnostics"; import { TypeNullable } from "../../util/TypeBoxUtil"; import { TypeSecCik } from "../../util/TypeSecCik"; import { ALL_FORM_NAMES } from "../forms/all-forms"; @@ -16,7 +17,7 @@ export { TypeSecCik } from "../../util/TypeSecCik"; // const TypeSECForm = () => Type.Union(ALL_FORM_NAMES.map((f) => Type.Literal(f))); Format.Set("sec-form", (value: string) => { - if (!ALL_FORM_NAMES.includes(value as any)) console.warn(`Unknown SEC form: ${value}`); + if (!ALL_FORM_NAMES.includes(value as any)) reportDiagnostic(`Unknown SEC form: ${value}`); return true; }); export const TypeSECForm = (annotations: Record = {}) => diff --git a/src/storage/entity/cikNameBulkWriter.ts b/src/storage/entity/cikNameBulkWriter.ts index ddaa425d..11f54cd9 100644 --- a/src/storage/entity/cikNameBulkWriter.ts +++ b/src/storage/entity/cikNameBulkWriter.ts @@ -6,6 +6,7 @@ import { globalServiceRegistry } from "workglow"; import { getDb } from "../../util/db"; +import { reportDiagnostic } from "../../util/diagnostics"; import { getPgPool } from "../../util/pg"; import { resolveSqlBackend } from "../../util/sqlBackend"; import { CIK_NAME_REPOSITORY_TOKEN, type CikNameRepositoryStorage } from "./CikNameSchema"; @@ -74,6 +75,9 @@ const PG_MAX_ROWS_PER_STATEMENT = 30_000; function createPostgresWriter(): CikNameBulkWriter { const pool = getPgPool(); + // Summed across the run and reported once at close: a per-slice line meant + // one per 30,000 rows of the lookup file, which lists many CIKs twice. + let duplicatesDropped = 0; return { async writeBatch(rows: ReadonlyArray): Promise { @@ -91,11 +95,7 @@ function createPostgresWriter(): CikNameBulkWriter { // 60_000-bind cap (PG_MAX_ROWS_PER_STATEMENT * 2) still holds. const dedup = new Map(); for (const r of slice) dedup.set(r.cik, r.name); - if (dedup.size < slice.length) { - console.debug( - `cikNameBulkWriter: dedup dropped ${slice.length - dedup.size} duplicate cik(s) within a ${slice.length}-row slice` - ); - } + duplicatesDropped += slice.length - dedup.size; if (dedup.size === 0) continue; const values: (number | string)[] = []; const placeholders: string[] = []; @@ -126,6 +126,11 @@ function createPostgresWriter(): CikNameBulkWriter { }, async close(): Promise { // Pool lifetime is managed by closePgPool() at CLI shutdown. + if (duplicatesDropped > 0) { + reportDiagnostic( + `cik_names: ${duplicatesDropped} duplicate CIK row(s) collapsed within a batch, last name kept` + ); + } }, }; } diff --git a/src/storage/xbrl/XbrlFactRepo.ts b/src/storage/xbrl/XbrlFactRepo.ts index 055c7a73..ec6e032c 100644 --- a/src/storage/xbrl/XbrlFactRepo.ts +++ b/src/storage/xbrl/XbrlFactRepo.ts @@ -5,6 +5,7 @@ */ import { globalServiceRegistry } from "workglow"; +import { reportDiagnostic } from "../../util/diagnostics"; import { XBRL_FACT_REPOSITORY_TOKEN, type XbrlFactRepositoryStorage, @@ -45,7 +46,7 @@ export class XbrlFactRepo { opts: { readonly intentionalClear?: boolean } = {} ): Promise { if (rows.length === 0 && !opts.intentionalClear) { - console.warn( + reportDiagnostic( `XbrlFactRepo.replaceForAccession: 0 rows for ${accession_number} without intentionalClear — no-op` ); return; diff --git a/src/task/bootstrap/BootstrapAccessionDocsTask.ts b/src/task/bootstrap/BootstrapAccessionDocsTask.ts index a6f155ea..505e9304 100644 --- a/src/task/bootstrap/BootstrapAccessionDocsTask.ts +++ b/src/task/bootstrap/BootstrapAccessionDocsTask.ts @@ -13,6 +13,7 @@ import { isDryRun } from "../../cli/isDryRun"; import { SEC_RAW_DATA_FOLDER } from "../../config/tokens"; import { assertInsideDir, sanitizePrimaryDoc, stripXslPrefix } from "../../util/accessionDocPath"; import { describeFailureReason } from "../../util/describeFailure"; +import { reportDiagnostic, reportDryRun } from "../../util/diagnostics"; import { parseDate } from "../../util/parseDate"; import { getHttpErrorStatus } from "../fetch/SecFetchJob"; import { SecFetchTask } from "../fetch/SecFetchTask"; @@ -178,7 +179,7 @@ export class BootstrapAccessionDocsTask extends Task< try { safeName = sanitizePrimaryDoc(primaryName); } catch { - console.warn( + reportDiagnostic( `Skipping unsafe primary_doc for cik=${filing.cik} accession=${filing.accession_number}: ${JSON.stringify(primaryName)}` ); hasPrimary = false; @@ -311,7 +312,7 @@ export class BootstrapAccessionDocsTask extends Task< if (isDryRun()) { const range = input.from || input.to ? ` in range [${input.from ?? "*"} .. ${input.to ?? "*"}]` : ""; - console.log( + reportDryRun( `Would download Feed tarballs for ${dates.length} filing day(s)${range} and extract accession documents into ${join(rawDataFolder, "accessiondocs")}` ); return { success: true, daysProcessed: 0, docsWritten: 0, failed: 0 }; @@ -351,7 +352,7 @@ export class BootstrapAccessionDocsTask extends Task< // thousands of days and the ones already extracted are still worth // keeping. The marker is not written, so the next run retries it. failed++; - console.warn( + reportDiagnostic( `Feed extraction failed for ${date}: ${describeFailureReason(err, MAX_FEED_REASON_LENGTH)}` ); continue; @@ -360,7 +361,9 @@ export class BootstrapAccessionDocsTask extends Task< // No Feed archive for this day yet (e.g. very recent). Do NOT mark it // done, so a later run retries once EDGAR publishes it. missing++; - console.warn(`No Feed tarball published for ${date} yet; skipping (will retry next run).`); + reportDiagnostic( + `No Feed tarball published for ${date} yet; skipping (will retry next run).` + ); continue; } @@ -371,7 +374,7 @@ export class BootstrapAccessionDocsTask extends Task< writeFileSync(marker, ""); } - console.log( + reportDiagnostic( `Accession-doc bootstrap complete: ${daysProcessed} day(s) processed, ${docsWritten} document(s) written` + (skipped ? `, ${skipped} day(s) already done` : "") + (missing ? `, ${missing} day(s) not yet published` : "") + diff --git a/src/task/bootstrap/BootstrapDownloadTask.ts b/src/task/bootstrap/BootstrapDownloadTask.ts index 127557ec..994eb9af 100644 --- a/src/task/bootstrap/BootstrapDownloadTask.ts +++ b/src/task/bootstrap/BootstrapDownloadTask.ts @@ -12,6 +12,7 @@ import { Dataflow, globalServiceRegistry, IExecuteContext, Task, TaskGraph } fro import { isDryRun } from "../../cli/isDryRun"; import { SecUserAgent } from "../../config/Constants"; import { SEC_RAW_DATA_FOLDER } from "../../config/tokens"; +import { reportDiagnostic, reportDryRun } from "../../util/diagnostics"; import { SecFetchTask } from "../fetch/SecFetchTask"; import { ArchiveToFileTask, type ArchiveToFileTaskOutput } from "./ArchiveToFileTask"; @@ -231,7 +232,7 @@ export class BootstrapDownloadTask extends Task< const force = input.force ?? false; if (dryRun) { - console.log(`Would download ${input.url} to ${targetDir}${force ? " (forced)" : ""}`); + reportDryRun(`Would download ${input.url} to ${targetDir}${force ? " (forced)" : ""}`); return { success: true }; } @@ -261,7 +262,7 @@ export class BootstrapDownloadTask extends Task< headers["If-Modified-Since"] = usableMarker.lastModified; } - console.log(`Downloading ${input.url} ...`); + await context.updateProgress(0, `Downloading ${input.url}`); // SEC bulk archives (submissions.zip, companyfacts.zip) are multi-GB, so // this streams to disk via a producer rather than FetchUrlTask, which @@ -288,7 +289,7 @@ export class BootstrapDownloadTask extends Task< const { bytes: downloadedBytes, wrote, notModified, etag, lastModified } = response; if (notModified) { - console.log( + reportDiagnostic( `${input.targetFolder}: unchanged since ${usableMarker?.extractedAt ?? "the last run"} (HTTP 304) — skipping download and extraction.` ); return { success: true }; @@ -317,13 +318,13 @@ export class BootstrapDownloadTask extends Task< usableMarker.contentLength === downloadedBytes ) { rmSync(zipPath, { force: true }); - console.log( + reportDiagnostic( `${input.targetFolder}: unchanged since ${usableMarker.extractedAt} (matching ETag) — skipping extraction.` ); return { success: true }; } - console.log(`Download complete (${downloadedBytes} bytes). Extracting to ${targetDir} ...`); + await context.updateProgress(0, `Extracting ${downloadedBytes} bytes to ${targetDir}`); const unzipPath = Bun.which("unzip"); if (!unzipPath) { @@ -340,14 +341,19 @@ export class BootstrapDownloadTask extends Task< const unzipFlags = force ? "-o" : "-uo"; try { + // Neither stream is inherited: unzip lists every member it writes, up + // to ~1M lines, and anything written to the terminal lands underneath + // the live frame. Its stderr is read concurrently (a full pipe would + // stall it) and kept for the error, which is where it is worth reading. const proc = Bun.spawn([unzipPath, unzipFlags, zipPath, "-d", targetDir], { - stdout: "inherit", - stderr: "inherit", + stdout: "ignore", + stderr: "pipe", }); - const exitCode = await proc.exited; + const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]); if (exitCode !== 0) { - throw new Error(`unzip exited with code ${exitCode}`); + const detail = stderr.trim().split("\n").slice(-5).join("\n"); + throw new Error(`unzip exited with code ${exitCode}${detail ? `: ${detail}` : ""}`); } // Record validators only after a clean extraction, so a crash mid-unzip @@ -369,7 +375,7 @@ export class BootstrapDownloadTask extends Task< // anything we own). rmSync(zipPath, { force: true }); } - console.log(`Extraction complete. Cleaned up ${zipPath}`); + await context.updateProgress(100, `Extracted to ${targetDir}`); return { success: true }; } diff --git a/src/task/document/ConvertFilingDocumentTask.ts b/src/task/document/ConvertFilingDocumentTask.ts index 57129714..15825c61 100644 --- a/src/task/document/ConvertFilingDocumentTask.ts +++ b/src/task/document/ConvertFilingDocumentTask.ts @@ -16,6 +16,7 @@ import { type FilingSection, } from "../../storage/document/FilingSectionSchema"; import { cachedAccessionDocPath, resolvePrimaryDocName } from "../../util/accessionDocPath"; +import { reportDryRun } from "../../util/diagnostics"; import { convertFilingSubmission, FILING_CONVERTER_VERSION, @@ -294,7 +295,7 @@ export class ConvertFilingDocumentTask extends Task< if (isDryRun()) { const sections = converted.reduce((sum, doc) => sum + doc.sections.length, 0); - console.log( + reportDryRun( `Would convert ${input.accessionNumber} (${source.docFile}) to ` + `${converted.length} documents, ${sections} sections` ); diff --git a/src/task/facts/BootstrapCompanyFactsTask.ts b/src/task/facts/BootstrapCompanyFactsTask.ts index de981dc7..ebea3496 100644 --- a/src/task/facts/BootstrapCompanyFactsTask.ts +++ b/src/task/facts/BootstrapCompanyFactsTask.ts @@ -13,6 +13,7 @@ import { SEC_RAW_DATA_FOLDER } from "../../config/tokens"; import { streamProcessedCikSet } from "../../storage/processing/processedCikSet"; import { PROCESSED_FACTS_REPOSITORY_TOKEN } from "../../storage/processing/ProcessedFactsSchema"; import { todayYYYYdMMdDD } from "../../util/dataCleaningUtils"; +import { reportDryRun } from "../../util/diagnostics"; import { fetchAndStoreCompanyFacts } from "./fetchAndStoreCompanyFacts"; export type BootstrapCompanyFactsTaskInput = { @@ -63,7 +64,7 @@ export class BootstrapCompanyFactsTask extends Task< const err = e as NodeJS.ErrnoException; if (err.code === "ENOENT") { if (isDryRun()) { - console.log( + reportDryRun( `Would bootstrap CIK company facts after bulk data populates ${companyfactsDir}` ); return { success: true }; @@ -96,7 +97,7 @@ export class BootstrapCompanyFactsTask extends Task< if (isDryRun()) { const label = input.force ? "all" : "unprocessed"; - console.log( + reportDryRun( `Would bootstrap ${ciksToProcess.length} ${label} CIK company facts (${ciks.length} total)` ); return { success: true }; diff --git a/src/task/facts/UpdateAllCompanyFactsTask.ts b/src/task/facts/UpdateAllCompanyFactsTask.ts index d20450ab..744e263f 100644 --- a/src/task/facts/UpdateAllCompanyFactsTask.ts +++ b/src/task/facts/UpdateAllCompanyFactsTask.ts @@ -13,6 +13,7 @@ import { type ProcessedFacts, } from "../../storage/processing/ProcessedFactsSchema"; import { todayYYYYdMMdDD } from "../../util/dataCleaningUtils"; +import { reportDryRun } from "../../util/diagnostics"; import { computeFactsWorklist, type FactsWorkItem } from "./computeFactsWorklist"; import { listFactsEligibleCiks } from "./factsEligibleCiks"; import { fetchAndStoreCompanyFacts } from "./fetchAndStoreCompanyFacts"; @@ -103,7 +104,7 @@ export class UpdateAllCompanyFactsTask extends Task< if (isDryRun()) { if (input.force) { - console.log( + reportDryRun( `Would update ${needsUpdating.length} company facts (force — reprocessing all)` ); } else { @@ -122,7 +123,7 @@ export class UpdateAllCompanyFactsTask extends Task< eligible === undefined ? " (unfiltered — every never-processed CIK)" : ` (of ${neverProcessed} never processed, ${eligible.size} pass the XBRL/SIC filter)`; - console.log( + reportDryRun( `Would update ${needsUpdating.length} changed${retrySuffix} and ${needsProcessing.length} new company facts${scope}` ); } diff --git a/src/task/facts/fetchAndStoreCompanyFacts.ts b/src/task/facts/fetchAndStoreCompanyFacts.ts index c6c1f83c..daed3cf0 100644 --- a/src/task/facts/fetchAndStoreCompanyFacts.ts +++ b/src/task/facts/fetchAndStoreCompanyFacts.ts @@ -8,6 +8,7 @@ import { IExecuteContext, TaskAbortedError } from "workglow"; import type { FactsReasonCode } from "../../storage/processing/ProcessedFactsSchema"; import { recordFactsOutcome } from "../../storage/processing/recordFactsOutcome"; import { todayYYYYdMMdDD } from "../../util/dataCleaningUtils"; +import { reportDiagnostic } from "../../util/diagnostics"; import { classifyFactsFetchError } from "./classifyFactsFetchError"; import { FetchCompanyFactsTask, FetchCompanyFactsTaskOutput } from "./FetchCompanyFactsTask"; import { StoreCompanyFactsTask } from "./StoreCompanyFactsTask"; @@ -56,7 +57,7 @@ export async function fetchAndStoreCompanyFactsWithDeps( const recordFailure = async (reason_code: FactsReasonCode, e: unknown): Promise => { const message = e instanceof Error ? e.message : String(e); - console.warn( + reportDiagnostic( `Failed to ${reason_code === "STORE_ERROR" ? "store" : "fetch"} company facts for CIK ${input.cik} (${reason_code}): ${message}` ); await recordFactsOutcome({ diff --git a/src/task/fetch/SecCachedFetchTask.ts b/src/task/fetch/SecCachedFetchTask.ts index 41511423..c6a6302f 100644 --- a/src/task/fetch/SecCachedFetchTask.ts +++ b/src/task/fetch/SecCachedFetchTask.ts @@ -14,6 +14,7 @@ import { TaskOutput, } from "workglow"; import { SEC_RAW_DATA_FOLDER } from "../../config/tokens"; +import { reportDiagnostic } from "../../util/diagnostics"; import { YYYYdMMdDD } from "../../util/parseDate"; import { SecFetchFileOutputCache } from "./SecFetchFileOutputCache"; import { SecFetchTask } from "./SecFetchTask"; @@ -134,7 +135,7 @@ export function resetFetchCacheWarningForTesting(): void { function warnFetchCacheDisabled(): void { if (warnedFetchCacheDisabled) return; warnedFetchCacheDisabled = true; - console.warn( + reportDiagnostic( `SecCachedFetchTask: SEC_RAW_DATA_FOLDER is not configured, so NO fetch cache is installed — ` + `every fetch re-downloads and nothing is read from disk, with no other symptom. Set it (env ` + `var or .env.local) to restore caching; it is also the root for accessiondocs/, so leaving ` + diff --git a/src/task/fetch/SecFetchFileOutputCache.ts b/src/task/fetch/SecFetchFileOutputCache.ts index f34c9f47..bc0bc003 100644 --- a/src/task/fetch/SecFetchFileOutputCache.ts +++ b/src/task/fetch/SecFetchFileOutputCache.ts @@ -18,6 +18,7 @@ import { } from "workglow"; import { isDryRun } from "../../cli/isDryRun"; import { tmpPathFor, writeFully } from "../../util/atomicFileWrite"; +import { reportDiagnostic } from "../../util/diagnostics"; import { secDate, YYYYdMMdDD } from "../../util/parseDate"; /** @@ -155,7 +156,7 @@ export class SecFetchFileOutputCache extends TaskOutputRepository { // ArrayBuffer without copying; saveOutput writes it in binary mode. return output.arraybuffer ? Buffer.from(output.arraybuffer as ArrayBuffer) : Buffer.alloc(0); } else { - console.warn(`Unknown response type: ${response_type}, assuming text`); + reportDiagnostic(`Unknown response type: ${response_type}, assuming text`); return output.text; } } diff --git a/src/task/fixtures/FetchFixturesTask.ts b/src/task/fixtures/FetchFixturesTask.ts index 616044f0..984c73db 100644 --- a/src/task/fixtures/FetchFixturesTask.ts +++ b/src/task/fixtures/FetchFixturesTask.ts @@ -7,6 +7,7 @@ import { Type } from "typebox"; import { Task } from "workglow"; import type { ExemptOfferingFormCode } from "../../sec/forms/exempt-offerings/form-slugs"; +import { reportDiagnostic } from "../../util/diagnostics"; import { fetchFixtures } from "./fetchFixtures"; export type FetchFixturesTaskInput = { @@ -54,7 +55,7 @@ export class FetchFixturesTask extends Task console.log(msg), + log: reportDiagnostic, }); return { downloaded: result.downloaded, diff --git a/src/task/fixtures/FetchS1FixturesTask.ts b/src/task/fixtures/FetchS1FixturesTask.ts index 5e516497..4430522b 100644 --- a/src/task/fixtures/FetchS1FixturesTask.ts +++ b/src/task/fixtures/FetchS1FixturesTask.ts @@ -6,6 +6,7 @@ import { Type } from "typebox"; import { Task } from "workglow"; +import { reportDiagnostic } from "../../util/diagnostics"; import { fetchS1Fixtures } from "./fetchS1Fixtures"; import { edgarS1Deps } from "./s1FixtureSource"; @@ -49,7 +50,7 @@ export class FetchS1FixturesTask extends Task console.log(msg)), + deps: edgarS1Deps(reportDiagnostic), }); return { downloaded: result.downloaded, diff --git a/src/task/fixtures/GoldenFixturesTask.ts b/src/task/fixtures/GoldenFixturesTask.ts index d881b8dd..b87309e7 100644 --- a/src/task/fixtures/GoldenFixturesTask.ts +++ b/src/task/fixtures/GoldenFixturesTask.ts @@ -6,6 +6,7 @@ import { Type } from "typebox"; import { Task } from "workglow"; +import { reportDiagnostic } from "../../util/diagnostics"; import type { GoldenFixtureMode } from "./goldenFixtures"; import { runGoldenFixtures } from "./goldenFixtures"; import { edgarGoldenFixtureDeps } from "./goldenFixtureSource"; @@ -55,7 +56,7 @@ export class GoldenFixturesTask extends Task console.log(msg)), + deps: edgarGoldenFixtureDeps(reportDiagnostic), }); return { ok: result.ok, diff --git a/src/task/fixtures/fetchFixtures.ts b/src/task/fixtures/fetchFixtures.ts index 772896fa..9eb62b97 100644 --- a/src/task/fixtures/fetchFixtures.ts +++ b/src/task/fixtures/fetchFixtures.ts @@ -30,6 +30,7 @@ import { type ExemptOfferingFormCode, } from "../../sec/forms/exempt-offerings/form-slugs"; import { accessionFromFileName, accessionWithoutDashes } from "../../util/accession"; +import { reportDiagnostic } from "../../util/diagnostics"; import { FetchQuarterlyFormIdxTask, type QuarterlyFormIdxRow, @@ -235,7 +236,7 @@ export async function fetchFixtures( } parseQuarterStrings(quarters); // validate shape - console.log(`[sec fetch] output dir: ${MOCK_ROOT}`); + reportDiagnostic(`[sec fetch] output dir: ${MOCK_ROOT}`); // Pull form.idx for each requested quarter (cached on disk via the // SecCachedFetchTask layer; second runs in the same quarter are free). diff --git a/src/task/fixtures/fetchS1Fixtures.ts b/src/task/fixtures/fetchS1Fixtures.ts index e85b5f35..583a0b72 100644 --- a/src/task/fixtures/fetchS1Fixtures.ts +++ b/src/task/fixtures/fetchS1Fixtures.ts @@ -23,6 +23,7 @@ import { existsSync, mkdirSync, readdirSync, writeFileSync } from "fs"; import { join, resolve } from "path"; +import { reportDiagnostic } from "../../util/diagnostics"; export const DEFAULT_S1_SAMPLE_COUNT = 10; export const DEFAULT_MIN_SPAC = 3; @@ -139,7 +140,7 @@ export async function fetchS1Fixtures( const shuffle = deps.shuffle ?? defaultShuffle; if (!existsSync(cacheDir)) mkdirSync(cacheDir, { recursive: true }); - console.log(`[sec fetch] output dir: ${cacheDir}`); + reportDiagnostic(`[sec fetch] output dir: ${cacheDir}`); log("Listing recent S-1 candidates ..."); const candidates = await deps.listCandidates(); diff --git a/src/task/index/CatchUpDailyIndexTask.test.ts b/src/task/index/CatchUpDailyIndexTask.test.ts index 653e52ab..2fed335b 100644 --- a/src/task/index/CatchUpDailyIndexTask.test.ts +++ b/src/task/index/CatchUpDailyIndexTask.test.ts @@ -14,6 +14,7 @@ import { setupAllDatabases } from "../../config/setupAllDatabases"; import { resetDependencyInjectionsForTesting } from "../../config/TestingDI"; import { SEC_DRY_RUN, SEC_RAW_DATA_FOLDER } from "../../config/tokens"; import { CIK_LAST_UPDATE_REPOSITORY_TOKEN } from "../../storage/processing/CikLastUpdateSchema"; +import { captureDiagnosticsForTesting } from "../../util/diagnostics"; import { DAILY_INDEX_CURSOR_ID, DAILY_INDEX_CURSOR_REPOSITORY_TOKEN, @@ -243,9 +244,14 @@ describe("CatchUpDailyIndexTask", () => { it("does not write the cursor in dry-run mode", async () => { globalServiceRegistry.registerInstance(SEC_DRY_RUN, true); const runSpy = vi.spyOn(FetchDailyIndexTask.prototype, "run"); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const reported = captureDiagnosticsForTesting(); - const result = await new CatchUpDailyIndexTask().execute({}, ctx()); + let result; + try { + result = await new CatchUpDailyIndexTask().execute({}, ctx()); + } finally { + reported.restore(); + } expect(runSpy).not.toHaveBeenCalled(); expect(result).toMatchObject({ @@ -255,7 +261,7 @@ describe("CatchUpDailyIndexTask", () => { todayFetched: false, lastSuccess: "2026-08-14", }); - expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Would fetch completed")); + expect(reported.lines).toEqual([expect.stringContaining("dry run: Would fetch completed")]); const cursor = await globalServiceRegistry .get(DAILY_INDEX_CURSOR_REPOSITORY_TOKEN) diff --git a/src/task/index/CatchUpDailyIndexTask.ts b/src/task/index/CatchUpDailyIndexTask.ts index f8b2241e..e60f20cd 100644 --- a/src/task/index/CatchUpDailyIndexTask.ts +++ b/src/task/index/CatchUpDailyIndexTask.ts @@ -16,6 +16,7 @@ import { DAILY_INDEX_CURSOR_ID, DAILY_INDEX_CURSOR_REPOSITORY_TOKEN, } from "../../storage/processing/DailyIndexCursorSchema"; +import { reportDryRun } from "../../util/diagnostics"; import { TypeSecDate } from "../../util/parseDate"; import { getHttpErrorStatus } from "../fetch/SecFetchJob"; import { @@ -155,7 +156,7 @@ export class CatchUpDailyIndexTask extends Task< }); if (isDryRun()) { - console.log(`Would fetch completed [${plan.completed.join(", ")}], today ${plan.today}`); + reportDryRun(`Would fetch completed [${plan.completed.join(", ")}], today ${plan.today}`); return { success: true, fetched: 0, diff --git a/src/task/index/FetchQuarterlyFormIdxTask.ts b/src/task/index/FetchQuarterlyFormIdxTask.ts index 11a2bc42..4fcb3c87 100644 --- a/src/task/index/FetchQuarterlyFormIdxTask.ts +++ b/src/task/index/FetchQuarterlyFormIdxTask.ts @@ -6,6 +6,7 @@ import { Type } from "typebox"; import { DataPortSchemaObject, IExecuteContext, Task, TaskAbortedError } from "workglow"; +import { reportDiagnostic } from "../../util/diagnostics"; import { parseDate, secDate, TypeOptionalSecDate, YYYYdMMdDD } from "../../util/parseDate"; import { response_type, SecCachedFetchTask } from "../fetch/SecCachedFetchTask"; @@ -116,7 +117,7 @@ export function parseQuarterlyFormIdx(content: string): QuarterlyFormIdxRow[] { // A single calendar-invalid dateFiled must not discard the whole // quarter's parsed rows (parseDate now throws on e.g. Feb 30). Skip // the row and continue. - console.warn( + reportDiagnostic( `parseQuarterlyFormIdx: skipping row with unparseable date "${dateFiled}": ${ err instanceof Error ? err.message : String(err) }` diff --git a/src/task/index/FetchQuarterlyIndexTask.ts b/src/task/index/FetchQuarterlyIndexTask.ts index 4a4af495..a8a63d0e 100644 --- a/src/task/index/FetchQuarterlyIndexTask.ts +++ b/src/task/index/FetchQuarterlyIndexTask.ts @@ -8,6 +8,7 @@ import { parse } from "csv-parse"; import { Type } from "typebox"; import { DataPortSchemaObject, IExecuteContext, Task, TaskAbortedError } from "workglow"; import { TypeSecCik } from "../../sec/submissions/EnititySubmissionSchema"; +import { reportDiagnostic } from "../../util/diagnostics"; import { parseDate, secDate, @@ -127,7 +128,7 @@ export class FetchQuarterlyIndexTask extends Task< // A single calendar-invalid `Date Filed` must not abort the // whole quarter's CIK-update batch (parseDate now throws on // e.g. Feb 30). Skip the row and continue. - console.warn( + reportDiagnostic( `FetchQuarterlyIndexTask: skipping cik ${cikStr} row with unparseable date "${date}": ${ err instanceof Error ? err.message : String(err) }` diff --git a/src/task/init/InitApplyTask.ts b/src/task/init/InitApplyTask.ts index c1492a68..5503eaf0 100644 --- a/src/task/init/InitApplyTask.ts +++ b/src/task/init/InitApplyTask.ts @@ -99,14 +99,11 @@ export class InitApplyTask extends Task, InitApply async execute(input: TaskPorts): Promise { const { envPath, ...config } = input; + // What this did is reported by `sec setup` from the output ports, once the + // run's frame has closed, rather than printed underneath it. writeFileSync(envPath, buildEnvConfig(config), "utf-8"); - console.log(`Wrote ${envPath}`); - mkdirSync(config.dbFolder, { recursive: true }); - console.log(`Created directory: ${config.dbFolder}`); - mkdirSync(config.rawDataFolder, { recursive: true }); - console.log(`Created directory: ${config.rawDataFolder}`); // Re-read env so DI picks up new values. Push every var the // wizard collected, including the Postgres set — assertSecCliEnvConfigured @@ -131,7 +128,6 @@ export class InitApplyTask extends Task, InitApply DefaultDI(); await setupAllDatabases(); - console.log("Database tables created."); return { envPath, dbFolder: config.dbFolder, rawDataFolder: config.rawDataFolder }; } diff --git a/src/task/kb/IndexFilingSectionsTask.ts b/src/task/kb/IndexFilingSectionsTask.ts index 6e99c627..eba7589a 100644 --- a/src/task/kb/IndexFilingSectionsTask.ts +++ b/src/task/kb/IndexFilingSectionsTask.ts @@ -25,6 +25,7 @@ import { type FilingSection, } from "../../storage/document/FilingSectionSchema"; import { accessionWithoutDashes } from "../../util/accession"; +import { reportDryRun } from "../../util/diagnostics"; import type { TaskPorts } from "../taskPorts"; import { countAlreadyIndexed, kbDocIdFor, selectDocumentsToIndex } from "./selectDocumentsToIndex"; @@ -209,7 +210,7 @@ export class IndexFilingSectionsTask extends Task< // between an upsert here and a committed row. The selection above is a // read, so it still reports what the run would do. if (isDryRun()) { - console.log(`Would index ${work.length} filing(s) into the knowledge base.`); + reportDryRun(`Would index ${work.length} filing(s) into the knowledge base.`); return { success: true, indexed: 0, sections: 0, skipped, truncated }; } diff --git a/src/task/submissions/BackfillNameHistoryTask.ts b/src/task/submissions/BackfillNameHistoryTask.ts index 46092df9..7e7be790 100644 --- a/src/task/submissions/BackfillNameHistoryTask.ts +++ b/src/task/submissions/BackfillNameHistoryTask.ts @@ -14,6 +14,7 @@ import { ENTITY_HISTORY_REPOSITORY_TOKEN, type EntityHistory, } from "../../storage/entity/EntityHistorySchema"; +import { reportDiagnostic, reportDryRun } from "../../util/diagnostics"; import { buildNameHistoryRows, NAME_HISTORY_CHANGE_SOURCE } from "./nameHistoryRows"; const CIK_FILE_PATTERN = /^CIK(\d+)\.json$/; @@ -92,7 +93,7 @@ export class BackfillNameHistoryTask extends Task< } if (isDryRun()) { - console.log(`Would scan ${files.length} submission file(s) in ${submissionsDir}`); + reportDryRun(`Would scan ${files.length} submission file(s) in ${submissionsDir}`); return { success: true, ciksWithRenames: 0, rowsWritten: 0 }; } @@ -153,7 +154,7 @@ export class BackfillNameHistoryTask extends Task< rowsWritten += pending.length; } - console.log( + reportDiagnostic( `Name history: ${rowsWritten} row(s) across ${ciksWithRenames} renamed CIK(s) from ${scanned} submission file(s).` ); return { success: true, ciksWithRenames, rowsWritten }; diff --git a/src/task/submissions/BootstrapSubmissionsTask.ts b/src/task/submissions/BootstrapSubmissionsTask.ts index 25e92b8a..8a069e60 100644 --- a/src/task/submissions/BootstrapSubmissionsTask.ts +++ b/src/task/submissions/BootstrapSubmissionsTask.ts @@ -12,6 +12,7 @@ import { isDryRun } from "../../cli/isDryRun"; import { SEC_RAW_DATA_FOLDER } from "../../config/tokens"; import { streamProcessedCikSet } from "../../storage/processing/processedCikSet"; import { PROCESSED_SUBMISSIONS_REPOSITORY_TOKEN } from "../../storage/processing/ProcessedSubmissionsSchema"; +import { reportDryRun } from "../../util/diagnostics"; import { fetchAndStoreSubmission } from "./fetchAndStoreSubmission"; export type BootstrapSubmissionsTaskInput = { @@ -62,7 +63,7 @@ export class BootstrapSubmissionsTask extends Task< const err = e as NodeJS.ErrnoException; if (err.code === "ENOENT") { if (isDryRun()) { - console.log( + reportDryRun( `Would bootstrap CIK submissions after bulk data populates ${submissionsDir}` ); return { success: true }; @@ -99,7 +100,7 @@ export class BootstrapSubmissionsTask extends Task< if (isDryRun()) { const label = input.force ? "all" : "unprocessed"; - console.log( + reportDryRun( `Would bootstrap ${ciksToProcess.length} ${label} CIK submissions (${ciks.length} total)` ); return { success: true }; diff --git a/src/task/submissions/FetchSubmissionsTask.ts b/src/task/submissions/FetchSubmissionsTask.ts index e5d91c7b..6f2dba43 100644 --- a/src/task/submissions/FetchSubmissionsTask.ts +++ b/src/task/submissions/FetchSubmissionsTask.ts @@ -72,15 +72,12 @@ export class FetchSubmissionsTask extends Task + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Raw `console.*` writes allowed under the trees a run executes in, per file, + * keyed by path from `src/`. `consoleDiagnostics.test.ts` fails on a count + * that does not match — in either direction, so a conversion lowers the + * number rather than leaving headroom for the next write. + * + * Not an allowlist to add to. A write from inside a run lands underneath the + * live Ink frame, whose erase counts lines, so the frame overwrites the tail of + * the message. Route it through `reportDiagnostic` (`./diagnostics`), report + * progress through the task's `updateProgress`, or throw when an operator must + * act immediately. A command's own output belongs in `src/cli/`, which renders + * after the run and is not scanned. + * + * The one entry is the sink everything else is routed to. + */ +export const CONSOLE_DIAGNOSTIC_BUDGET: Readonly> = { + "util/diagnostics.ts": 1, +}; diff --git a/src/util/consoleDiagnostics.test.ts b/src/util/consoleDiagnostics.test.ts new file mode 100644 index 00000000..c3632774 --- /dev/null +++ b/src/util/consoleDiagnostics.test.ts @@ -0,0 +1,105 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { readdirSync, readFileSync } from "node:fs"; +import { join, relative } from "node:path"; +import { describe, expect, it } from "vitest"; +import { CONSOLE_DIAGNOSTIC_BUDGET } from "./consoleDiagnosticBudget"; + +const SRC_DIR = join(import.meta.dirname, ".."); + +/** + * Every tree a task's work can reach while the frame is up. `src/cli/` is the + * exception: it holds the commands, which render their answer after the run, + * and the entrypoints at the top of `src/` print before any run exists. + */ +const SCANNED = [ + "commands", + "config", + "kb", + "sec", + "storage", + "task", + "util", + "verify", + "web", +] as const; + +const CONSOLE_RE = /\bconsole\.(log|warn|error|info|debug|trace)\s*\(/g; + +function tsFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "mock_data" || entry.name === "node_modules") continue; + out.push(...tsFiles(full)); + continue; + } + if (!entry.name.endsWith(".ts") || entry.name.endsWith(".test.ts")) continue; + out.push(full); + } + return out; +} + +function countConsoleWrites(): Map { + const counts = new Map(); + for (const tree of SCANNED) { + for (const file of tsFiles(join(SRC_DIR, tree))) { + const matches = readFileSync(file, "utf8").match(CONSOLE_RE); + if (matches) counts.set(relative(SRC_DIR, file), matches.length); + } + } + return counts; +} + +/** + * A console write from inside a run prints underneath the live Ink frame, + * whose erase is a line count, so the frame overwrites the tail of the message + * — usually the part worth reading. `runWorkflowCli` defers whatever goes + * through `reportDiagnostic` until the frame has closed. + * + * All the console methods count: the hazard is the line, not the severity, and + * most of the writes converted here were `console.log` summaries and dry-run + * lines rather than warnings. The budget is exact in both directions, so a + * converted file cannot quietly grow back to where it was. + */ +describe("console writes under the run trees", () => { + const counts = countConsoleWrites(); + + it("does not grow past the recorded budget", () => { + const regressions: string[] = []; + for (const [file, count] of [...counts].sort()) { + const budget = CONSOLE_DIAGNOSTIC_BUDGET[file] ?? 0; + if (count > budget) { + regressions.push( + `${file}: ${count} console write(s), budget ${budget} — use reportDiagnostic() from ` + + `src/util/diagnostics.ts, the task's updateProgress for progress text, or throw if ` + + `an operator must act on it immediately` + ); + } + } + expect(regressions).toEqual([]); + }); + + it("has no budget left over from a conversion that already happened", () => { + const stale: string[] = []; + for (const [file, budget] of Object.entries(CONSOLE_DIAGNOSTIC_BUDGET).sort()) { + const count = counts.get(file) ?? 0; + if (count < budget) { + stale.push( + count === 0 + ? `${file}: none left — remove its entry from CONSOLE_DIAGNOSTIC_BUDGET` + : `${file}: ${count} left, budget ${budget} — lower it to ${count}` + ); + } + } + expect(stale).toEqual([]); + }); + + it("finds the sink, so a broken scan cannot pass by finding nothing", () => { + expect(counts.get("util/diagnostics.ts")).toBe(1); + }); +}); diff --git a/src/util/diagnostics.test.ts b/src/util/diagnostics.test.ts new file mode 100644 index 00000000..041f83c1 --- /dev/null +++ b/src/util/diagnostics.test.ts @@ -0,0 +1,204 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + reportDiagnostic, + reportDryRun, + resetDiagnosticsForTesting, + withDeferredDiagnostics, +} from "./diagnostics"; + +/** Captures everything `reportDiagnostic` lets through to the console. */ +function captureWarnings(): { readonly lines: string[]; restore: () => void } { + const lines: string[] = []; + const warn = vi.spyOn(console, "warn").mockImplementation((...args: unknown[]) => { + lines.push(String(args[0] ?? "")); + }); + return { lines, restore: () => warn.mockRestore() }; +} + +describe("reportDiagnostic", () => { + afterEach(() => { + resetDiagnosticsForTesting(); + }); + + it("writes straight to the console when no scope is open", () => { + // The fall-through case, and the reason routing a call site through this + // module is safe on its own: a parser called from a unit test, a library + // consumer or any path that never reaches the CLI keeps printing exactly + // as it did before. Nothing is buffered into a scope that will never flush. + const { lines, restore } = captureWarnings(); + try { + reportDiagnostic("straight through"); + } finally { + restore(); + } + expect(lines).toEqual(["straight through"]); + }); + + it("buffers a dry-run line like any other, marked as one", async () => { + // A `--dry-run` task runs under the same live frame as a real one, so its + // "Would …" line is deferred the same way — and prefixed, so the flushed + // lines tell what was skipped on purpose from what failed. + const { lines, restore } = captureWarnings(); + let duringRun: string[] = []; + try { + await withDeferredDiagnostics(async () => { + reportDryRun("Would download the archive"); + duringRun = [...lines]; + }); + } finally { + restore(); + } + expect(duringRun).toEqual([]); + expect(lines).toEqual(["dry run: Would download the archive"]); + }); + + it("says nothing until the deferred scope closes, then says it in order", async () => { + const { lines, restore } = captureWarnings(); + let duringRun: string[] = []; + try { + await withDeferredDiagnostics(async () => { + reportDiagnostic("first"); + reportDiagnostic("second"); + // The property the whole module exists for: while the run is live — + // and on a TTY, while the Ink frame is mounted — the console has seen + // nothing at all. + duringRun = [...lines]; + }); + } finally { + restore(); + } + expect(duringRun).toEqual([]); + expect(lines).toEqual(["first", "second"]); + }); + + it("flushes what it collected when the run throws", async () => { + // The diagnostics a failing run collected are usually the explanation for + // the failure, so they must survive it — and reach the console BEFORE the + // error propagates to the command's handler, so they read above it. + const { lines, restore } = captureWarnings(); + try { + await expect( + withDeferredDiagnostics(async () => { + reportDiagnostic("observed before the throw"); + throw new Error("run failed"); + }) + ).rejects.toThrow("run failed"); + } finally { + restore(); + } + expect(lines).toEqual(["observed before the throw"]); + }); + + it("flushes once, at the outermost scope", async () => { + // A command that nests runs would otherwise have the inner scope print into + // the frame the outer one is still drawing — the exact bug being fixed, + // reintroduced by the fix. + const { lines, restore } = captureWarnings(); + let afterInner: string[] = []; + try { + await withDeferredDiagnostics(async () => { + reportDiagnostic("outer"); + await withDeferredDiagnostics(async () => { + reportDiagnostic("inner"); + }); + afterInner = [...lines]; + }); + } finally { + restore(); + } + expect(afterInner).toEqual([]); + expect(lines).toEqual(["outer", "inner"]); + }); + + it("reopens cleanly for the next run", async () => { + const { lines, restore } = captureWarnings(); + try { + await withDeferredDiagnostics(async () => reportDiagnostic("run one")); + await withDeferredDiagnostics(async () => reportDiagnostic("run two")); + } finally { + restore(); + } + expect(lines).toEqual(["run one", "run two"]); + }); + + it("caps the buffer by dropping the middle, keeping both ends", async () => { + // Volume is driven by the archive, not by us. The cap keeps a pathological + // snapshot from holding an unbounded array, and the marker keeps it from + // reading as the complete list. + // + // It drops the MIDDLE rather than the tail because of what sits at each + // end: a run's cheap repetitive lines (one per malformed row batch, one per + // failed ticker window) are what reach the cap, and its end-of-run + // accounting — the replacement report, the quote sweep's anchor failures — + // is raised last. Keeping a prefix alone threw away the outcome and kept + // the noise that caused the overrun. + const { lines, restore } = captureWarnings(); + try { + await withDeferredDiagnostics(async () => { + for (let i = 0; i < 250; i += 1) reportDiagnostic(`line ${i}`); + }); + } finally { + restore(); + } + expect(lines).toHaveLength(201); + expect(lines[0]).toBe("line 0"); + expect(lines[149]).toBe("line 149"); + expect(lines[150]).toContain("50 further diagnostic(s)"); + expect(lines[151]).toBe("line 200"); + expect(lines[200]).toBe("line 249"); + }); + + it("flushes from a process exit the run never returns from", async () => { + // The `finally` is not enough on its own. On a TTY the workflow renderer + // answers a thrown run by calling `process.exit(1)` itself, so nothing on + // the stack unwinds and the lines explaining the failure would be lost in + // exactly the environment the deferral exists for. + const { lines, restore } = captureWarnings(); + const before = process.listeners("exit"); + let duringRun = 0; + let flushedOnExit: string[] = []; + try { + await withDeferredDiagnostics(async () => { + reportDiagnostic("observed before the exit"); + const installed = process.listeners("exit").filter((l) => !before.includes(l)); + duringRun = installed.length; + // What `process.exit()` does to the listener this scope installed — + // only that one, so the runner's own exit handling is left alone. + for (const listener of installed) listener(0); + flushedOnExit = [...lines]; + }); + } finally { + restore(); + resetDiagnosticsForTesting(); + } + expect(duringRun).toBe(1); + expect(flushedOnExit).toEqual(["observed before the exit"]); + // The listener is removed with the scope, so runs do not accumulate them. + expect(process.listenerCount("exit")).toBe(before.length); + }); + + it("survives a reset taken from inside an open scope", async () => { + // `resetDiagnosticsForTesting` exists to be called after a test threw + // inside a scope, so it zeroes the depth the scope is still holding. A bare + // decrement in the scope's `finally` then left it negative, and every later + // run in the process silently stopped deferring. + const { lines, restore } = captureWarnings(); + try { + await withDeferredDiagnostics(async () => { + resetDiagnosticsForTesting(); + }); + await withDeferredDiagnostics(async () => { + reportDiagnostic("still deferred"); + expect(lines).toEqual([]); + }); + } finally { + restore(); + } + expect(lines).toEqual(["still deferred"]); + }); +}); diff --git a/src/util/diagnostics.ts b/src/util/diagnostics.ts new file mode 100644 index 00000000..71f78762 --- /dev/null +++ b/src/util/diagnostics.ts @@ -0,0 +1,163 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Operator-facing diagnostics raised from inside a run: a CIK that failed to + * fetch, a skipped index row, a run's closing summary, what a dry run would + * have done. Worth telling an operator, never worth failing over. + * + * Inside {@link withDeferredDiagnostics} they are buffered and printed after + * the run, clear of the CLI's live progress frame. Anywhere else — a unit test, + * a library caller — they go straight to `console.warn`. + */ + +/** How many diagnostics one deferred run prints; the rest are counted and dropped. */ +const MAX_BUFFERED = 200; + +/** + * How much of {@link MAX_BUFFERED} is reserved for the LAST diagnostics a run + * raised: end-of-run accounting comes last, and the repetitive per-row lines + * that fill the buffer come first, so an overrun keeps both ends and drops the + * middle. + */ +const TAIL_BUFFERED = 50; +const HEAD_BUFFERED = MAX_BUFFERED - TAIL_BUFFERED; + +let buffered: string[] | null = null; +/** The most recent {@link TAIL_BUFFERED} diagnostics, once the head is full. */ +let bufferedTail: string[] = []; +let dropped = 0; +/** Nesting depth of {@link withDeferredDiagnostics}; only the outermost scope flushes. */ +let depth = 0; +/** Where a test told the console's lines to go instead; see {@link captureDiagnosticsForTesting}. */ +let captured: string[] | null = null; + +/** The one sink: the capture a test installed, else the console. */ +function emit(message: string): void { + if (captured !== null) { + captured.push(message); + return; + } + console.warn(message); +} + +/** + * Buffered while a deferred scope is open, written to `console.warn` otherwise. + * + * No level and no category — everything here is the same severity. Anything an + * operator must act on immediately should throw instead. + */ +export function reportDiagnostic(message: string): void { + if (buffered === null) { + emit(message); + return; + } + if (buffered.length < HEAD_BUFFERED) { + buffered.push(message); + return; + } + bufferedTail.push(message); + if (bufferedTail.length > TAIL_BUFFERED) { + bufferedTail.shift(); + dropped += 1; + } +} + +/** + * What a `--dry-run` would have done, stated as `Would …`. A task under + * `--dry-run` still runs under the live frame, so a `console.log` of that line + * shifts the frame exactly as a warning does; it goes through the same buffer, + * with a prefix so the flushed lines tell a skipped write from a failed one. + */ +export function reportDryRun(message: string): void { + reportDiagnostic(`dry run: ${message}`); +} + +/** + * Runs `body` with diagnostics buffered, flushing them once it settles. + * + * The `finally` is backed by a `process.on("exit")` flush, since the TTY + * renderer answers a thrown run with `process.exit(1)` rather than by + * rejecting; there they land below the `Error:` line it prints first. A run + * killed by a signal loses them — `exit` does not fire. + */ +export async function withDeferredDiagnostics(body: () => Promise): Promise { + if (depth === 0) { + buffered = []; + bufferedTail = []; + dropped = 0; + process.on("exit", flushDiagnostics); + } + depth += 1; + try { + return await body(); + } finally { + // Clamped, since `resetDiagnosticsForTesting` zeroes the depth from inside + // an open scope. + depth = Math.max(0, depth - 1); + if (depth === 0) { + process.removeListener("exit", flushDiagnostics); + flushDiagnostics(); + } + } +} + +/** Prints and clears whatever the open scope collected. */ +function flushDiagnostics(): void { + const head = buffered ?? []; + const tail = bufferedTail; + const suppressed = dropped; + buffered = null; + bufferedTail = []; + dropped = 0; + for (const message of head) emit(message); + if (suppressed > 0) { + emit( + `... ${suppressed} further diagnostic(s) from this run are not shown — the buffer holds ` + + `${MAX_BUFFERED}, the first ${HEAD_BUFFERED} and the last ${TAIL_BUFFERED}. A run raising ` + + `this many is reporting a systematic problem with the data rather than a handful of ` + + `rows, so read the lines above as a sample and the lines below as the outcome` + ); + } + for (const message of tail) emit(message); +} + +/** + * Routes every line that would reach the console into `lines` instead, until + * `restore()`. Test only. + * + * A test that wants to know what a run told the operator used to spy on + * `console.warn`, which pins it to the sink rather than to the seam: the same + * spy also catches a stray console write from anything else on the path, and + * its restore is one more thing for the `finally` to get right. This observes + * exactly what {@link reportDiagnostic} lets through — the immediate + * fall-through outside a scope, and the flush at the end of one, overrun line + * included. Captures nest; each `restore()` reinstates the sink it replaced. + */ +export function captureDiagnosticsForTesting(): { + readonly lines: readonly string[]; + restore(): void; +} { + const previous = captured; + const lines: string[] = []; + captured = lines; + return { + lines, + restore(): void { + captured = previous; + }, + }; +} + +/** Drops any open scope and any capture. Test setup only. */ +export function resetDiagnosticsForTesting(): void { + buffered = null; + bufferedTail = []; + dropped = 0; + depth = 0; + captured = null; + process.removeListener("exit", flushDiagnostics); +} From 5b0d2e1b688bc7c4b45c39f289e0776a85cab3da Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:25:33 +0000 Subject: [PATCH 4/9] fix(xbrl): scale numeric facts exactly, and never read empty text as 0 normalizeFactValue multiplied Number(text) by 10 ** scale, which rounds twice: 0.07 at scale 2 became 7.000000000000001, and across two-decimal values at the scales filings use about one in ten missed by an ulp. Those land in xbrl_facts.value_numeric, which is compared exactly. The scale is now folded into the literal's exponent and parsed once, so the result is Number(text + "e" + scale). Number() also read "" and whitespace as 0 (and 0x10 as 16). Numeric text must now be a decimal literal, optionally with an exponent; anything else is null. Fixes #394 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- src/sec/xbrl/parseXbrlCore.test.ts | 94 ++++++++++++++++++++++++++++++ src/sec/xbrl/parseXbrlCore.ts | 22 +++++-- 2 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 src/sec/xbrl/parseXbrlCore.test.ts diff --git a/src/sec/xbrl/parseXbrlCore.test.ts b/src/sec/xbrl/parseXbrlCore.test.ts new file mode 100644 index 00000000..1f14e195 --- /dev/null +++ b/src/sec/xbrl/parseXbrlCore.test.ts @@ -0,0 +1,94 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, expect, it } from "vitest"; +import { normalizeFactValue } from "./parseXbrlCore"; + +function numeric(rawText: string, scale: number | null, sign: "-" | null = null): number | null { + return normalizeFactValue({ + rawText, + format: null, + scale, + sign, + isNumeric: true, + isNil: false, + }).numericValue; +} + +/** + * `value_numeric` is compared exactly — equality joins, de-duplication against + * `company_facts` — so a scaled fact must be the decimal it states, not that + * decimal times a power of ten rounded twice. + */ +describe("normalizeFactValue scales without float error", () => { + it.each([ + ["0.07", 2, 7], + ["0.07", -2, 0.0007], + ["0.14", 2, 14], + ["0.29", 2, 29], + ["0.13", -2, 0.0013], + ["0.57", -2, 0.0057], + ["0.28", 2, 28], + ])("%s at scale %d is exactly %d", (text, scale, expected) => { + expect(numeric(text, scale)).toBe(expected); + }); + + it("equals the decimal literal with the scale as its exponent, across two-decimal values", () => { + // 0.00 through 714.29 at the scales filings use. Multiplying by 10 ** scale + // missed on about one in ten of these. + const misses: string[] = []; + for (let cents = 0; cents < 71_430; cents += 1) { + const text = (cents / 100).toFixed(2); + for (const scale of [-2, 2, 3, 6, 9]) { + const got = numeric(text, scale); + const exact = Number(`${text}e${scale}`); + if (got !== exact && misses.length < 10) misses.push(`${text} e${scale}: ${got}`); + } + } + expect(misses).toEqual([]); + }); + + it("applies the sign after scaling", () => { + expect(numeric("1234", 3, "-")).toBe(-1_234_000); + expect(numeric("0.07", 2, "-")).toBe(-7); + }); + + it("adds the scale to an exponent the text already carries", () => { + expect(numeric("1.5e3", 3)).toBe(1_500_000); + expect(numeric("7E-2", 2)).toBe(7); + }); +}); + +describe("normalizeFactValue refuses numeric text that is not a number", () => { + // `Number()` reads every one of these as a finite number: "" and whitespace + // as 0, "0x10" as 16. A numeric fact with no content is null, never zero. + it.each(["", " ", " ", "0x10", "Infinity", "1,234", "12 34"])("%j is null", (text) => { + expect(numeric(text, null)).toBeNull(); + expect(numeric(text, 3)).toBeNull(); + }); + + it("keeps a nil fact null and a non-numeric fact's text", () => { + expect( + normalizeFactValue({ + rawText: "", + format: null, + scale: null, + sign: null, + isNumeric: true, + isNil: true, + }) + ).toEqual({ value: "", numericValue: null }); + expect( + normalizeFactValue({ + rawText: " Acme Corp ", + format: null, + scale: null, + sign: null, + isNumeric: false, + isNil: false, + }) + ).toEqual({ value: "Acme Corp", numericValue: null }); + }); +}); diff --git a/src/sec/xbrl/parseXbrlCore.ts b/src/sec/xbrl/parseXbrlCore.ts index bc89598e..d78382d8 100644 --- a/src/sec/xbrl/parseXbrlCore.ts +++ b/src/sec/xbrl/parseXbrlCore.ts @@ -163,10 +163,22 @@ export interface NormalizedFactValue { readonly numericValue: number | null; } +/** + * A decimal literal with an optional exponent. `Number()` alone accepts more — + * `""` and whitespace as 0, `0x10` as 16, `Infinity` — none of which is a + * numeric fact's value. + */ +const DECIMAL_LITERAL = /^([+-]?(?:\d+(?:\.\d*)?|\.\d+))(?:[eE]([+-]?\d+))?$/; + /** * Applies the ixt format transform, then (for numeric facts) sign and scale. * An unregistered transform keeps the raw text and yields a null numericValue - * rather than guessing. + * rather than guessing, and so does numeric text that is not a decimal literal. + * + * The scale is folded into the literal's exponent and the result parsed once, + * rather than multiplying by `10 ** scale`: `Number("0.07e2")` is exactly 7, + * while `0.07 * 100` is `7.000000000000001`, and that product was off by an + * ulp for roughly one scaled two-decimal value in eleven. */ export function normalizeFactValue(args: { readonly rawText: string; @@ -185,8 +197,10 @@ export function normalizeFactValue(args: { if (!isNumeric) return { value, numericValue: null }; if (transformed === null && format !== null) return { value, numericValue: null }; - const base = Number(value); - if (!Number.isFinite(base)) return { value, numericValue: null }; - const scaled = base * Math.pow(10, scale ?? 0); + const literal = DECIMAL_LITERAL.exec(value); + if (literal === null) return { value, numericValue: null }; + const exponent = Number(literal[2] ?? 0) + (scale ?? 0); + const scaled = Number(`${literal[1]}e${exponent}`); + if (!Number.isFinite(scaled)) return { value, numericValue: null }; return { value, numericValue: sign === "-" ? -scaled : scaled }; } From 00e90dd3ee4813e299b044f5b895192fc13bf6ee Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:28:50 +0000 Subject: [PATCH 5/9] fix: refuse an unsafe primary_doc before it becomes a sec.gov URL primary_doc is filer-written. The bootstrap and cache-read paths ran it through sanitizePrimaryDoc, but the two fetch-on-miss callers - the conversion sweep and `sec read --fetch` - passed it straight to SecFetchAccessionDocTask, which spliced it into the request URL. A name with ../, ? or # reached a different sec.gov path, and the request was spent against the rate budget before the cache write's guard ran. - sanitizePrimaryDoc also refuses ?, # and % (a query, a fragment, an escape a server may decode) and control characters; safePrimaryDocName is its predicate form. - SecFetchAccessionDocTask validates fileName in inputToUrl and inputToFileName and percent-encodes the segment. The constructor builds the URL, so a refused name throws a TaskError before anything is queued. - conversionCandidates drops an unsafe name and conversionFetchFileName returns undefined for it; the task reports the filing and fetches nothing. A form read as the whole submission still fetches its .txt, which never uses the name, and a filing naming no primary document still falls back to it. - loadFilingHtml refuses an unsafe name with a message saying so. Fixes #395 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- .../document/ConvertFilingDocumentTask.ts | 28 +++++-- src/task/document/unsafePrimaryDoc.test.ts | 84 +++++++++++++++++++ .../fetch/SecFetchAccessionDocTask.test.ts | 53 +++++++++++- src/task/fetch/SecFetchAccessionDocTask.ts | 30 ++++++- src/util/accessionDocPath.test.ts | 26 ++++++ src/util/accessionDocPath.ts | 38 ++++++--- src/verify/loadFilingHtml.test.ts | 67 +++++++++++++++ src/verify/loadFilingHtml.ts | 16 +++- 8 files changed, 321 insertions(+), 21 deletions(-) create mode 100644 src/task/document/unsafePrimaryDoc.test.ts create mode 100644 src/verify/loadFilingHtml.test.ts diff --git a/src/task/document/ConvertFilingDocumentTask.ts b/src/task/document/ConvertFilingDocumentTask.ts index 15825c61..8c1b5f21 100644 --- a/src/task/document/ConvertFilingDocumentTask.ts +++ b/src/task/document/ConvertFilingDocumentTask.ts @@ -15,8 +15,12 @@ import { FILING_SECTION_REPOSITORY_TOKEN, type FilingSection, } from "../../storage/document/FilingSectionSchema"; -import { cachedAccessionDocPath, resolvePrimaryDocName } from "../../util/accessionDocPath"; -import { reportDryRun } from "../../util/diagnostics"; +import { + cachedAccessionDocPath, + resolvePrimaryDocName, + safePrimaryDocName, +} from "../../util/accessionDocPath"; +import { reportDiagnostic, reportDryRun } from "../../util/diagnostics"; import { convertFilingSubmission, FILING_CONVERTER_VERSION, @@ -98,7 +102,8 @@ export function conversionCandidates( accessionNumber: string, primaryDoc: string | null | undefined ): string[] { - const primary = resolvePrimaryDocName(primaryDoc); + const resolved = resolvePrimaryDocName(primaryDoc); + const primary = resolved === undefined ? undefined : safePrimaryDocName(resolved); return [fullSubmissionFileName(accessionNumber), primary].filter( (n): n is string => n !== undefined ); @@ -119,6 +124,11 @@ export function conversionCandidates( * family and Reg A annual reports are all full-submission forms. A proxy * converts from its primary document and shows one document, which is what it * has. + * + * `undefined` when the filing's primary document is the file to fetch and its + * filer-written name is unsafe (see `sanitizePrimaryDoc`): nothing is fetched + * rather than a request spliced from it. A filing naming no primary document + * at all still falls back to the whole submission. */ export function conversionFetchFileName( form: string | null | undefined, @@ -128,7 +138,9 @@ export function conversionFetchFileName( if (form !== null && form !== undefined && submissionFetchKind(form) === "full-submission") { return fullSubmissionFileName(accessionNumber); } - return resolvePrimaryDocName(primaryDoc) ?? fullSubmissionFileName(accessionNumber); + const primary = resolvePrimaryDocName(primaryDoc); + if (primary === undefined) return fullSubmissionFileName(accessionNumber); + return safePrimaryDocName(primary); } /** The primary document's filename, falling back to the file that was loaded. */ @@ -226,7 +238,13 @@ export class ConvertFilingDocumentTask extends Task< input.accessionNumber, input.primaryDoc ?? null ); - if (fileName === undefined) return undefined; + if (fileName === undefined) { + reportDiagnostic( + `Refusing unsafe primary_doc for ${input.cik}/${input.accessionNumber}: ` + + `${JSON.stringify(input.primaryDoc)} — not fetched` + ); + return undefined; + } const fetchTask = context.own( new SecFetchAccessionDocTask( { cik: input.cik, accessionNumber: input.accessionNumber, fileName }, diff --git a/src/task/document/unsafePrimaryDoc.test.ts b/src/task/document/unsafePrimaryDoc.test.ts new file mode 100644 index 00000000..75f8f831 --- /dev/null +++ b/src/task/document/unsafePrimaryDoc.test.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * `primary_doc` is copied from the filer's submissions JSON. The conversion + * sweep fetches it on a cache miss, so a name that is not one file in the + * accession folder must stop there: no request, and a line saying why. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { IExecuteContext } from "workglow"; +import { resetDependencyInjectionsForTesting } from "../../config/TestingDI"; +import { captureDiagnosticsForTesting, resetDiagnosticsForTesting } from "../../util/diagnostics"; +import { + ConvertFilingDocumentTask, + type ConvertFilingDocumentTaskInput, + conversionCandidates, + conversionFetchFileName, +} from "./ConvertFilingDocumentTask"; + +const ACCESSION = "0001213900-26-000001"; +const UNSAFE = ["../../x", "a?b", "a#b"] as const; + +describe("conversionCandidates and conversionFetchFileName with an unsafe primary_doc", () => { + it.each(UNSAFE)("%j is never probed", (primaryDoc) => { + expect(conversionCandidates(ACCESSION, primaryDoc)).toEqual([`${ACCESSION}.txt`]); + }); + + it.each(UNSAFE)("%j is never fetched as a primary document", (primaryDoc) => { + expect(conversionFetchFileName("DEF 14A", ACCESSION, primaryDoc)).toBeUndefined(); + }); + + it.each(UNSAFE)( + "%j does not stop a form fetched as the whole submission, which never uses it", + (primaryDoc) => { + expect(conversionFetchFileName("8-K", ACCESSION, primaryDoc)).toBe(`${ACCESSION}.txt`); + } + ); + + it("still falls back to the whole submission when no primary document is named", () => { + expect(conversionFetchFileName("DEF 14A", ACCESSION, null)).toBe(`${ACCESSION}.txt`); + }); +}); + +/** Exposes the source loader, the seam between the cache, the network and the parser. */ +class LoadOnly extends ConvertFilingDocumentTask { + load(input: ConvertFilingDocumentTaskInput, context: IExecuteContext) { + return this.loadSource(input, context); + } +} + +describe("ConvertFilingDocumentTask with an unsafe primary_doc", () => { + let capture: ReturnType; + + beforeEach(() => { + resetDependencyInjectionsForTesting(); + resetDiagnosticsForTesting(); + capture = captureDiagnosticsForTesting(); + }); + + afterEach(() => { + capture.restore(); + resetDiagnosticsForTesting(); + }); + + it.each(UNSAFE)("%j makes no request and says why", async (primaryDoc) => { + const own = vi.fn(); + const context = { own, disown: vi.fn() } as unknown as IExecuteContext; + + const source = await new LoadOnly().load( + { cik: 1811882, accessionNumber: ACCESSION, form: "DEF 14A", primaryDoc }, + context + ); + + expect(source).toBeUndefined(); + // Every fetch goes through `context.own(new SecFetchAccessionDocTask(...))`. + expect(own).not.toHaveBeenCalled(); + expect(capture.lines).toEqual([ + `Refusing unsafe primary_doc for 1811882/${ACCESSION}: ${JSON.stringify(primaryDoc)} — not fetched`, + ]); + }); +}); diff --git a/src/task/fetch/SecFetchAccessionDocTask.test.ts b/src/task/fetch/SecFetchAccessionDocTask.test.ts index 5f5f0a52..9b43b071 100644 --- a/src/task/fetch/SecFetchAccessionDocTask.test.ts +++ b/src/task/fetch/SecFetchAccessionDocTask.test.ts @@ -5,7 +5,7 @@ */ import { describe, expect, it } from "vitest"; -import type { FetchUrlTaskInput, IExecuteContext } from "workglow"; +import { type FetchUrlTaskInput, type IExecuteContext, TaskError } from "workglow"; import { SecFetchAccessionDocTask } from "./SecFetchAccessionDocTask"; describe("SecFetchAccessionDocTask", () => { @@ -72,4 +72,55 @@ describe("SecFetchAccessionDocTask", () => { ); expect(resolved.response_type).toBe("stream"); }); + + /** + * The name is filer-written and becomes a URL path segment here. Refusing it + * in the constructor, which builds the URL, means a caller that skipped the + * check still queues no request. + */ + it.each(["../../x", "a?b", "a#b", "sub/doc.htm", "%2e%2e%2fx", ""])( + "refuses %j before any request exists", + (fileName) => { + const build = () => + new SecFetchAccessionDocTask({ + cik: 2097288, + accessionNumber: "0001213900-26-012345", + fileName, + }); + expect(build).toThrow(TaskError); + expect(build).toThrow(/Refusing unsafe document name for 2097288\/0001213900-26-012345/); + } + ); + + it("refuses the same names from inputToUrl and inputToFileName", () => { + const task = new SecFetchAccessionDocTask({ + cik: 2097288, + accessionNumber: "0001213900-26-012345", + fileName: "ea123.htm", + }); + for (const fileName of ["../../x", "a?b", "a#b"]) { + const input = { cik: 2097288, accessionNumber: "0001213900-26-012345", fileName }; + expect(() => task.inputToUrl(input)).toThrow(TaskError); + expect(() => task.inputToFileName(input)).toThrow(TaskError); + } + }); + + it("keeps a safe name's URL and cache path exactly as before", () => { + const task = new SecFetchAccessionDocTask({ + cik: 2097288, + accessionNumber: "0001213900-26-012345", + fileName: "tm2412345-1_s1.htm", + }); + const input = { + cik: 2097288, + accessionNumber: "0001213900-26-012345", + fileName: "tm2412345-1_s1.htm", + }; + expect(task.inputToUrl(input)).toBe( + "https://www.sec.gov/Archives/edgar/data/0002097288/000121390026012345/tm2412345-1_s1.htm" + ); + expect(task.inputToFileName(input)).toBe( + "accessiondocs/0002097288/000121390026012345-tm2412345-1_s1.htm" + ); + }); }); diff --git a/src/task/fetch/SecFetchAccessionDocTask.ts b/src/task/fetch/SecFetchAccessionDocTask.ts index 899702a0..9d958e58 100644 --- a/src/task/fetch/SecFetchAccessionDocTask.ts +++ b/src/task/fetch/SecFetchAccessionDocTask.ts @@ -5,8 +5,9 @@ */ import { Type } from "typebox"; -import { DataPortSchemaObject, TaskInput } from "workglow"; +import { DataPortSchemaObject, TaskError, TaskInput } from "workglow"; import { TypeSecCik } from "../../sec/submissions/EnititySubmissionSchema"; +import { safePrimaryDocName } from "../../util/accessionDocPath"; import { SecCachedFetchTask } from "./SecCachedFetchTask"; // NOTE: accession documents are immutable, so we don't need to pass in a date for invalidation @@ -44,11 +45,34 @@ export class SecFetchAccessionDocTask extends SecCachedFetchTask { expect(sanitizePrimaryDoc("wf-form4.xml")).toBe("wf-form4.xml"); expect(sanitizePrimaryDoc(" wf-form4.xml ")).toBe("wf-form4.xml"); }); + + // The name also becomes a URL path segment, where these reach another path + // than the file they name: a query, a fragment, or an escape the server may + // decode into a separator. + it.each(["a?b", "a#b", "d10k.htm?x=1", "%2e%2e%2fx", "a%2Fb", "a\tb", "a\u007fb"])( + "rejects %j, which would not survive as one URL segment", + (name) => { + expect(() => sanitizePrimaryDoc(name)).toThrow(/Refusing unsafe primary_doc/); + } + ); + + it.each(["d123456d10k.htm", "tm2412345-1_s1.htm", "primary_doc.xml", "0001-26-1.txt"])( + "keeps the names EDGAR actually serves, %j among them", + (name) => { + expect(sanitizePrimaryDoc(name)).toBe(name); + } + ); +}); + +describe("safePrimaryDocName", () => { + it("is the sanitized name, or undefined where sanitizePrimaryDoc throws", () => { + expect(safePrimaryDocName(" s1.htm ")).toBe("s1.htm"); + expect(safePrimaryDocName("../../x")).toBeUndefined(); + expect(safePrimaryDocName("a?b")).toBeUndefined(); + }); }); describe("cachedAccessionDocPath", () => { diff --git a/src/util/accessionDocPath.ts b/src/util/accessionDocPath.ts index 4fda8287..0a976067 100644 --- a/src/util/accessionDocPath.ts +++ b/src/util/accessionDocPath.ts @@ -42,11 +42,20 @@ export function resolvePrimaryDocName(primaryDoc: string | null | undefined): st return stripXslPrefix(raw); } +/** + * Characters a filename cannot carry into a URL path segment and still name the + * same file: `?` starts a query, `#` a fragment, and `%` is an escape a server + * may decode into `/` or `..`. EDGAR's own filenames never contain them. + */ +// oxlint-disable-next-line no-control-regex -- control characters are what it refuses +const URL_UNSAFE = /[?#%\u0000-\u001f\u007f]/; + /** * Validates a filer-authored primary-document filename before it is used to - * compose an on-disk cache path. Rejects anything that could escape the - * accession-doc directory (path separators, parent-directory refs, absolute - * paths, NUL bytes) and returns the trimmed basename. + * compose an on-disk cache path or a sec.gov URL. Rejects anything that could + * escape the accession-doc directory (path separators, parent-directory refs, + * absolute paths, NUL bytes) or reach a different URL than the file it names + * ({@link URL_UNSAFE}), and returns the trimmed basename. */ export function sanitizePrimaryDoc(name: string): string { const trimmed = name.trim(); @@ -59,13 +68,26 @@ export function sanitizePrimaryDoc(name: string): string { trimmed.includes("/") || trimmed.includes("\\") || trimmed.includes("\0") || - trimmed.startsWith("/") + trimmed.startsWith("/") || + URL_UNSAFE.test(trimmed) ) { throw new Error(`Refusing unsafe primary_doc name: ${JSON.stringify(name)}`); } return trimmed; } +/** + * {@link sanitizePrimaryDoc} as a predicate: the safe name, or `undefined` + * where it would throw. + */ +export function safePrimaryDocName(name: string): string | undefined { + try { + return sanitizePrimaryDoc(name); + } catch { + return undefined; + } +} + /** * Confirms that a composed path resolves inside a trusted directory. Throws * when the resolved path escapes the base directory, naming both the raw and @@ -92,12 +114,8 @@ export function cachedAccessionDocPath( accessionNumber: string, fileName: string ): string | undefined { - let safeName: string; - try { - safeName = sanitizePrimaryDoc(fileName); - } catch { - return undefined; - } + const safeName = safePrimaryDocName(fileName); + if (safeName === undefined) return undefined; const cikDir = path.join(root, "accessiondocs", String(cik).padStart(10, "0")); const rel = `accessiondocs/${String(cik).padStart(10, "0")}/${accessionNumber.replaceAll( "-", diff --git a/src/verify/loadFilingHtml.test.ts b/src/verify/loadFilingHtml.test.ts new file mode 100644 index 00000000..94d7c74d --- /dev/null +++ b/src/verify/loadFilingHtml.test.ts @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { IExecuteContext } from "workglow"; +import { resetDependencyInjectionsForTesting } from "../config/TestingDI"; +import { EntityRepo } from "../storage/entity/EntityRepo"; +import type { Filing } from "../storage/filing/FilingSchema"; +import { loadFilingHtml } from "./loadFilingHtml"; + +const CIK = 1811882; +const ACCESSION = "0001213900-26-000001"; + +function recordFiling(primary_doc: string, form: string): void { + vi.spyOn(EntityRepo.prototype, "getFiling").mockResolvedValue({ + cik: CIK, + accession_number: ACCESSION, + form, + primary_doc, + } as unknown as Filing); +} + +/** + * `sec read --fetch` reaches EDGAR for a filing named only by CIK and + * accession, using the `primary_doc` the filer recorded. A name that is not + * one file in the accession folder is refused before any request. + */ +describe("loadFilingHtml with an unsafe primary_doc", () => { + beforeEach(() => { + resetDependencyInjectionsForTesting(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each(["../../x", "a?b", "a#b"])("refuses %j and fetches nothing", async (primaryDoc) => { + recordFiling(primaryDoc, "DEF 14A"); + const own = vi.fn(); + const context = { own, disown: vi.fn() } as unknown as IExecuteContext; + + await expect( + loadFilingHtml({ cik: CIK, accession: ACCESSION, allowFetch: true }, context) + ).rejects.toThrow(`Refusing unsafe primary_doc for ${CIK}/${ACCESSION}`); + expect(own).not.toHaveBeenCalled(); + }); + + it("still fetches the whole submission for a form read that way", async () => { + // An 8-K is read from its `.txt`; the unsafe name is never part of the URL. + recordFiling("a?b", "8-K"); + const run = vi.fn().mockResolvedValue({ text: "8-K" }); + const own = vi.fn((task: { defaults: { fileName: string } }) => { + expect(task.defaults.fileName).toBe(`${ACCESSION}.txt`); + return { run }; + }); + const context = { own, disown: vi.fn() } as unknown as IExecuteContext; + + const source = await loadFilingHtml( + { cik: CIK, accession: ACCESSION, allowFetch: true }, + context + ); + expect(source.html).toBe("8-K"); + expect(own).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/verify/loadFilingHtml.ts b/src/verify/loadFilingHtml.ts index 19840949..5708f83a 100644 --- a/src/verify/loadFilingHtml.ts +++ b/src/verify/loadFilingHtml.ts @@ -10,7 +10,11 @@ import { globalServiceRegistry, type IExecuteContext } from "workglow"; import { SEC_RAW_DATA_FOLDER } from "../config/tokens"; import { EntityRepo } from "../storage/entity/EntityRepo"; import { FILING_REPOSITORY_TOKEN } from "../storage/filing/FilingSchema"; -import { cachedAccessionDocPath, resolvePrimaryDocName } from "../util/accessionDocPath"; +import { + cachedAccessionDocPath, + resolvePrimaryDocName, + safePrimaryDocName, +} from "../util/accessionDocPath"; import { resolveAsset } from "../util/resolveAsset"; import { fullSubmissionFileName, @@ -82,7 +86,10 @@ export async function loadFilingHtml( ); } const filing = await new EntityRepo().getFiling(cik, accession); - const primaryDoc = resolvePrimaryDocName(filing?.primary_doc); + const recordedDoc = resolvePrimaryDocName(filing?.primary_doc); + // A filer-written name that is not one file in the accession folder is never + // probed or fetched; for a form read as the whole submission it is not needed. + const primaryDoc = recordedDoc === undefined ? undefined : safePrimaryDocName(recordedDoc); const form = filing?.form ?? null; // WHICH FILE is on disk is `submissionFetchKind`'s answer, not this module's. // The registration family, Reg A annual reports and every 8-K are cached as @@ -95,6 +102,11 @@ export async function loadFilingHtml( candidates.push(fullSubmissionFileName(accession)); } if (primaryDoc !== undefined) candidates.push(primaryDoc); + if (candidates.length === 0 && recordedDoc !== undefined) { + throw new Error( + `Refusing unsafe primary_doc for ${cik}/${accession}: ${JSON.stringify(filing?.primary_doc)}` + ); + } if (candidates.length === 0) { throw new Error( `No primary document recorded for ${cik}/${accession} — the filing is not in the local database, or it names none` From 787113fa0fde869370477335a0ca0350a41df058 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:33:33 +0000 Subject: [PATCH 6/9] fix(fixtures): fetch through the queue, with SecUserAgent, and never read a failure as empty The S-1 and golden fixture sources called fetch() directly with `process.env.SEC_USER_AGENT ?? `. That skipped the queue's rate and in-flight caps, the cluster limiter and the 429 cooldown, and the ?? kept an empty SEC_USER_AGENT, so EDGAR got no User-Agent at all. The S-1 feeds also swallowed errors with .catch(() => ""), so a blocked run reported no candidates. - edgarFixtureFetch.ts fetches text or bytes through SecFetchTask: queued, SecUserAgent (trimmed, with the compiled-in fallback), uncached, so the golden audit still sees live bytes. A failure or missing body throws. - The S-1 feeds no longer swallow errors; a filer skipped during enrichment is logged with the reason. The hand-rolled sleeps are gone, since the queue paces requests. - startSecJobQueue registers and starts the fetch queue. bootstrapSecRuntime calls it, and so does scripts/checkFixtures.ts, which has no CLI boot; without it FetchUrlTask would build a default queue without any of this. - rawFetch.test.ts fails on any bare fetch() under src/ or scripts/, and on any read of SEC_USER_AGENT outside Constants.ts. Fixes #393 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- scripts/checkFixtures.ts | 23 ++++-- src/config/bootstrapSecRuntime.ts | 16 +--- src/task/fetch/SecJobQueue.ts | 25 ++++++ src/task/fetch/rawFetch.test.ts | 65 +++++++++++++++ src/task/fixtures/edgarFixtureFetch.test.ts | 92 +++++++++++++++++++++ src/task/fixtures/edgarFixtureFetch.ts | 36 ++++++++ src/task/fixtures/goldenFixtureSource.ts | 29 ++----- src/task/fixtures/s1FixtureSource.ts | 27 +++--- 8 files changed, 256 insertions(+), 57 deletions(-) create mode 100644 src/task/fetch/rawFetch.test.ts create mode 100644 src/task/fixtures/edgarFixtureFetch.test.ts create mode 100644 src/task/fixtures/edgarFixtureFetch.ts diff --git a/scripts/checkFixtures.ts b/scripts/checkFixtures.ts index c21e2066..d4ea5c06 100644 --- a/scripts/checkFixtures.ts +++ b/scripts/checkFixtures.ts @@ -9,21 +9,32 @@ * Audits the committed EDGAR golden fixtures against their pinned manifest. * * A CI check on committed bytes, not something a user runs — which is why it is - * a script and not a command. It reaches EDGAR over a plain fetch and writes - * nothing, so it needs no configured database. + * a script and not a command. It writes nothing and needs no configured + * database, but it reaches EDGAR the way the pipeline does: through the fetch + * queue, started here because no CLI boot brings it up. Its rate limiter is + * in-process, since there is no database to share one through. * * bun run check-fixtures # verify, exit non-zero on any mismatch * bun run check-fixtures --write # re-download what no longer matches */ +import { shutdownCliResources } from "../src/cli/shutdownCliResources"; +import { startSecJobQueue } from "../src/task/fetch/SecJobQueue"; import { GoldenFixturesTask } from "../src/task/fixtures/GoldenFixturesTask"; const write = process.argv.includes("--write"); const force = process.argv.includes("--force"); -const result = await new GoldenFixturesTask().run({ - mode: write ? "download" : "verify", - force, -}); +await startSecJobQueue(); +let result; +try { + result = await new GoldenFixturesTask().run({ + mode: write ? "download" : "verify", + force, + }); +} finally { + // The queue's workers keep the process alive until they are stopped. + await shutdownCliResources(); +} for (const problem of result.problems) console.error(problem); console.log(`Done. ok=${result.ok} written=${result.written} failed=${result.failed}`); diff --git a/src/config/bootstrapSecRuntime.ts b/src/config/bootstrapSecRuntime.ts index 67d5709f..084f4676 100644 --- a/src/config/bootstrapSecRuntime.ts +++ b/src/config/bootstrapSecRuntime.ts @@ -4,8 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { getTaskQueueRegistry, Sqlite } from "workglow"; -import { getSecJobQueue } from "../task/fetch/SecJobQueue"; +import { Sqlite } from "workglow"; +import { startSecJobQueue } from "../task/fetch/SecJobQueue"; import { DefaultDI } from "./DefaultDI"; import { EnvToDI } from "./EnvToDI"; import { registerSecModels } from "./registerModels"; @@ -34,15 +34,5 @@ export async function bootstrapSecRuntime(): Promise { // Built lazily (after DI/env) so the Postgres-backed shared rate limiter can // read the pool + SEC_DB_TYPE from the registry. - const secJobQueue = await getSecJobQueue(); - getTaskQueueRegistry().registerQueue({ - server: secJobQueue.server, - client: secJobQueue.client, - storage: secJobQueue.storage, - }); - // Must await: otherwise a fast command can finish and stopQueues() while - // start() is still in fixupJobs(); stop() then completes before workers - // start, and start() resumes and leaves workers running — the process never - // exits (e.g. `sec db status`). - await secJobQueue.server.start(); + await startSecJobQueue(); } diff --git a/src/task/fetch/SecJobQueue.ts b/src/task/fetch/SecJobQueue.ts index 09215a27..c0802717 100644 --- a/src/task/fetch/SecJobQueue.ts +++ b/src/task/fetch/SecJobQueue.ts @@ -10,6 +10,7 @@ import { EvenlySpacedRateLimiter, FetchUrlTaskInput, FetchUrlTaskOutput, + getTaskQueueRegistry, globalServiceRegistry, InMemoryQueueStorage, InMemoryRateLimiterStorage, @@ -209,3 +210,27 @@ export async function getSecJobQueue(): Promise { handles = { server, client, storage }; return handles; } + +/** + * Registers the fetch queue (see {@link getSecJobQueue}) under its name and + * starts it, so every `SecFetchTask` — and every task built on it — runs + * through it rather than through a default queue `FetchUrlTask` would create + * on a miss, which has none of this queue's caps or 429 handling. + * + * `bootstrapSecRuntime` calls this; so does a script that fetches from EDGAR + * with no database to bring up. Awaited because a fast command can otherwise + * reach `stopQueues()` while `start()` is still in `fixupJobs()`: stop then + * completes before the workers start, they start anyway, and the process never + * exits. + */ +export async function startSecJobQueue(): Promise { + const secJobQueue = await getSecJobQueue(); + if (getTaskQueueRegistry().getQueue(SecJobQueueName) === undefined) { + getTaskQueueRegistry().registerQueue({ + server: secJobQueue.server, + client: secJobQueue.client, + storage: secJobQueue.storage, + }); + } + await secJobQueue.server.start(); +} diff --git a/src/task/fetch/rawFetch.test.ts b/src/task/fetch/rawFetch.test.ts new file mode 100644 index 00000000..e76c94b2 --- /dev/null +++ b/src/task/fetch/rawFetch.test.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { readdirSync, readFileSync } from "node:fs"; +import { join, relative } from "node:path"; +import { describe, expect, it } from "vitest"; + +const ROOT = join(import.meta.dirname, "../../.."); + +function sourceFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "mock_data" || entry.name === "node_modules") continue; + out.push(...sourceFiles(full)); + } else if (entry.name.endsWith(".ts") && !entry.name.endsWith(".test.ts")) { + out.push(full); + } + } + return out; +} + +/** Code lines only: a comment naming `fetch()` is not a call. */ +function codeLines(file: string): string[] { + return readFileSync(file, "utf8") + .split("\n") + .filter((line) => !/^\s*(\/\/|\/?\*)/.test(line)); +} + +function offenders(pattern: RegExp, allowed: readonly string[]): string[] { + const found: string[] = []; + for (const tree of ["src", "scripts"]) { + for (const file of sourceFiles(join(ROOT, tree))) { + const path = relative(ROOT, file); + if (allowed.includes(path)) continue; + codeLines(file).forEach((line) => { + if (pattern.test(line)) found.push(`${path}: ${line.trim()}`); + }); + } + } + return found; +} + +/** + * Every EDGAR request goes through the fetch queue — `SecFetchTask` and the + * tasks built on it — which is what applies the request rate shared across + * processes, the in-flight cap, the 429 cooldown and `SecUserAgent`. A bare + * `fetch()` gets none of them, and two fixture commands had one each, with a + * User-Agent default of their own that an empty `SEC_USER_AGENT` blanked. + */ +describe("EDGAR requests", () => { + it("never call fetch() directly", () => { + // `fetch(` and `globalThis.fetch(` — not a method that happens to be named + // so, and not prose ("to fetch (YYYY-MM-DD)"): the formatter never puts a + // space between a callee and its arguments. + expect(offenders(/(?:^|[^\w$.])fetch\(|globalThis\.fetch\(/, [])).toEqual([]); + }); + + it("read SEC_USER_AGENT in one place, which trims it and falls back", () => { + expect(offenders(/SEC_USER_AGENT/, ["src/config/Constants.ts"])).toEqual([]); + }); +}); diff --git a/src/task/fixtures/edgarFixtureFetch.test.ts b/src/task/fixtures/edgarFixtureFetch.test.ts new file mode 100644 index 00000000..16a1eb5c --- /dev/null +++ b/src/task/fixtures/edgarFixtureFetch.test.ts @@ -0,0 +1,92 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ +import { afterEach, describe, expect, it, vi } from "vitest"; + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + vi.resetModules(); +}); + +/** The User-Agent a fixture fetch puts on the wire, with `SEC_USER_AGENT` set to `value`. */ +async function userAgentSent(value: string | undefined): Promise { + vi.resetModules(); + if (value === undefined) vi.stubEnv("SEC_USER_AGENT", undefined); + else vi.stubEnv("SEC_USER_AGENT", value); + const { SecFetchTask } = await import("../fetch/SecFetchTask"); + const { fetchEdgarText } = await import("./edgarFixtureFetch"); + let sent: string | undefined; + vi.spyOn(SecFetchTask.prototype, "run").mockImplementation(async function (this: { + defaults: { headers?: Record }; + }) { + sent = this.defaults.headers?.["User-Agent"]; + return { text: "ok" }; + }); + await fetchEdgarText("https://www.sec.gov/x"); + return sent ?? ""; +} + +describe("fixture fetches identify themselves as the pipeline does", () => { + it("sends the compiled-in default when SEC_USER_AGENT is empty or blank", async () => { + // The fixture commands used `process.env.SEC_USER_AGENT ?? default`, which + // keeps an empty string — and EDGAR refuses a request with no User-Agent. + const unset = await userAgentSent(undefined); + expect(unset.length).toBeGreaterThan(0); + expect(await userAgentSent("")).toBe(unset); + expect(await userAgentSent(" ")).toBe(unset); + }); + + it("sends the configured value, trimmed", async () => { + expect(await userAgentSent(" Acme Research ops@acme.example ")).toBe( + "Acme Research ops@acme.example" + ); + }); +}); + +describe("a fixture fetch that fails", () => { + it("throws rather than returning an empty body", async () => { + const { SecFetchTask } = await import("../fetch/SecFetchTask"); + const { fetchEdgarText, fetchEdgarBytes } = await import("./edgarFixtureFetch"); + vi.spyOn(SecFetchTask.prototype, "run").mockRejectedValue(new Error("429 Too Many Requests")); + await expect(fetchEdgarText("https://www.sec.gov/x")).rejects.toThrow("429"); + await expect(fetchEdgarBytes("https://www.sec.gov/x")).rejects.toThrow("429"); + }); + + it("throws on a response with no body", async () => { + const { SecFetchTask } = await import("../fetch/SecFetchTask"); + const { fetchEdgarText } = await import("./edgarFixtureFetch"); + vi.spyOn(SecFetchTask.prototype, "run").mockResolvedValue({}); + await expect(fetchEdgarText("https://www.sec.gov/x")).rejects.toThrow(/no body/); + }); +}); + +describe("the S-1 candidate feeds", () => { + it("fail the listing when a feed fails, instead of reporting no candidates", async () => { + // A blocked run used to read as "Selected 0 of 0": each feed swallowed its + // error into an empty document. + const { SecFetchTask } = await import("../fetch/SecFetchTask"); + const { edgarS1Deps } = await import("./s1FixtureSource"); + vi.spyOn(SecFetchTask.prototype, "run").mockRejectedValue(new Error("429 Too Many Requests")); + await expect(edgarS1Deps(() => {}).listCandidates()).rejects.toThrow("429"); + }); + + it("names a filer it skips", async () => { + const { SecFetchTask } = await import("../fetch/SecFetchTask"); + const { edgarS1Deps } = await import("./s1FixtureSource"); + vi.spyOn(SecFetchTask.prototype, "run").mockImplementation(async function (this: { + defaults: { url: string }; + }) { + const { url } = this.defaults; + if (url.includes("SIC=6770")) return { text: "" }; + if (url.includes("getcurrent")) return { text: "" }; + throw new Error("404 Not Found"); + }); + const lines: string[] = []; + const candidates = await edgarS1Deps((m) => lines.push(m)).listCandidates(); + expect(candidates).toEqual([]); + expect(lines).toEqual(["Skipping CIK 0001234567: 404 Not Found"]); + }); +}); diff --git a/src/task/fixtures/edgarFixtureFetch.ts b/src/task/fixtures/edgarFixtureFetch.ts new file mode 100644 index 00000000..a6fb1bfb --- /dev/null +++ b/src/task/fixtures/edgarFixtureFetch.ts @@ -0,0 +1,36 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * How the fixture commands reach EDGAR: through {@link SecFetchTask}, the same + * queue the pipeline fetches through, so they are paced by both caps and the + * cluster limiter, retried on a 429 with its `Retry-After`, and identified by + * `SecUserAgent`. They used to call `fetch()` with a User-Agent default of + * their own, which an empty `SEC_USER_AGENT` turned into no User-Agent at all. + * + * Nothing here is cached: `SecFetchTask` carries no output cache, and the golden + * audit exists to see the bytes EDGAR serves now. A non-OK status the queue + * does not retry, or one it gave up on, is thrown, never read as an empty body. + */ +import { SecFetchTask } from "../fetch/SecFetchTask"; + +export async function fetchEdgarText(url: string): Promise { + const { text } = await new SecFetchTask( + { url, response_type: "text" }, + { title: `Fetch ${url}` } + ).run(); + if (typeof text !== "string") throw new Error(`EDGAR returned no body for ${url}`); + return text; +} + +export async function fetchEdgarBytes(url: string): Promise { + const { arraybuffer } = await new SecFetchTask( + { url, response_type: "arraybuffer" }, + { title: `Fetch ${url}` } + ).run(); + if (!(arraybuffer instanceof ArrayBuffer)) throw new Error(`EDGAR returned no body for ${url}`); + return new Uint8Array(arraybuffer); +} diff --git a/src/task/fixtures/goldenFixtureSource.ts b/src/task/fixtures/goldenFixtureSource.ts index 12819dfc..0530d453 100644 --- a/src/task/fixtures/goldenFixtureSource.ts +++ b/src/task/fixtures/goldenFixtureSource.ts @@ -7,34 +7,17 @@ /** * EDGAR-backed {@link GoldenFixtureDeps} for {@link runGoldenFixtures}. * - * Deliberately a plain `fetch` rather than the cached/queued `SecCachedFetchTask` - * path: this command audits the bytes EDGAR serves *right now*, so a cache layer - * between it and sec.gov would defeat the point. + * Uncached on purpose: this command audits the bytes EDGAR serves *right now*, + * so the cached `SecCachedFetchTask` path would defeat the point. It still goes + * through the fetch queue ({@link fetchEdgarBytes}), which paces it and retries + * a 429. */ +import { fetchEdgarBytes } from "./edgarFixtureFetch"; import type { GoldenFixtureDeps } from "./goldenFixtures"; -const SEC_UA = process.env.SEC_USER_AGENT ?? "workglow-sec research contact@workglow.dev"; - -/** EDGAR asks for <= 10 req/s; the corpus is tiny, so pace it well under that. */ -const REQUEST_SPACING_MS = 250; - -const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); - export function edgarGoldenFixtureDeps( log: (msg: string) => void = (m) => process.stderr.write(m + "\n") ): GoldenFixtureDeps { - let previous = Promise.resolve(); - return { - log, - fetchDoc: async (url: string) => { - // Serialize + space the requests: the manifest walk is sequential today, - // but rate limiting belongs with the transport rather than the caller. - previous = previous.then(() => sleep(REQUEST_SPACING_MS)); - await previous; - const res = await fetch(url, { headers: { "User-Agent": SEC_UA } }); - if (!res.ok) throw new Error(`${res.status} ${res.statusText} for ${url}`); - return new Uint8Array(await res.arrayBuffer()); - }, - }; + return { log, fetchDoc: fetchEdgarBytes }; } diff --git a/src/task/fixtures/s1FixtureSource.ts b/src/task/fixtures/s1FixtureSource.ts index d5774e98..a8448fbb 100644 --- a/src/task/fixtures/s1FixtureSource.ts +++ b/src/task/fixtures/s1FixtureSource.ts @@ -9,31 +9,27 @@ * from two feeds — recent S-1s (any issuer) and blank-check (SIC 6770) S-1 * filers so the SPAC floor is satisfiable — then enriches each via the * submissions API to obtain `sicCode` + the primary HTML document. + * + * Every request goes through the fetch queue ({@link fetchEdgarText}), which + * paces it and retries a 429. A feed that still fails is thrown rather than + * read as an empty one: a blocked run used to report "no candidates". */ +import { describeFailureReason } from "../../util/describeFailure"; +import { fetchEdgarText as getText } from "./edgarFixtureFetch"; import type { S1Candidate, S1FixtureDeps } from "./fetchS1Fixtures"; -const SEC_UA = process.env.SEC_USER_AGENT ?? "workglow-sec research contact@workglow.dev"; -const H = { "User-Agent": SEC_UA } as const; - -const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); const pad10 = (cik: string): string => cik.replace(/\D/g, "").padStart(10, "0"); -async function getText(url: string): Promise { - const r = await fetch(url, { headers: H }); - if (!r.ok) throw new Error(`${r.status} ${url}`); - return r.text(); -} - async function ciksBySic(sic: string, count: number): Promise { const url = `https://www.sec.gov/cgi-bin/browse-edgar?action=getcompany&SIC=${sic}&type=S-1&dateb=&owner=include&count=${count}&output=atom`; - const xml = await getText(url).catch(() => ""); + const xml = await getText(url); return [...xml.matchAll(/CIK=(\d+)/g)].map((m) => m[1]); } async function recentS1Ciks(count: number): Promise { const url = `https://www.sec.gov/cgi-bin/browse-edgar?action=getcurrent&type=S-1&count=${count}&output=atom`; - const xml = await getText(url).catch(() => ""); + const xml = await getText(url); return [...xml.matchAll(/edgar\/data\/(\d+)\//g)].map((m) => m[1]); } @@ -69,13 +65,14 @@ export function edgarS1Deps( const ciks = [...new Set([...spacCiks, ...recentCiks])]; const out: S1Candidate[] = []; for (const cik of ciks) { + // One filer that cannot be read is skipped, and said so: the sample + // only needs enough of them, but a skip is never silent. try { const c = await candidateFromCik(cik); if (c) out.push(c); - } catch { - /* skip unreachable/odd filers */ + } catch (err) { + log(`Skipping CIK ${cik}: ${describeFailureReason(err, 200)}`); } - await sleep(110); // stay well under EDGAR's 10 req/s } return out; }, From 212afbf04946a57ffeb1f192cd118393e9987a13 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:17:50 +0000 Subject: [PATCH 7/9] refactor!: remove the sec-base binary sec-base was the generic Workglow CLI (task, model, mcp, workflow, agent, web) carrying a curated list of sec's tasks. It booted sec's whole runtime before commander parsed, so --help and --version died unconfigured, and it was the one binary that consumed libs' runWorkglowCli and the one the smoke check could not cover. sec's own commands are the surface; the generic task runner over them earned none of that. Removed with it: the bin entry and its build scripts, dist/libs-cli.js from the required tarball contents, the registerSecTasks list (nothing else read it), and the plain format "cik" field widget, which only task run's schema-driven forms rendered. sec's commands use "sec:cik". BREAKING CHANGE: the sec-base binary is gone. Use the sec commands. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FdP6Nwc3vwqfC3vq8urgqT --- .claude/CLAUDE.md | 16 +++-- README.md | 16 ++--- package.json | 7 +-- scripts/checkPackedContents.ts | 4 +- scripts/smokeBinaries.ts | 2 +- src/config/bootstrapSecRuntime.ts | 8 +-- src/config/registerTasks.test.ts | 39 ------------ src/config/registerTasks.ts | 55 ----------------- src/libs-cli.ts | 71 ---------------------- src/task/adv/IngestAdvSnapshotTask.test.ts | 6 +- src/web/secFieldWidgets.ts | 4 -- 11 files changed, 23 insertions(+), 205 deletions(-) delete mode 100644 src/config/registerTasks.test.ts delete mode 100644 src/config/registerTasks.ts delete mode 100755 src/libs-cli.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index f83cc232..7045e5aa 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -34,7 +34,7 @@ independently, and a comment must explain the code in front of it. ```bash bun install -bun run build # bundles the two binaries +bun run build # bundles the binary bun run typecheck # bun check over src, scripts AND tests bun run test # vitest run bunx vitest run # what you normally want — the full suite is slow @@ -46,7 +46,7 @@ CI and `bun run release-checks` run one gate list, `GATES` in `scripts/releaseChecks.ts`: `format-check` → `lint` → `typecheck` → `build` → `prepack-check` → `smoke-bins` → `test`, cheapest first. Add a gate there, never as a workflow step of its own — `toolchainPins.test.ts` fails on a restated one. -`smoke-bins` starts each built binary unconfigured and requires `--help` and +`smoke-bins` starts the built `sec` unconfigured and requires `--help` and `--version` to exit 0. **Tests are vitest with `pool: "forks"` and `isolate: true`.** Run them with @@ -65,7 +65,7 @@ same version as a floor and `@types/bun` is the same version exactly; commit. `bun-latest.yml` runs the gates on the newest Bun weekly, gating nothing. There is one tsconfig covering src, scripts and tests. It emits nothing — the -binaries bundle — so there is no declaration build to keep tests out of. +binary bundles — so there is no declaration build to keep tests out of. ## The CLI @@ -118,12 +118,10 @@ that list. `TestingDI` binds every entry as in-memory; call ## Cross-cutting rules **`bootstrapSecRuntime` is the one path that brings up the runtime** — the SQLite -binding, DI, models, providers, the started fetch queue. Both binaries call it. A -second entrypoint booting another way drifts silently, with late and misleading -failures (a task resolving no model, a fetch with no rate limiter). - -**`registerSecTasks` is a curated list**, not every class under `src/task/`. A task -earns its place there by answering a question on its own. +binding, DI, models, providers, the started fetch queue. `sec` calls it from its +one `preAction` hook, and any new entrypoint must call it too: one booting another +way drifts silently, with late and misleading failures (a task resolving no +model, a fetch with no rate limiter). **`getDb()` is SQLite-only** and throws when `SEC_DB_TYPE` is not sqlite. Before that guard it would open a stray SQLite file under Postgres, and rows written diff --git a/README.md b/README.md index a8939bd1..341590a7 100644 --- a/README.md +++ b/README.md @@ -62,14 +62,6 @@ answer says which one wrote it. The old names still work: `sync`, `bootstrap`, `query`, `init` are aliases. -There is a second binary, `sec-base`, which is the generic Workglow CLI — `task`, -`model`, `mcp`, `workflow`, `agent`, `web` — with this repo's tasks registered: - -```sh -sec-base task list -sec-base task run QueryFilings --input-json '{"cik":320193}' -``` - ## What does the work Each step is one Workglow package doing its job, and the point of the example is @@ -130,11 +122,11 @@ Bun 1.4.3, read from `packageManager` in `package.json` — the one place the version is written, since `typecheck` is `bun check` and a new Bun can change its answer on an unchanged commit. -### The package is two binaries, not a library +### The package is a binary, not a library `package.json` carries `bin` and no `exports`, `main` or `types`. That is the -shape on purpose: the re-founding retired the library surface, both binaries -bundle their dependencies, and `import ... from "@workglow/sec"` is not something +shape on purpose: the re-founding retired the library surface, the binary +bundles its dependencies, and `import ... from "@workglow/sec"` is not something this package offers. A manifest with no import entry point normally reads as a field someone deleted by mistake, so it is worth stating here: the absence is the decision. @@ -142,7 +134,7 @@ the decision. ### Cutting a release `bun run release-checks` is the gate set (format, lint, typecheck, build, packed -contents, a start of each built binary with no configuration, tests) — the same +contents, a start of the built binary with no configuration, tests) — the same list CI runs on every push, from `scripts/releaseChecks.ts`. One script runs it and then bumps: diff --git a/package.json b/package.json index 21811464..3e875fb2 100644 --- a/package.json +++ b/package.json @@ -18,14 +18,12 @@ "smoke-bins": "bun ./scripts/smokeBinaries.ts", "dev": "concurrently -c 'auto' -n 'sec:' 'bun:dev-*'", "dev-sec": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/sec.js ./src/sec.ts", - "dev-sec-base": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/libs-cli.js ./src/libs-cli.ts", "dev-hft-worker": "bun build --watch --target=bun --sourcemap=external --packages=external --outfile ./dist/hftWorker.js ./src/config/hftWorker.ts", "dev-types": "tsc --noEmit --watch --preserveWatchOutput", "build": "bun run build-clean && bun run build-js", "build-clean": "rm -fr dist/*", "build-js": "bun run build-sec", - "build-sec": "bun build --target=bun --packages=external --outfile ./dist/sec.js ./src/sec.ts && bun ./scripts/shebang-bin.ts ./dist/sec.js && bun run build-sec-base", - "build-sec-base": "bun build --target=bun --packages=external --outfile ./dist/libs-cli.js ./src/libs-cli.ts && bun ./scripts/shebang-bin.ts ./dist/libs-cli.js && bun run build-hft-worker", + "build-sec": "bun build --target=bun --packages=external --outfile ./dist/sec.js ./src/sec.ts && bun ./scripts/shebang-bin.ts ./dist/sec.js && bun run build-hft-worker", "build-hft-worker": "bun build --target=bun --packages=external --outfile ./dist/hftWorker.js ./src/config/hftWorker.ts", "typecheck": "bun check", "use-source": "bun ./scripts/bunsrc.ts source", @@ -40,8 +38,7 @@ "lint-fix": "oxlint --fix src scripts" }, "bin": { - "sec": "./dist/sec.js", - "sec-base": "./dist/libs-cli.js" + "sec": "./dist/sec.js" }, "files": [ "dist", diff --git a/scripts/checkPackedContents.ts b/scripts/checkPackedContents.ts index f5cee883..5c5601bf 100644 --- a/scripts/checkPackedContents.ts +++ b/scripts/checkPackedContents.ts @@ -42,14 +42,14 @@ const FORBIDDEN_PREFIXES = ["src/"] as const; const FORBIDDEN_SUBSTRINGS = ["/mock_data/"] as const; /** - * Files the package cannot run without. The two `bin` entries are obvious; the + * Files the package cannot run without. The `bin` entry is obvious; the * worker is not, and is the reason this list exists — it is spawned by URL at * runtime rather than imported, so nothing in the build or the type system * notices when it stops being emitted, and the provider that needs it only * warns. `sec index` then fails on a published install while every check here * stays green. */ -const REQUIRED_PATHS = ["dist/sec.js", "dist/libs-cli.js", "dist/hftWorker.js"] as const; +const REQUIRED_PATHS = ["dist/sec.js", "dist/hftWorker.js"] as const; interface PackReport { readonly source: string; diff --git a/scripts/smokeBinaries.ts b/scripts/smokeBinaries.ts index 28f557a6..711adc5a 100644 --- a/scripts/smokeBinaries.ts +++ b/scripts/smokeBinaries.ts @@ -20,7 +20,7 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -/** Each must print its help and version unconfigured. */ +/** Each `bin` entry; each must print its help and version unconfigured. */ const BINARIES = ["dist/sec.js"] as const; const ARGS = [["--help"], ["--version"]] as const; diff --git a/src/config/bootstrapSecRuntime.ts b/src/config/bootstrapSecRuntime.ts index 084f4676..85074444 100644 --- a/src/config/bootstrapSecRuntime.ts +++ b/src/config/bootstrapSecRuntime.ts @@ -15,10 +15,10 @@ import { registerSecProviders } from "./registerProviders"; * Brings up everything a sec task needs before it runs: the SQLite binding, the * DI container, models, providers, and the started fetch queue. * - * The `sec` CLI reaches this through its `preAction` hook; a second entrypoint - * that boots sec's runtime some other way would drift from it silently — the - * failures are late and look like unrelated bugs (a task resolving no model, a - * fetch with no rate limiter), so both callers share this one path. + * The `sec` CLI reaches this through its `preAction` hook. Any other entrypoint + * must call this rather than boot the runtime its own way, which drifts from it + * silently — the failures are late and look like unrelated bugs (a task + * resolving no model, a fetch with no rate limiter). */ export async function bootstrapSecRuntime(): Promise { // Load the SQLite native binding only when a database may be opened. diff --git a/src/config/registerTasks.test.ts b/src/config/registerTasks.test.ts deleted file mode 100644 index 3e32bbff..00000000 --- a/src/config/registerTasks.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -/** - * @license - * Copyright 2026 Steven Roussey - * SPDX-License-Identifier: Apache-2.0 - */ -import { describe, expect, it } from "vitest"; -import { TaskRegistry } from "workglow"; -import { registerSecTasks, SEC_CLI_TASKS } from "./registerTasks"; - -/** - * `task list` renders one row per registered task as `type` + `description`, so - * a task registered without a description reads as a blank cell — the CLI's own - * tasks all carry one, and sec's looked broken beside them until this was - * enforced rather than remembered. - */ -describe("registerSecTasks", () => { - registerSecTasks(); - - // Iterate the curated list itself rather than filtering the registry by - // category: a category filter silently covers a subset and still passes. - const registered = SEC_CLI_TASKS; - - it("registers every curated task into the global registry", () => { - for (const task of registered) { - expect(TaskRegistry.all.get(task.type)).toBe(task); - } - }); - - it("is idempotent, so a second entrypoint booting twice cannot throw", () => { - expect(() => registerSecTasks()).not.toThrow(); - }); - - it("gives every registered task a non-empty title and description", () => { - const missing = registered - .filter((ctor) => !ctor.title || !ctor.description) - .map((ctor) => ctor.type); - expect(missing).toEqual([]); - }); -}); diff --git a/src/config/registerTasks.ts b/src/config/registerTasks.ts deleted file mode 100644 index 599ff2c3..00000000 --- a/src/config/registerTasks.ts +++ /dev/null @@ -1,55 +0,0 @@ -/** - * @license - * Copyright 2026 Steven Roussey - * SPDX-License-Identifier: Apache-2.0 - */ - -import { TaskRegistry } from "workglow"; -import { ListFormTypesTask } from "../task/query/ListFormTypesTask"; -import { QueryCiksTask } from "../task/query/QueryCiksTask"; -import { QueryEntitiesTask } from "../task/query/QueryEntitiesTask"; -import { IngestAdvSnapshotTask } from "../task/adv/IngestAdvSnapshotTask"; -import { AskTask } from "../task/kb/AskTask"; -import { IndexFilingSectionsTask } from "../task/kb/IndexFilingSectionsTask"; -import { QueryAdvisersTask } from "../task/query/QueryAdvisersTask"; -import { QueryFactsTask } from "../task/query/QueryFactsTask"; -import { QueryFilingsTask } from "../task/query/QueryFilingsTask"; -import { QueryXbrlTask } from "../task/query/QueryXbrlTask"; - -/** - * The sec tasks worth offering as standalone `task run` targets. - * - * Deliberately a curated list rather than every class under `src/task/`. Most - * of those are pipeline steps — bulk writers, version ceremonies, per-filing - * store handlers — that mean nothing invoked alone and would bury the readable - * entries in a `task list` of a hundred rows. These read the database and - * answer a question, which is what a task runner is for. - * - * {@link ParseFilingDocumentTask} is here for a narrower reason: it is the only - * way to see what a parser makes of a filing when nothing in this deployment - * extracts that form, and whoever is working on such a parser has to be able to - * reach it. It writes nothing and fetches nothing, so being reachable costs - * nothing either. - */ -export const SEC_CLI_TASKS = [ - ListFormTypesTask, - QueryCiksTask, - QueryEntitiesTask, - AskTask, - IndexFilingSectionsTask, - IngestAdvSnapshotTask, - QueryAdvisersTask, - QueryFactsTask, - QueryFilingsTask, - QueryXbrlTask, -] as const; - -/** - * Registers sec's tasks into the global {@link TaskRegistry}, which is what - * `task list` / `task run` and the web console enumerate. Idempotent. - */ -export function registerSecTasks(): void { - for (const task of SEC_CLI_TASKS) { - TaskRegistry.registerTask(task); - } -} diff --git a/src/libs-cli.ts b/src/libs-cli.ts deleted file mode 100755 index 9b806e74..00000000 --- a/src/libs-cli.ts +++ /dev/null @@ -1,71 +0,0 @@ -#!/usr/bin/env bun -/** - * @license - * Copyright 2026 Steven Roussey - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * `sec-base` — the Workglow CLI, carrying sec's tasks. - * - * The `sec` binary is the data pipeline: named commands over EDGAR. This is the - * other half of the same runtime — the generic Workglow surface (`task`, - * `model`, `mcp`, `workflow`, `agent`, `credential`, `web`) with sec's own - * tasks registered into the task registry, so `sec-base task run QueryFilings` - * and the web console list them alongside the built-in ones. - * - * The body is `runWorkglowCli` from `@workglow/cli`, not a copy of it: the - * boot sequence, the HuggingFace worker path and the command set stay owned by - * that package. - */ -import { runWorkglowCli } from "@workglow/cli"; -import { - installCliSignalTeardown, - shouldInstallCliSignalTeardown, -} from "./cli/installCliSignalTeardown"; -import { shutdownCliResources } from "./cli/shutdownCliResources"; -import { bootstrapSecRuntime } from "./config/bootstrapSecRuntime"; -import { registerSecTasks } from "./config/registerTasks"; -import { registerSecWebUi } from "./web/registerSecWebUi"; - -// The command's own failure, kept across the shutdown below. `sec.ts` does the -// same, and for the same two reasons: a `process.exit(0)` in the success path -// reports a failed command as a success — `sec-base task run` is scripted, and -// a caller that cannot see a non-zero status cannot see a failure at all — and -// a throw that escapes this block skips the cleanup entirely, leaving the fetch -// queue's workers running so the process never exits. -let primaryError: unknown; -try { - await runWorkglowCli({ - name: "sec-base", - description: "Workglow CLI carrying sec's tasks", - // sec's tasks read the database and fetch through the rate-limited queue, so - // the runtime comes up before any of them can be listed or run. - registerTasks: async () => { - await bootstrapSecRuntime(); - registerSecTasks(); - // The console's contributed UI. `task run` reads a task's input schema, and - // every sec CIK port carries `format: "cik"` — so the pickers and the - // operator rail apply here too, even though this binary carries none of - // sec's own commands for the panels to attach to. - registerSecWebUi(); - }, - registerCommands: (program) => { - program.hook("preAction", (_thisCommand, actionCommand) => { - if (!shouldInstallCliSignalTeardown(actionCommand.name())) return; - installCliSignalTeardown({ close: shutdownCliResources }); - }); - }, - exitOnComplete: false, - }); -} catch (err) { - primaryError = err; - process.exitCode = 1; -} finally { - // Mirror the `sec` CLI's shutdown: allSettled so a crashing step cannot mask - // another or skip it. runWorkglowCli is told not to exit so this can run. - await shutdownCliResources(); -} - -if (primaryError !== undefined) throw primaryError; -process.exit(process.exitCode ?? 0); diff --git a/src/task/adv/IngestAdvSnapshotTask.test.ts b/src/task/adv/IngestAdvSnapshotTask.test.ts index 84c37c82..2a8cd209 100644 --- a/src/task/adv/IngestAdvSnapshotTask.test.ts +++ b/src/task/adv/IngestAdvSnapshotTask.test.ts @@ -193,9 +193,9 @@ describe("IngestAdvSnapshotTask", () => { const outside = mkdtempSync(join(tmpdir(), "sec-adv-outside-")); writeFileSync(join(outside, "Private.csv"), "secret\nvalue\n"); try { - // The same shape `BootstrapDownloadTask` guards: the task is registered, - // so this input arrives from `sec-base task run`, the console form and - // the MCP tool surface, not only from the sync leaf. + // The same shape `BootstrapDownloadTask` guards: the folder is task + // input, and the task checks it itself rather than trusting the sync + // leaf that usually supplies it. await expect( new IngestAdvSnapshotTask().run({ snapshot: "2026-06", diff --git a/src/web/secFieldWidgets.ts b/src/web/secFieldWidgets.ts index 22680928..2dbfbea3 100644 --- a/src/web/secFieldWidgets.ts +++ b/src/web/secFieldWidgets.ts @@ -150,10 +150,6 @@ async function searchForms( export function registerSecFieldWidgets(): void { const source = "sec"; registerWebFieldWidget({ format: "sec:cik", source, search: searchCiks }); - // `TypeSecCik` stamps `format: "cik"` on every CIK port in every sec task - // schema, so registering the same picker there gives `task run` — and the - // whole `sec-base` surface — the search box for free, with no annotation. - registerWebFieldWidget({ format: "cik", source, search: searchCiks }); registerWebFieldWidget({ format: "sec:accession", source, search: searchAccessions }); registerWebFieldWidget({ format: "sec:form", source, search: searchForms }); } From ba97ffcccf2f316ff38e1e7fd1de265ed55117bc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:30:16 +0000 Subject: [PATCH 8/9] fix(fixtures): audit what EDGAR stores, not what its CDN adds `check-fixtures` reported 41 of 43 golden fixtures as remote-changed. The filings were byte-identical: sec.gov's CDN (Akamai Bot Manager; the response carries x-akamai-transformed and _abck / bm_sz cookies) now splices an empty sensor '; +const withInjection = (html: string): string => + html.replace(/<\/body>/i, (close) => `${INJECTED}${close}`); + function depsServing(map: Record): GoldenFixtureDeps { return { log: () => {}, @@ -91,6 +98,34 @@ describe("applyGoldenFixtureTransform", () => { }); }); +describe("stripCdnInjection", () => { + it("removes the CDN's script before , in either case, and nothing else", () => { + for (const html of [ + "

Prospectus

\n\n\n\n", + "

Prospectus

\n\n\n\n\n\n", + ]) { + expect(stripCdnInjection(enc(withInjection(html)))).toEqual(enc(html)); + } + }); + + it("keeps a document the CDN left alone byte for byte, non-UTF-8 bytes included", () => { + const latin1 = new Uint8Array([0x3c, 0x70, 0x3e, 0xe9, 0xa0, 0xff, 0x3c, 0x2f, 0x70, 0x3e]); + expect(stripCdnInjection(latin1)).toBe(latin1); + const withBytes = new Uint8Array([...latin1, ...enc(`${INJECTED}`)]); + expect(stripCdnInjection(withBytes)).toEqual(new Uint8Array([...latin1, ...enc("")])); + }); + + it("leaves a script that is not the CDN's: inline, elsewhere, or not before ", () => { + for (const html of [ + '', + `${INJECTED}

after

`, + '', + ]) { + expect(stripCdnInjection(enc(html))).toEqual(enc(html)); + } + }); +}); + describe("goldenFixtureUrl", () => { it("builds the EDGAR archive path", () => { expect(goldenFixtureUrl(verbatimEntry)).toBe( @@ -171,6 +206,35 @@ describe("runGoldenFixtures", () => { expect(() => readFileSync(join(root, "s1", verbatimEntry.file))).toThrow(); }); + it("matches a document the CDN injected its script into, in both modes", async () => { + // What every HTML fixture looked like once sec.gov's edge began rewriting + // responses: the filing byte-identical, one script added before . + const injectedDeps = () => + depsServing({ + [goldenFixtureUrl(verbatimEntry)]: enc(withInjection(BODY)), + [goldenFixtureUrl(wrappedEntry)]: enc(withInjection(WRAPPED)), + }); + + const downloaded = await runGoldenFixtures({ + mode: "download", + deps: injectedDeps(), + rootDir: root, + entries, + }); + expect(downloaded.written).toBe(2); + expect(downloaded.failed).toBe(0); + expect(readFileSync(join(root, "s1", verbatimEntry.file), "utf-8")).toBe(BODY); + + const verified = await runGoldenFixtures({ + mode: "verify", + deps: injectedDeps(), + rootDir: root, + entries, + }); + expect(verified.ok).toBe(2); + expect(verified.failed).toBe(0); + }); + it("reports a fetch failure as an error without aborting the remaining entries", async () => { const deps = depsServing({ [goldenFixtureUrl(verbatimEntry)]: new Error("403 Forbidden"), diff --git a/src/task/fixtures/goldenFixtures.ts b/src/task/fixtures/goldenFixtures.ts index 3946ff61..6ca185e8 100644 --- a/src/task/fixtures/goldenFixtures.ts +++ b/src/task/fixtures/goldenFixtures.ts @@ -39,6 +39,32 @@ export function sha256Hex(bytes: Uint8Array): string { return createHash("sha256").update(bytes).digest("hex"); } +/** + * The sensor script sec.gov's CDN (Akamai Bot Manager — the response carries + * `x-akamai-transformed` and `_abck` / `bm_sz` cookies) splices into an HTML + * response just before ``: an empty `