From 0b254325018aafd7dac54f776f0b1404f3c7cd39 Mon Sep 17 00:00:00 2001 From: Mike Geehan Date: Tue, 18 Aug 2026 20:48:00 -0500 Subject: [PATCH] =?UTF-8?q?fix(runtime):=20stpa=20actually=20runs=20on=20N?= =?UTF-8?q?ode=20=E2=80=94=20and=20CI=20can=20now=20tell?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #9 shipped "runs on Node or Bun" green and broken. Every subcommand that spawned a tool died on Node with `SyntaxError: Cannot use import statement outside a module`; only `stpa --help` worked, because it never spawns anything. The root cause is not the one it looks like. Node's module-syntax detection reads these .ts tools as ESM perfectly well in a clean directory — the bug does not reproduce there. It needs an ANCESTOR whose package.json declares {"type":"commonjs"}, because an explicit ancestor declaration beats detection. That is not a corner case: `~/.claude/package.json` declares commonjs and skills install into `~/.claude/skills/`, so the documented install path was precisely the configuration that broke. Anyone who followed the README got a toolkit where nothing but --help ran. The fix is a dependency-free package.json declaring {"type":"module"}, which shadows any ancestor and makes resolution explicit instead of a property of where the repo happens to sit. That declaration then makes the launcher and one tool genuinely ESM, so: - `stpa` moves from require() to import, and from __dirname to fileURLToPath(import.meta.url). fileURLToPath rather than import.meta.dirname so the launcher still covers the whole Node range its own RUNTIME block claims. - `Tools/UcaGrid.ts` had two inline require("node:fs") calls and no imports at all — it was implicitly CommonJS, which is why it alone survived. Now it imports at the top like every sibling. CI grew a Node lane on 22 and 24 that runs the pipeline end-to-end through the CLI, then does it again with the repo staged under a commonjs ancestor. That second step is the one that matters: a Node job in a clean checkout would have passed for the whole four weeks the toolkit was broken. And because a gate that cannot fail is decoration, a final step deletes package.json and asserts the break comes back. Also swept the usage strings: 28 `bun .ts` lines across 13 tools became `stpa `. They hardcoded a runtime the toolkit no longer requires and pointed people at files instead of the command. Closes #11 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 77 +++++++++++++++++++++++++++++++++++ README.md | 10 +++++ Tools/ComposeChains.ts | 2 +- Tools/ControlInventory.ts | 4 +- Tools/ControlStructureScan.ts | 4 +- Tools/DiscoveryGate.ts | 2 +- Tools/EvidenceGate.ts | 4 +- Tools/MergePlanes.ts | 6 +-- Tools/Prioritize.ts | 6 +-- Tools/RenderReport.ts | 4 +- Tools/RenderSummary.ts | 4 +- Tools/ReportLink.ts | 2 +- Tools/ScopeGate.ts | 4 +- Tools/UcaGrid.ts | 20 +++++---- Tools/VerifyGate.ts | 4 +- package.json | 6 +++ stpa | 13 ++++-- 17 files changed, 136 insertions(+), 36 deletions(-) create mode 100644 package.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f20bf2..ffb7c53 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,3 +43,80 @@ jobs: grep -q 'NOT INDEPENDENTLY REVIEWED' Examples/ledgerline/SUMMARY.html # and it must link the full analysis rather than stand in for it grep -q 'REPORT.html' Examples/ledgerline/SUMMARY.html + + # The runtime lane that was missing. PR #9 claimed "runs on Node or Bun" and + # shipped broken on Node for four weeks, because every job above installs Bun and + # invokes the tools as `bun Tools/X.ts` — nothing ever ran `./stpa ` past + # --help, and nothing ever ran Node at all. + # + # Note what the last step reproduces, because a clean checkout does NOT reproduce + # the bug: Node's module-syntax detection resolves these .ts tools as ESM just + # fine when nothing overrides it. The failure needs an ANCESTOR directory whose + # package.json says {"type":"commonjs"} — which is exactly where this repo gets + # installed, since `~/.claude/package.json` declares commonjs and skills clone + # into `~/.claude/skills/`. An ancestor declaration beats detection, so every + # import in Tools/ became a SyntaxError for anyone who installed it as a skill. + # A gate that only runs in a clean directory would have passed the whole time. + node: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node: ['22', '24'] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + + - name: CLI loads under Node + run: chmod +x stpa && ./stpa --help + + - name: Grid arithmetic under Node + run: | + node Tools/UcaGrid.ts init Fixtures/example-model.json -o /tmp/grid.json + node Tools/UcaGrid.ts status /tmp/grid.json + + - name: Full pipeline end-to-end through the CLI + run: | + # `stpa run` exits non-zero by design on the worked example (it carries no + # independent peer review), so the gate is the artifacts, not the exit code. + cp -R Examples/ledgerline /tmp/run && rm -f /tmp/run/*.html + ./stpa run /tmp/run 2>&1 | tee /tmp/runlog || true + ! grep -qE 'SyntaxError|ReferenceError|ERR_MODULE_NOT_FOUND' /tmp/runlog + test -s /tmp/run/REPORT.html + test -s /tmp/run/SUMMARY.html + + - name: Survives a CommonJS ancestor — the real install condition + run: | + set -e + # stage the repo under a parent that declares commonjs, as `~/.claude` does + mkdir -p /tmp/ancestor/skills + echo '{"name":"ancestor","type":"commonjs"}' > /tmp/ancestor/package.json + cp -R "$GITHUB_WORKSPACE" /tmp/ancestor/skills/stpa + cd /tmp/ancestor/skills/stpa + # our own package.json must shadow the ancestor for every tool + for f in Tools/*.ts; do + out=$(node "$f" --help 2>&1 || true) + case "$out" in + *"Cannot use import statement outside a module"*|*"require is not defined"*|*"ERR_MODULE_NOT_FOUND"*) + echo "::error file=$f::ancestor commonjs defeated module resolution"; echo "$out"; exit 1;; + esac + done + cp -R Examples/ledgerline /tmp/anc-run && rm -f /tmp/anc-run/*.html + ./stpa run /tmp/anc-run 2>&1 | tee /tmp/anc-log || true + ! grep -qE 'SyntaxError|ReferenceError|ERR_MODULE_NOT_FOUND' /tmp/anc-log + test -s /tmp/anc-run/REPORT.html + test -s /tmp/anc-run/SUMMARY.html + + - name: The gate can fail — remove package.json and the ancestor wins + run: | + set -e + # A gate that cannot fail is decoration. Prove this one bites. + cd /tmp/ancestor/skills/stpa && rm -f package.json + if node Tools/RenderReport.ts --help 2>&1 | grep -q "Cannot use import statement outside a module"; then + echo "ok — without package.json the ancestor forces CommonJS and the tools break, as expected" + else + echo "::error::negative control did not reproduce; the ancestor step above proves nothing" + exit 1 + fi diff --git a/README.md b/README.md index ab41a51..1eec2a6 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,16 @@ whichever runtime is executing it, so `./stpa` uses Node (≥22.6; types are str natively from 22.18, and the flag is passed automatically below that) and `bun stpa` uses Bun. There are no Bun-specific APIs anywhere in the toolkit. +The repo ships a dependency-free `package.json` whose only job is `"type": "module"`. +**Do not delete it.** Node's own module-syntax detection handles these files fine in +isolation — the declaration is there because an *ancestor* directory can override +detection, and the most common install location does exactly that: `~/.claude/package.json` +declares `{"type":"commonjs"}`, and skills clone into `~/.claude/skills/`. Under that +ancestor, and without this file, every `import` in `Tools/` is a `SyntaxError` and nothing +but `stpa --help` runs. CI reproduces that ancestor on Node 22 and 24 — and asserts the +check still fails when `package.json` is removed, because a gate that cannot fail is +decoration. + Everything is offline either way: no API keys, no network calls, no telemetry. Nothing is downloaded at install or at analysis time. diff --git a/Tools/ComposeChains.ts b/Tools/ComposeChains.ts index c2a0240..6c0d276 100644 --- a/Tools/ComposeChains.ts +++ b/Tools/ComposeChains.ts @@ -49,7 +49,7 @@ * id:tenant-key a tenant identifier used as a scoping or lookup key * * Usage: - * bun ComposeChains.ts [analysis-dir] [--check] [--max-depth N] + * stpa compose [analysis-dir] [--check] [--max-depth N] * * Reads remediation.json (+ grid.json for statements/labels) * Writes 07-chains.json, 07-chains.md diff --git a/Tools/ControlInventory.ts b/Tools/ControlInventory.ts index 16b7fe6..4540a91 100644 --- a/Tools/ControlInventory.ts +++ b/Tools/ControlInventory.ts @@ -27,7 +27,7 @@ * look, which is the actual failure mode. * * Usage: - * bun ControlInventory.ts [--warn-only] + * stpa controls [--warn-only] * * Reads candidates.json (from `stpa scan --json`), grid.json, remediation.json * Writes control-inventory.json @@ -43,7 +43,7 @@ if (argv.includes("--help") || argv.includes("-h")) { [ "ControlInventory.ts — cross-check absence claims against the guards the scan found", "", - "Usage: bun ControlInventory.ts [--warn-only]", + "Usage: stpa controls [--warn-only]", "", "Requires candidates.json in the analysis dir:", " stpa scan --depth deep --json > /candidates.json", diff --git a/Tools/ControlStructureScan.ts b/Tools/ControlStructureScan.ts index 144a39e..20604c8 100755 --- a/Tools/ControlStructureScan.ts +++ b/Tools/ControlStructureScan.ts @@ -25,7 +25,7 @@ * and unknown stacks still get the generic sweep. * * Usage: - * bun ControlStructureScan.ts [--json] [--max-per-category N] [--include-tests] + * stpa scan [--json] [--max-per-category N] [--include-tests] * * Default output is a human-readable report; --json emits the structured candidate * set that feeds the ModelControlStructure workflow. @@ -305,7 +305,7 @@ if (!root) { [ "ControlStructureScan.ts — candidate extraction for STPA Step 2", "", - "Usage: bun ControlStructureScan.ts [options]", + "Usage: stpa scan [options]", "", " --focus comma-separated; only patterns carrying these tags", " e.g. --focus authz,tenancy or --focus api", diff --git a/Tools/DiscoveryGate.ts b/Tools/DiscoveryGate.ts index 1b868db..4aa66a1 100644 --- a/Tools/DiscoveryGate.ts +++ b/Tools/DiscoveryGate.ts @@ -28,7 +28,7 @@ * registries, and dynamic execution primitives. * * Usage: - * bun DiscoveryGate.ts [analysis-dir] [--json] [--check] + * stpa discover [analysis-dir] [--json] [--check] * * Reads source, and /discovery.json if present * Writes /discovery.json (a template, when absent) diff --git a/Tools/EvidenceGate.ts b/Tools/EvidenceGate.ts index 709de45..2f856e4 100644 --- a/Tools/EvidenceGate.ts +++ b/Tools/EvidenceGate.ts @@ -35,7 +35,7 @@ * disagreement. * * Usage: - * bun EvidenceGate.ts [--warn-only] [--fix-numbers] + * stpa evidence [--warn-only] [--fix-numbers] * * Exit: 0 pass · 2 bad input · 9 unresolved trust root or a wrong number in prose */ @@ -51,7 +51,7 @@ if (argv.includes("--help") || argv.includes("-h")) { [ "EvidenceGate.ts — trust-root provenance + derived-number consistency", "", - "Usage: bun EvidenceGate.ts [--warn-only] [--fix-numbers]", + "Usage: stpa evidence [--warn-only] [--fix-numbers]", "", "Every processModels[].variables[] entry needs trustRoot ∈", " " + [...ROOTS].join(" | "), diff --git a/Tools/MergePlanes.ts b/Tools/MergePlanes.ts index a0ff0ee..f366274 100755 --- a/Tools/MergePlanes.ts +++ b/Tools/MergePlanes.ts @@ -28,8 +28,8 @@ * of what showed up. * * Usage: - * bun MergePlanes.ts --expect # merge + gate - * bun MergePlanes.ts --expect --check + * stpa merge --expect # merge + gate + * stpa merge --expect --check * * manifest.json — written BEFORE dispatch, from the control-action inventory: * { "planes": { "auth": { "file": "planes/auth.json", @@ -91,7 +91,7 @@ if (argv.includes("--help") || argv.includes("-h")) [ "MergePlanes.ts — reconciliation gate for parallel STPA analysis", "", - "Usage: bun MergePlanes.ts --expect [--check]", + "Usage: stpa merge --expect [--check]", "", "Refuses to merge until every expected plane file exists and every expected", "cell is present or explicitly declared incomplete. Exit 4 = gap detected.", diff --git a/Tools/Prioritize.ts b/Tools/Prioritize.ts index cf67957..5306782 100755 --- a/Tools/Prioritize.ts +++ b/Tools/Prioritize.ts @@ -23,8 +23,8 @@ * CVSS-comparable. It orders THIS analysis's findings for THIS team. * * Usage: - * bun Prioritize.ts [analysis-dir] # writes 06-remediation.{json,md} - * bun Prioritize.ts [dir] --check # exit 1 if any finding lacks remediation + * stpa plan [analysis-dir] # writes 06-remediation.{json,md} + * stpa plan [dir] --check # exit 1 if any finding lacks remediation */ import { readFileSync, writeFileSync, existsSync } from "node:fs"; @@ -98,7 +98,7 @@ function die(m: string, c = 1): never { const argv = process.argv.slice(2); if (argv.includes("--help") || argv.includes("-h")) - die("Usage: bun Prioritize.ts [analysis-dir] [--check]\n\nReads grid.json + remediation.json, writes 06-remediation.{json,md}.", 2); + die("Usage: stpa plan [analysis-dir] [--check]\n\nReads grid.json + remediation.json, writes 06-remediation.{json,md}.", 2); const dir = resolve(argv.find((a) => !a.startsWith("-")) ?? ".stpa"); const checkOnly = argv.includes("--check"); diff --git a/Tools/RenderReport.ts b/Tools/RenderReport.ts index 1a9e4c2..903eab7 100755 --- a/Tools/RenderReport.ts +++ b/Tools/RenderReport.ts @@ -15,7 +15,7 @@ * whose integrity the rest of the skill works to protect. * * Usage: - * bun RenderReport.ts [analysis-dir] [-o out.html] [--title "..."] + * stpa report [analysis-dir] [-o out.html] [--title "..."] * * Inputs (all optional except grid.json — missing sections are simply omitted): * model.json 01-scope.md 02-control-structure.md grid.json @@ -241,7 +241,7 @@ if (argv.includes("--help") || argv.includes("-h")) { [ "RenderReport.ts — self-contained HTML report for an STPA analysis", "", - "Usage: bun RenderReport.ts [analysis-dir] [-o out.html] [--title \"...\"]", + "Usage: stpa report [analysis-dir] [-o out.html] [--title \"...\"]", "", "Reads grid.json (required) plus model.json and any 0*.md artifacts present.", ].join("\n"), diff --git a/Tools/RenderSummary.ts b/Tools/RenderSummary.ts index c51a205..a21ff7e 100644 --- a/Tools/RenderSummary.ts +++ b/Tools/RenderSummary.ts @@ -21,7 +21,7 @@ * Every number here is computed from the artifacts. None is typed by hand — the * evidence gate exists because a stale hand-typed count reached a deliverable once. * - * Usage: RenderSummary.ts [analysis-dir] [-o out.html] [--title "..."] + * Usage: stpa summary [analysis-dir] [-o out.html] [--title "..."] * Reads grid.json (required), plus 06-remediation.json, 01-scope.md, model.json and * review-scorecard.json when present. Writes /SUMMARY.html. */ @@ -57,7 +57,7 @@ if (argv.includes("--help") || argv.includes("-h")) { [ "RenderSummary.ts — one-page executive summary for an STPA analysis", "", - "Usage: RenderSummary.ts [analysis-dir] [-o out.html] [--title \"...\"]", + "Usage: stpa summary [analysis-dir] [-o out.html] [--title \"...\"]", "", "Reads grid.json (required) plus 06-remediation.json, 01-scope.md, model.json", "and review-scorecard.json when present. Writes /SUMMARY.html.", diff --git a/Tools/ReportLink.ts b/Tools/ReportLink.ts index ea0df84..1e292a1 100644 --- a/Tools/ReportLink.ts +++ b/Tools/ReportLink.ts @@ -33,7 +33,7 @@ * and print the one command that resolves it. * * Usage: - * bun ReportLink.ts [analysis-dir] [--copy-to ] [--quiet] + * stpa link [analysis-dir] [--copy-to ] [--quiet] * * Env: * STPA_HOST_MAP comma-separated container=host prefix pairs, e.g. diff --git a/Tools/ScopeGate.ts b/Tools/ScopeGate.ts index fc6fb1f..3f8ba11 100755 --- a/Tools/ScopeGate.ts +++ b/Tools/ScopeGate.ts @@ -31,7 +31,7 @@ * cannot manufacture a clean percentage. * * Usage: - * bun ScopeGate.ts [--inventory N] [--warn-only] + * stpa scope [--inventory N] [--warn-only] */ import { existsSync, readFileSync } from "node:fs"; @@ -48,7 +48,7 @@ if (argv.includes("--help") || argv.includes("-h")) [ "ScopeGate.ts — hold the analysis to the scope that was requested", "", - "Usage: bun ScopeGate.ts [--inventory N] [--warn-only]", + "Usage: stpa scope [--inventory N] [--warn-only]", "", " --inventory N the target's real entry-point count, to sanity-check the", " candidate denominator (e.g. number of API route files)", diff --git a/Tools/UcaGrid.ts b/Tools/UcaGrid.ts index d64806c..255e697 100755 --- a/Tools/UcaGrid.ts +++ b/Tools/UcaGrid.ts @@ -10,12 +10,12 @@ * cells. An unresolved cell is a hole in the analysis, and now you can count them. * * Usage: - * bun UcaGrid.ts init [-o grid.json] # generate the empty grid - * bun UcaGrid.ts init --merge -o grid.json + * stpa init [-o grid.json] # generate the empty grid + * stpa init --merge -o grid.json * # re-analysis: carry resolved * # cells forward, list only new ones - * bun UcaGrid.ts status # coverage report - * bun UcaGrid.ts markdown [-o grid.md] # analyst-facing checklist + * stpa status # coverage report + * stpa grid [-o grid.md] # analyst-facing checklist * * Input model.json shape (produced by ModelControlStructure workflow): * { @@ -53,6 +53,8 @@ * Coverage = (bound findings + reasoned tombstones) / total cells. */ +import { readFileSync, writeFileSync } from "node:fs"; + const UCA_TYPES = [ { key: "not-provided", label: "Not providing causes hazard" }, { key: "provided", label: "Providing causes hazard" }, @@ -133,7 +135,7 @@ function die(msg: string, code = 1): never { /** Write through die() rather than letting a bad path dump a raw stack trace. */ function writeOut(path: string, content: string, note: string): void { try { - require("node:fs").writeFileSync(path, content); + writeFileSync(path, content); } catch (e) { die(`cannot write ${path}: ${(e as Error).message}`); } @@ -146,9 +148,9 @@ function usage(): never { "UcaGrid.ts — STPA Step 3 coverage grid", "", "Usage:", - " bun UcaGrid.ts init [--merge ] [-o ]", - " bun UcaGrid.ts status ", - " bun UcaGrid.ts markdown [-o ]", + " stpa init [--merge ] [-o ]", + " stpa status ", + " stpa grid [-o ]", "", "Cell states: open | uca | tombstone (tombstone requires a `reason`).", "Coverage = (BOUND findings + reasoned tombstones) / totalCells.", @@ -161,7 +163,7 @@ function usage(): never { function readJson(path: string): unknown { let text: string; try { - text = require("node:fs").readFileSync(path, "utf8"); + text = readFileSync(path, "utf8"); } catch { die(`cannot read: ${path}`); } diff --git a/Tools/VerifyGate.ts b/Tools/VerifyGate.ts index 4a6b599..2ecbf05 100755 --- a/Tools/VerifyGate.ts +++ b/Tools/VerifyGate.ts @@ -16,7 +16,7 @@ * and refuses to pass if it is missing, self-reviewed, or incomplete. * * Usage: - * bun VerifyGate.ts [--warn-only] + * stpa verify [--warn-only] * * reviews.json (written by the adversarial-review pass): * { @@ -47,7 +47,7 @@ if (argv.includes("--help") || argv.includes("-h")) { [ "VerifyGate.ts — adversarial peer-review gate", "", - "Usage: bun VerifyGate.ts [--warn-only]", + "Usage: stpa verify [--warn-only]", "", "Refuses to certify an analysis unless every UCA finding was reviewed by an", "INDEPENDENT model and every confirmed-live finding names a deployed path.", diff --git a/package.json b/package.json new file mode 100644 index 0000000..56f553b --- /dev/null +++ b/package.json @@ -0,0 +1,6 @@ +{ + "name": "stpa", + "private": true, + "type": "module", + "description": "Control-theoretic threat modeling (STPA / STPA-Sec) for codebases and design documents. Zero dependencies. This file exists ONLY for the \"type\" field: Node's module-syntax detection reads these ESM tools correctly on its own, but an ancestor directory declaring {\"type\":\"commonjs\"} overrides detection — and the usual install path is ~/.claude/skills/, under a ~/.claude/package.json that declares exactly that. Without this file, every import in Tools/ is a SyntaxError there. Do not delete it." +} diff --git a/stpa b/stpa index d8589fe..546b1f2 100755 --- a/stpa +++ b/stpa @@ -22,11 +22,16 @@ * Everything is offline. No API keys, no network, no telemetry. */ -const { spawnSync } = require("node:child_process"); -const { existsSync, mkdirSync, writeFileSync } = require("node:fs"); -const { join, resolve } = require("node:path"); +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { join, resolve, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; -const HERE = __dirname; +// package.json declares "type": "module", so this file is ESM under both runtimes +// and __dirname does not exist. fileURLToPath is used rather than the newer +// import.meta.dirname so the launcher keeps working on the whole Node range the +// RUNTIME block below claims to support. +const HERE = dirname(fileURLToPath(import.meta.url)); const TOOLS = join(HERE, "tools"); const LEGACY_TOOLS = join(HERE, "Tools"); // tolerate either casing const toolDir = existsSync(TOOLS) ? TOOLS : LEGACY_TOOLS;