Skip to content

fix(publish): build native addon from source in preflight#954

Merged
carlos-alm merged 2 commits into
mainfrom
fix/publish-preflight-build-native
Apr 17, 2026
Merged

fix(publish): build native addon from source in preflight#954
carlos-alm merged 2 commits into
mainfrom
fix/publish-preflight-build-native

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

The publish workflow's preflight job ran npm install && npm test against the last-published native binary, while the WASM engine used the current TypeScript source. When a PR updated both the TS and Rust extractors in lockstep (e.g. #947), the build-parity test passed on CI — which rebuilds native from source via the parity job — but failed on every subsequent publish.yml run until the next release caught up.

This is what blocked the v3.9.4 release (run 24588894845): the preflight loaded the v3.9.3 native binary (21 nodes, 37 edges) while WASM produced 26 nodes, 41 edges from PR #947's extractor additions.

Fix

Mirror the ci.yml parity job pattern inline in preflight:

  • Setup Rust toolchain + cache
  • npm install (pulls the stale published binary into node_modules/@optave/codegraph-linux-x64-gnu)
  • napi build --release from crates/codegraph-core
  • node scripts/ci-install-native.mjs copies the freshly-built .node over the published binary
  • npm test — both engines now run from the same commit

Test plan

  • Preflight runs on this PR (dev publish on push-to-main won't trigger until merge, but the preflight job runs on workflow_dispatch and release events; CI's own parity job already validates the native build step works)
  • After merge, re-run the v3.9.4 release workflow and confirm preflight passes

The preflight job ran `npm install && npm test` against the last-published
native binary, while WASM ran from current source. When a PR updated both
extractors in lockstep, the parity test would pass on CI (which rebuilds
native from source) but fail on every subsequent publish run until the
next release caught up.

Mirror the ci.yml `parity` job pattern: compile the native addon from the
current Rust source, copy it over the installed platform binary via
ci-install-native.mjs, then run tests. WASM and native now come from the
same commit.
@claude
Copy link
Copy Markdown

claude Bot commented Apr 17, 2026

Claude finished @carlos-alm's task —— View job


PR Review: Native Addon Build Fix

Review Checklist:

  • Read and analyze the workflow changes
  • Phase 1: Root cause analysis
  • Phase 2: Configuration change detection
  • Phase 3: Test integrity assessment
  • Phase 4: Code quality review
  • Phase 5: Backlog compliance evaluation
  • Provide final assessment and recommendation

Root Cause Analysis

Problem Being Solved: The publish workflow's preflight job was using stale published native binaries (from the last npm release) while the WASM engine used current TypeScript source. When PRs updated both TS and Rust extractors in lockstep (like #947), this created a version mismatch that caused build-parity tests to fail during publish.yml runs, even though they passed in CI.

Why This Approach: The solution correctly mirrors the proven pattern from ci.yml's parity job. Instead of using the stale published binary from npm, it rebuilds the native addon from the current commit's Rust source, ensuring both engines are synchronized. This is the right architectural approach.

Risk Assessment: ✅ LOW RISK. This is a defensive fix that eliminates a source of false failures. The added build step is isolated to preflight and mirrors existing proven CI patterns.

Backlog Compliance

  • Zero-dep: ✅ — No new runtime dependencies added
  • Foundation-aligned: ✅ — Maintains dual-engine architecture integrity (P1: always-current)
  • Problem-fit: 4/5 — Directly prevents false CI failures that block releases, improving reliability for AI agents depending on stable builds
  • Breaking: No — Pure CI infrastructure change, no user-facing impact
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Technical Analysis

Workflow Changes (lines 35-58):

  • Setup Rust + cache: Standard rust-toolchain@stable with Swatinem cache
  • napi CLI installation: Matches CI pattern exactly (@napi-rs/cli@3)
  • Build sequence: npm installnapi build --releaseci-install-native.mjsnpm test
  • Working directory: Correctly scoped to crates/codegraph-core

Script Integration: The node scripts/ci-install-native.mjs script is well-tested in CI and handles platform detection, libc detection, and binary replacement safely.

Comparison with CI Pattern:

  • CI native-host-build + parity jobs: Build native → upload artifact → download → install over published binary
  • This PR: Build native inline → install over published binary immediately
  • Simplified but equivalent: Removes the artifact upload/download step since everything runs on the same runner

Code Quality Assessment

Security: ✅ No security concerns. Uses standard GitHub Actions and existing trusted scripts.

Correctness: ✅ The build sequence matches proven CI patterns. The ci-install-native.mjs script correctly:

  • Detects platform (linux-x64-gnu, darwin-arm64, etc.)
  • Locates the built .node file in crates/codegraph-core/
  • Copies it over the published binary in node_modules/@optave/codegraph-*

Performance: ✅ Adds ~2-3 minutes to preflight but eliminates false release failures that cost much more time to debug.

Maintainability: ✅ Follows established patterns and uses existing utilities.

Concerns & Validation

Minor Observations:

  1. The comment (lines 35-38) clearly explains the motivation
  2. Rust cache configuration matches CI exactly
  3. Error handling relies on GitHub Actions' default failure behavior (appropriate)

Test Plan Validation: ✅ The test plan is sound:

  • Preflight will run on this PR to validate the build steps work
  • After merge, re-running the v3.9.4 release should pass (resolving the original blocking issue)

Critical Concerns

None identified. This is a well-designed fix addressing a real infrastructure issue.

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Version mismatch between native binary (stale published) and WASM engine (current source) causing false parity test failures in publish workflow
**Why This Approach**: Mirrors proven CI pattern, rebuilds native from current source to match WASM
**Risk Assessment**: Low risk defensive fix, eliminates false failures

## Backlog Compliance
- Zero-dep: ✓ — no new runtime dependencies
- Foundation-aligned: ✓ — preserves dual-engine architecture integrity  
- Problem-fit: 4/5 — improves release reliability, prevents blocking false failures
- Breaking: No — CI infrastructure change only
- Tier: 1

## Critical Concerns
None

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: **APPROVE**
- Reasoning: Well-architected fix that addresses real CI infrastructure issue using proven patterns. Clear problem identification, appropriate solution scope, and low risk. The only reason it's not 5/5 is that it's infrastructure maintenance rather than feature innovation.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR fixes a publish workflow regression where the preflight job tested the last-published native binary against the current TypeScript/WASM source, causing false parity failures when a PR updated both the Rust extractor and TypeScript in lockstep. The fix mirrors the ci.yml native-host-build pattern: set up Rust, build the .node addon from the current commit, copy it over the stale npm install binary via scripts/ci-install-native.mjs, then run npm test with both engines sourced from the same commit.

Confidence Score: 5/5

Safe to merge — the fix is a direct port of the proven ci.yml pattern with no new failure modes.

Single workflow file change with no P0/P1 findings. Step ordering (npm install → napi build → install-native → test) is correct, Rust cache is wired up to avoid cold-build overhead, and the napi-rs CLI version pin matches the rest of the codebase.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/publish.yml Adds Rust toolchain setup, in-place native addon build, and ci-install-native.mjs invocation to the preflight job — mirrors the proven ci.yml native-host-build → test/parity pattern correctly.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Actions
    participant Rust as Rust Toolchain
    participant npm as npm registry
    participant napi as napi-rs CLI
    participant script as ci-install-native.mjs
    participant tests as npm test

    GH->>npm: npm install (pulls stale published binary)
    npm-->>GH: node_modules/@optave/codegraph-linux-x64-gnu/ (v3.9.3 binary)
    GH->>Rust: Setup + cache (dtolnay/rust-toolchain, Swatinem/rust-cache)
    GH->>napi: npm install -g @napi-rs/cli@3
    GH->>napi: napi build --release (crates/codegraph-core/)
    napi-->>GH: crates/codegraph-core/*.node (built from HEAD)
    GH->>script: node scripts/ci-install-native.mjs
    script-->>GH: copies HEAD .node to node_modules/@optave/codegraph-linux-x64-gnu/codegraph-core.node
    GH->>tests: npm test (WASM + native both from same commit)
Loading

Reviews (1): Last reviewed commit: "fix(publish): build native addon from so..." | Re-trigger Greptile

@carlos-alm carlos-alm merged commit 9882f5d into main Apr 17, 2026
13 checks passed
@carlos-alm carlos-alm deleted the fix/publish-preflight-build-native branch April 17, 2026 23:29
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant