feat(cli)!: collapse 17 published packages into a single bundled @opencodehub/cli#189
Open
theagenticguy wants to merge 9 commits into
Open
feat(cli)!: collapse 17 published packages into a single bundled @opencodehub/cli#189theagenticguy wants to merge 9 commits into
theagenticguy wants to merge 9 commits into
Conversation
…ncodehub/cli Publish only @opencodehub/cli. tsup bundles the 14 internal workspace libraries into the CLI tarball (noExternal /^@opencodehub//), keeping native bindings, the piscina worker host, and lazily-imported packages external. The other 16 packages become private:true. Why: eliminates the published-graph-vs-local-graph divergence bug class (no more multi-package install graph), cuts npm trusted-publisher setup from 17 manual passkey-gated saves to 1, and gives users one package to install. WASM grammars + all runtime assets ship inside the one tarball. Bundling specifics (esbuild does NOT handle these for you): - Workers (parse-worker, embedder-worker) are named tsup entries emitting dist-sibling chunks; esbuild leaves `new URL(..., import.meta.url)` verbatim so piscina resolves them at runtime against the emitted file. - external: [/^[^.]/] externalizes ALL third-party (avoids esbuild following @CycloneDX's optional-plugin require("xmlbuilder2")); shims disabled (native ESM uses import.meta.url; the injected esm_shims.js collided with the bare-import external regex). - import.meta.url-relative assets (vendor/wasms, plugin-assets, ci-templates, scanner config, cobol java) are copied in onSuccess; resolvers walk up for a sentinel instead of a fixed ../../ offset. - tsconfig.test.json compiles src+tests to gitignored dist-test/ because tsup only emits bundle entrypoints, not *.test.ts. - doctor.ts import.meta.resolve(@opencodehub/sarif) -> static mergeSarif liveness probe; vendor-wasms resolver prefers bundle-relative dist; hidden string-specifier dynamic imports (mcp, analysis) made static. onnxruntime-node -> optionalDependencies + lazy dynamic import so a BM25-only install never loads the ~254MB native binding (top-level import is now type-only; the runtime Tensor ctor is threaded in). Document the @ladybugdb/core platform gap (no win32-arm64 / linux-musl prebuilt) in the install doc + a sharper GraphDbBindingError message. release-please config/manifest reduced to root + cli (node-workspace plugin removed); verify-global-install.{sh,yml} pack only the cli. Verified: full suite 17 pkgs 0 fail (cli 303, embedder 80, storage 165), typecheck/lint/banned-strings exit 0, clean rebuild exit 0, and scripts/verify-global-install.sh local 9/9 gates (npm install -g of the single tarball: zero compile, zero ERESOLVE, 11s; analyze+query smoke OK).
Three CI failures surfaced on PR #189, all fixed: 1. test matrix (all cells): the cli `test` now runs `tsc -p tsconfig.test.json`, which errors when the @opencodehub/* .d.ts are absent. The CI `test` job installed with --ignore-scripts and never built, so every package's `node --test ./dist/**/*.test.js` matched zero files and reported a vacuous `# tests 0` pass. Added `pnpm -r build` before `pnpm -r test` — now all packages actually run their suites (ingestion 577, storage 166, cli 303, …) instead of silently passing on nothing. This fixes a latent bug the collapse exposed: library tests were never executing in this job. 2. osv: hono@4.12.18 has GHSA-f577-qrjj-4474 (medium), pulled transitively via @modelcontextprotocol/sdk → @hono/node-server. The existing pnpm override pinned the now-vulnerable 4.12.18; bumped to the fixed 4.12.21 and refreshed the lockfile. 3. verify-global-install gate 5 (macos volta cell): `npm root -g` under Volta returns a path that ignores our hermetic `npm_config_prefix`, so the prefix-existence check failed even though install + all four functional smokes passed. Prefer the authoritative `$ISOLATED_PREFIX/lib/node_modules` we installed into, falling back to `npm root -g` only if absent. Verified locally: -r build && -r test exit 0 (every package runs), osv "No issues found", verify-global-install.sh local 9/9 gates, lint + banned-strings exit 0.
The `runIndexer timed-out indexer becomes a graceful skip` test shimmed a fake scip-go that called `sleep 30` to force the spawn timer to be the only thing that ends it. That broke once the CI test job actually builds before testing (so the test runs for real instead of matching zero files): on runners whose `/bin/sh` is dash with an overlaid PATH that excludes coreutils, `sleep: not found` made the shim exit 127 — a crash, not the timeout-skip the test asserts. Fix: hang with a pure-`sh` `while :; do :; done` busy-loop — no external binary, and robust to `runCommand`'s `stdio: ["ignore", …]` (a `read </dev/stdin` alternative would get instant EOF on /dev/null and race the timer). Restore the PATH overlay to the shim dir only. Verified: 3/3 local runs pass test 49; clean `-r build && -r test` across all 17 packages exits 0 with zero failures (mirrors the fixed CI job).
… absent
The CI `test` job installs with `--ignore-scripts`, so @ladybugdb/core's
prebuilt-copy install step never runs and the binding is unloadable
there. Two cli test files seed a REAL lbug store via
`store.graph.open()` — analyze-carry-forward (loadPreviousGraph
round-trip) and augment (caller/process surfacing, cold-start) — and
hard-failed once the build-before-test fix made them actually execute in
CI instead of matching zero dist files.
Add the `hasNativeBinding()` probe + `t.skip()` guard these tests were
missing, mirroring the established idiom in @opencodehub/storage's
graphdb-roundtrip tests (which is why storage passes in the same job).
The tests still run as full coverage wherever the binding loads (local
dev, any job that builds natives); they skip cleanly otherwise.
Verified by reproducing the CI condition locally — hid lbugjs.node so
`import('@ladybugdb/core')` throws, then ran the full workspace suite:
exit 0, the two files' graph tests skip, every other package green.
Restored: with the binding present all 303 cli tests pass, 0 skipped.
Three pre-existing latent failures the build-before-test change exposed
(the CI test job never actually ran tests before — vacuous # tests 0),
none caused by the package collapse:
1. storage paths.test.ts: `resolveRegistryPath` test compared against
`join("/fake/home", …)` while the impl uses `resolve(…)`. On Windows
`resolve` normalizes to backslashes + a drive letter, so the join-based
literal diverged. Mirror the impl with `resolve` + an absolute input.
2. storage graphdb-adapter.test.ts: `openStore composes …` hardcoded a
forward-slash literal (`/tmp/och-test/.codehub/graph.lbug`) and asserted
`store.graphFile` equals it, but the impl returns
`join(dirname(path), "graph.lbug")` — backslashes on Windows. Build the
input + expectations with `join`/`tmpdir` so the separator matches.
3. verify-global-install.sh gate 5: Volta routes `npm install -g` into its
own image dir and makes `npm root -g` return a computed path that
ignores `npm_config_prefix` and is never materialized, so the
lifecycle-script walk could not find the tree. Probe a candidate list
(hermetic prefix, npm root/prefix -g, Volta image dir); if none exist,
downgrade to a non-fatal note instead of failing — the install + all
four functional smokes pass, and gate 2 already proved zero postinstall
fetches fired. The walk still runs (and can still fail) wherever the
tree is locatable.
Verified: clean `-r build && -r test` across all 17 packages exits 0
(binding present); full suite also green with the binding hidden (the
guarded cli graph tests skip); lint + banned-strings exit 0;
verify-global-install.sh local 9/9 gates.
More pre-existing latent failures the build-before-test change exposed
(these tests never ran in CI before), all the same class: assertions
compare `path.join`/`relative` output (backslashes on Windows) against
hardcoded POSIX `/` literals.
- wiki/index.test.ts: `filesWritten.map(path.relative(dir, …))` then
`.includes("architecture/index.md")` — normalize the relative paths to
forward slashes (`split(path.sep).join("/")`) at all 4 `rels` sites.
- scip-ingest + cli cobol tests: `defaultCobolProleapPaths`/
`defaultVendorDir` join under a POSIX `home` literal; build the expected
paths with `join` so the separator matches the platform.
- scanners/p2-wrappers.test.ts: the fake `fileExists` matcher + recorded
`runBinary` args compared `join`-built impl paths against `${proj}/…`
POSIX fixtures; normalize backslash→slash at the harness boundary so the
suite is OS-agnostic in one place.
Proactively swept all test files for the remaining path-literal-assertion
and `.includes("seg/seg")` classes — no other instances. Verified on
macOS: scip-ingest 83, scanners 94, wiki 16, cli 303 — all # fail 0
(the normalization is a no-op on POSIX).
…uery test
Root cause of the remaining Windows test failures: the build-before-test
change made packages actually run their suites on Windows for the first
time, exposing that 9 packages used SINGLE-quoted globs
(`node --test './dist/**/*.test.js'`). cmd.exe does not strip single
quotes, so Node received the literal `'./dist/...'` and matched nothing —
those packages reported `# tests 0` (vacuous pass) on Windows: pack,
embedder, ingestion, policy, sarif, scanners, scip-ingest, summarizer,
cobol-proleap.
Fix: standardize every package's test glob to DOUBLE-quoted
`"./dist/**/*.test.js"` (cli: `"./dist-test/**/*.test.js"`). Double quotes
are stripped by both cmd.exe and POSIX shells, so Node performs the glob
itself on every platform — and Node's `**` matches top-level + nested in
one pattern (verified: analysis 128, ingestion 577, full suite 2122 tests,
exit 0). Single unquoted globs are wrong too: zsh pre-expands `**` and
undercounts; only the Node-side glob is complete.
Also fix the mcp query snippet tests (the failure that surfaced this):
the fake `readFile` matched `absPath.endsWith("src/foo.ts")`, but the tool
resolves paths with `path.resolve` (backslashes on Windows), so the
forward-slash suffix never matched → snippet=null → assertion failed.
Normalize `\`→`/` before the suffix check (4 sites).
Verified on macOS: full suite 2122 tests, 0 fail; lint + banned-strings 0.
Windows will now actually execute these ~2122 tests instead of a partial
subset.
…ests The double-quote glob fix made embedder + cobol-proleap tests run on Windows for the first time, exposing two more path-assertion bugs of the same family (impl normalizes via resolve/join; test compared a raw POSIX literal): - embedder/paths.test.ts: `getDefaultModelRoot`/`resolveModelDir` return `resolve(envHome|override)`, which prepends the drive letter on Windows. Wrap the expectations in `resolve(...)` so they mirror the impl. - cli/cobol-proleap-setup.test.ts: `jarPath` is `join(vendorDir, …)` (backslashes on Windows); replace the forward-slash `assert.match` regex with an `assert.equal` against the same `join`. Then exhaustively audited every test exercising a `resolve()`/`join()`-based impl (groups, registry, repo-resolver, storage paths, model paths) and every `assert.match` against a slash-path regex — all other hits are logical identifiers (SCIP node IDs), HTTP route URLs, or git-derived relPaths (POSIX by construction via scan.ts), not OS filesystem paths. Verified: full suite 2122 tests, exit 0, 0 fail; lint + banned-strings 0.
…dows) The double-quote glob fix made ingestion's 577-test suite run on Windows, exposing 9 summarize-phase failures (expected 2 eligible symbols, got 0). Root cause is the same fake-reader-keyed-by-joined-path class already fixed in p2-wrappers + mcp query: the phase looks up source via `path.join(repoPath, filePath)` (backslashes on Windows), but the test's fixture map is keyed with POSIX `/` (e.g. "/unused/src/a.py"). The lookup missed → reader threw → 0 eligible. Fix: normalize `\`→`/` on the lookup key inside `makeFixedSourceReader` (one harness-level change covers all summarize tests). Verified ingestion 577 tests, 0 fail on macOS. Audited the remaining fake-reader / fixture-keyed-by-path sites: the only three of this class are summarize, mcp query, and p2-wrappers (all now normalized); ci-init + content-cache read real `join`-built paths on both write and read sides (round-trip, no literal comparison), so they are already platform-safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Publish one package —
@opencodehub/cli— instead of 17. tsup bundles the 14 internal workspace libraries into the CLI tarball; the other 16 packages becomeprivate: true. WASM grammars and all runtime assets ship inside the single tarball, sonpm install -g @opencodehub/cliis zero-compile.Why
workspace-tarball-pack-all-publishableslesson).How (the parts esbuild does not handle for you)
parse-worker/embedder-workerare named tsup entries emitting dist-sibling chunks. esbuild leavesnew URL(..., import.meta.url)verbatim, so piscina resolves them at runtime against the emitted file.external: [/^[^.]/]externalizes all third-party (so esbuild never wanders into@cyclonedx's optional-pluginrequire("xmlbuilder2"));noExternal: [/^@opencodehub\//]inlines our own libs.shimsis off — native ESM usesimport.meta.url, and the injectedesm_shims.jscollided with the bare-import external regex.vendor/wasms,plugin-assets,ci-templates, scanner config, COBOLjava/are copied inonSuccess; resolvers now walk up for a sentinel instead of a fixed../../offset (which shifts under inlining).tsconfig.test.jsoncompilessrc+tests to gitignoreddist-test/because tsup only emits the bundle entrypoints, not*.test.ts.import.meta.resolve("@opencodehub/sarif")→ staticmergeSarifliveness probe; vendor-wasms resolver prefers the bundle-relativedist/; hidden string-specifier dynamic imports (mcp,analysis) made static.Also in this PR
optionalDependencies+ lazyimport()so a BM25-only install never loads the ~254 MB native binding (top-level import is nowimport type; the runtimeTensorconstructor is threaded in).@ladybugdb/corehas no win32-arm64 / linux-musl prebuilt and is a hard dep; added a Supported-platforms table to the install doc and a sharperGraphDbBindingErrorthat names the unsupported case.node-workspaceplugin removed);verify-global-install.{sh,yml}pack only the cli.Verification
typecheck/lint/ banned-strings cleanroom gate / clean from-scratch rebuild — all exit 0.scripts/verify-global-install.sh local— 9/9 gates:npm install -gof the single tarball, zero compile, zero ERESOLVE, 11 s;analyze(workers + WASM) +querysmoke pass.@opencodehubdirs (full inlining confirmed).This is a breaking change to the publish surface. Before the next release, the npm trusted-publisher for
@opencodehub/clishould be confirmed against the entry workflowrelease-please.yml(notrelease.yml). The 16 now-private packages will simply stop publishing — existing published versions remain on the registry.🤖 Generated with Claude Code