fix(ci): sync Cargo.toml version before native binary build

[carlos-alm]

Summary

• The build-native CI job compiled Rust binaries in parallel with compute-version, so env!("CARGO_PKG_VERSION") baked in the stale Cargo.toml version instead of the release version
• codegraph info reported "binary built as 3.2.0" on a 3.3.0 release (bug: incremental builds produce fewer edges than full builds (3.3% gap) #533 suggestion 10.1)
• Fix: make build-native depend on compute-version and update Cargo.toml before cargo build

Test plan

• Verify publish.yml YAML is valid (CI will check)
• On next release, confirm codegraph info shows matching binary and package versions
• Verify native builds still complete on all 6 platform targets

[claude]

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

---

Reviewing PR: sync Cargo.toml version before native binary build

Todo List

• Read and analyze the workflow changes
• Apply 5-phase review process
• Evaluate backlog compliance
• Provide final assessment and rating

Root Cause Analysis

Problem Being Solved: The build-native CI job was running in parallel with compute-version, causing the Rust binary to be compiled with the stale version from Cargo.toml instead of the dynamically computed release version. This resulted in codegraph info showing mismatched versions (e.g., "binary built as 3.2.0" on a 3.3.0 release).

Why This Approach: The fix correctly addresses the root cause by:

1. Making build-native depend on compute-version to ensure proper ordering
2. Adding a step to sync the computed version into Cargo.toml before compilation
3. Using env!("CARGO_PKG_VERSION") at compile time now picks up the correct version

Risk Assessment: ✅ LOW RISK - This is a proper fix that addresses the actual problem rather than masking symptoms.

Configuration Change Detection

• No linting rules modified
• No test timeouts or coverage thresholds changed
• No TypeScript strict mode changes
• ✅ Clean CI workflow change with clear justification

Test Integrity

• No tests modified
• No assertions weakened
• ✅ Test plan in PR body shows awareness of validation steps

Code Quality

• The awk command for version replacement is solid: /^version =/{$0="version = \""v"\""}1
• Proper error handling with temp file approach: > "${CARGO}.tmp" && mv "${CARGO}.tmp" "$CARGO"
• Dependency ordering fix is correct: needs: [preflight, compute-version]
• Shell script follows best practices with quoted variables

Minor Improvement Suggestion: The awk pattern could be more specific to avoid accidentally matching commented lines, but given Cargo.toml's structure, this is unlikely to be an issue in practice.

Backlog Compliance

• Zero-dep: ✅ No new runtime dependencies added
• Foundation-aligned: ✅ Purely CI infrastructure improvement, doesn't violate any principles
• Problem-fit: 3 - Useful but doesn't directly address the core "lost AI" problem
• Breaking: No - No user-facing API or behavior changes
• Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Critical Concerns

None identified. This is a straightforward CI fix that properly addresses a version synchronization issue.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: Clean fix that addresses the root cause without introducing risks. Well-tested approach with proper dependency ordering. The only reason it's not 5 stars is that it's a CI fix rather than a feature that directly enhances the product. Implementation is solid and the test plan demonstrates awareness of validation steps.

Recommendation: This PR should be merged. It fixes a real version synchronization issue in the CI pipeline with a proper solution.

[greptile-apps]

Greptile Summary

This PR fixes a CI race condition where build-native ran in parallel with compute-version, causing Rust binaries to embed the stale Cargo.toml version via env!("CARGO_PKG_VERSION") instead of the actual release version.

The fix has two parts:

• build-native now declares needs: [preflight, compute-version] so it cannot start before the version is computed.
• A new "Sync Cargo.toml version" step patches crates/codegraph-core/Cargo.toml with the computed version before napi build / cargo build runs, ensuring CARGO_PKG_VERSION is baked in correctly.

Key implementation details:

• A guard ([[ -n "$VERSION" ]] || exit 1) provides a clear failure message if compute-version output is missing.
• The awk command uses an early-exit flag (!done) to replace only the first ^version = line, consistent with the first-occurrence-only behavior of scripts/sync-native-versions.js and safe against future TOML sections that also contain a version = key.
• shell: bash is explicitly set so the bash syntax ([[ ]]) and awk command work on all six matrix platforms, including windows-latest.
• The fix correctly integrates with the existing publish job flow, which independently calls node scripts/sync-native-versions.js to produce the version-bump PR.

Confidence Score: 5/5

• This PR is safe to merge — it is a targeted, well-guarded CI fix with no changes to application source code.
• The root cause (parallel job execution) is correctly addressed by the new needs dependency. The version-patch step is minimal, guarded against empty input, idempotent within a single job run, and consistent with the existing scripts/sync-native-versions.js approach. The shell: bash declaration ensures cross-platform compatibility. No application logic is touched. Both concerns raised in the previous review threads (empty VERSION guard and first-occurrence-only awk) have been incorporated into the final implementation visible in the diff.
• No files require special attention.

Important Files Changed

Filename	Overview
.github/workflows/publish.yml	Adds compute-version as a dependency of build-native and inserts a "Sync Cargo.toml version" step with an empty-VERSION guard and first-occurrence-only awk replacement. All six matrix targets (including Windows) are correctly covered via shell: bash.

Sequence Diagram

sequenceDiagram
    participant P as preflight
    participant CV as compute-version
    participant BN as build-native (x6 matrix)
    participant PD as publish-dev / publish

    P->>CV: needs preflight
    CV->>BN: needs preflight + compute-version (NEW)
    Note over BN: 1. Patch Cargo.toml with VERSION
    Note over BN: 2. napi build --release --target
    Note over BN: 3. Upload .node artifact
    BN->>PD: needs compute-version + build-native
    Note over PD: Download artifacts, npm publish / gh release

Loading

_{Last reviewed commit: "fix: scope awk versi..."}

[greptile-apps]

awk replaces all ^version = lines, not just the [package] version

The pattern /^version =/ is a "replace all" in awk (every matching line is rewritten). Currently crates/codegraph-core/Cargo.toml only has one such line so this works, but it would silently corrupt the file the moment a second top-level version = … appears (e.g. a workspace [package.metadata] block, a feature-flag version, etc.).

Compare this with scripts/sync-native-versions.js, which uses a first-match-only replace (no g flag on the regex). For consistency and safety, scope the replacement to the first occurrence only:

Suggested change

	run: |
	CARGO="crates/codegraph-core/Cargo.toml"
	awk -v v="$VERSION" '/^version =/{$0="version = \""v"\""}1' "$CARGO" > "${CARGO}.tmp"
	mv "${CARGO}.tmp" "$CARGO"
	CARGO="crates/codegraph-core/Cargo.toml"
	sed -i.tmp '0,/^version =/{s/^version = "[^"]*"/version = "'"$VERSION"'"/}' "$CARGO"
	rm -f "${CARGO}.tmp"

Alternatively, using awk with an early-exit flag after the first replacement:

awk -v v="$VERSION" '!done && /^version =/{$0="version = \""v"\""; done=1}1' "$CARGO" > "${CARGO}.tmp"
mv "${CARGO}.tmp" "$CARGO"

[carlos-alm]

Fixed in ff3e76a. Added an early-exit flag to the awk command so only the first ^version = line is replaced:

awk -v v="$VERSION" '!done && /^version =/{$0="version = \""v"\""; done=1}1' "$CARGO" > "${CARGO}.tmp"

This matches the first-occurrence-only behavior of scripts/sync-native-versions.js and prevents corruption if a second version = line appears in a different TOML section.

[greptile-apps]

No guard against an empty $VERSION value

If compute-version does not produce a version output (e.g. a job failure that somehow still lets downstream jobs proceed, or a future refactor that renames the output key), the awk command will silently write version = "" into Cargo.toml. Cargo will then error with a confusing TOML validation message rather than a clear "version is empty" diagnostic.

A one-line guard at the top of the run block keeps the failure obvious:

Suggested change

	- name: Sync Cargo.toml version
	env:
	VERSION: ${{ needs.compute-version.outputs.version }}
	shell: bash
	run: |
	CARGO="crates/codegraph-core/Cargo.toml"
	awk -v v="$VERSION" '/^version =/{$0="version = \""v"\""}1' "$CARGO" > "${CARGO}.tmp"
	mv "${CARGO}.tmp" "$CARGO"
	- name: Sync Cargo.toml version
	env:
	VERSION: ${{ needs.compute-version.outputs.version }}
	shell: bash
	run: |
	[[ -n "$VERSION" ]] || { echo "::error::VERSION is empty — compute-version output missing"; exit 1; }
	CARGO="crates/codegraph-core/Cargo.toml"
	awk -v v="$VERSION" '/^version =/{$0="version = \""v"\""}1' "$CARGO" > "${CARGO}.tmp"
	mv "${CARGO}.tmp" "$CARGO"

[carlos-alm]

Fixed in c43a2e4. Added a guard at the top of the run block:

[[ -n "$VERSION" ]] || { echo "::error::VERSION is empty — compute-version output missing"; exit 1; }

The workflow will now fail immediately with a clear GitHub Actions error annotation if $VERSION is empty, rather than silently writing version = "" into Cargo.toml.

[carlos-alm]

Addressed both Greptile review findings:

1. Empty VERSION guard (c43a2e4) — Added [[ -n "$VERSION" ]] check that fails the step with a clear ::error:: annotation if compute-version output is missing.

2. First-occurrence-only awk replacement (ff3e76a) — Added !done flag so only the first ^version = line is rewritten, consistent with scripts/sync-native-versions.js and safe against future Cargo.toml sections containing their own version = lines.

@greptileai