Skip to content

Commit 60c93d7

Browse files
abhigyanpatwariclaudemagyargergo
authored
feat: upgrade @ladybugdb/core to 0.15.2 and remove segfault workarounds (#374)
* feat: upgrade @ladybugdb/core to 0.15.2 and remove segfault workarounds The upstream fix (ladybug-nodejs#1) resolves the child QueryResult lifetime segfault, making .close() safe on all platforms. This removes 6 workaround sites: - Remove `dangerouslyIgnoreUnhandledErrors` from vitest config - Remove platform-conditional .close() guards in global-setup and test helper - Delete test/setup.ts (process._getActiveHandles unref hack) - Replace no-op cleanup in test-indexed-db.ts with real adapter close - Fix pool adapter closeOne() to properly close connections with shared Database refcount guard and orphaned connection handling in checkin() - Update segfault-related comments across the codebase Also bumps @ladybugdb/wasm-core to ^0.15.2 in gitnexus-web for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: keep dangerouslyIgnoreUnhandledErrors for macOS N-API exit crash The N-API destructor ordering crash during worker fork exit on macOS is independent of the QueryResult lifetime fix in 0.15.2. Tests pass, but the exit triggers a crash. Keep the flag with an updated comment explaining the actual cause. Can be removed once LadybugDB fixes all destructor ordering issues upstream. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * ci: unify test run for single-pass coverage - Update `npm test` to run all tests (unit + integration + lbug-db) via `vitest run` instead of `vitest run test/unit` - Add `test:unit` script for running unit tests only - Remove `ci-integration.yml` — the per-file lbug-db process isolation is no longer needed with `dangerouslyIgnoreUnhandledErrors` and `fileParallelism: false` handling fork exit issues - Update `ci-unit-tests.yml` to run all tests with build + coverage - Simplify `ci.yml` gate (two jobs: quality + tests) - Simplify `ci-report.yml` (single coverage artifact, no merge step) * fix: update cli-commands test for renamed test:all → test:unit script * fix: set USERPROFILE in setup-skills test for Windows compatibility os.homedir() checks USERPROFILE on Windows, not HOME. * fix: add isolate: false to lbug-db project to prevent fork crashes On macOS, N-API destructors crash fork workers on exit. With isolate: true (default), vitest recycles the fork between files, triggering the crash after each file. After several crashes, the remaining lbug-db files never execute. isolate: false keeps all 8 lbug-db files in a single fork — the fork only exits once after all files complete, and that single exit crash is caught by dangerouslyIgnoreUnhandledErrors. * fix: add unique sequence.groupOrder to vitest projects Vitest v4 requires unique groupOrder when projects have different maxWorkers (lbug-db has fileParallelism: false → maxWorkers: 1). * fix: await async close() in global-setup and remove isolate: false global-setup.ts called conn.close() and db.close() without await — these return Promise<void> in @ladybugdb/core 0.15.2. The setup function returned before the DB was fully closed, so vitest forks hit a stale file lock when opening the same DB path, crashing the lbug-db worker before any test ran. isolate: false caused native state corruption after 2-3 open/close cycles in the same fork (vitest-specific, not reproducible in plain Node.js). Without it, each file gets its own module scope and the N-API destructor crash at fork exit is caught by dangerouslyIgnoreUnhandledErrors. Also fixes fire-and-forget close() calls in the pool adapter — try/catch around an async close() never catches rejections; changed to .catch(() => {}) for proper unhandled-rejection prevention. Before: 0/8 lbug-db files ran on macOS CI (fork crash). After: 8/8 pass, 84 files, 3077 tests, zero errors. * fix: update project index references in AGENTS.md and CLAUDE.md to reflect correct symbol counts and relationships * feat: enhance lbug adapter with external database support and write operation validation * feat: create ci-tests workflow for comprehensive test coverage across platforms * ci: move PR report inline to ci.yml, delete ci-report.yml The old ci-report.yml used workflow_run which always runs code from the default branch (main). This meant the PR comment used main's stale report template that still referenced the old unit/integration split architecture — causing "Merge coverage reports" failures. Moving the report inline to ci.yml means it runs from the PR branch and uses the current report template. The report now shows: - per-platform status (Ubuntu/Windows/macOS columns) - unified test counts from the single vitest run - coverage with base branch (main) delta comparison - commit SHA for traceability Also removes the save-pr-meta job since the report no longer needs a separate workflow_run trigger. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
1 parent 1e19986 commit 60c93d7

File tree

20 files changed

+532
-887
lines changed

20 files changed

+532
-887
lines changed

.github/workflows/ci-integration.yml

Lines changed: 0 additions & 199 deletions
This file was deleted.

0 commit comments

Comments
 (0)