fix(ci sync): probe a state commit that bundles source edits instead of trusting convergence by luvkapur · Pull Request #10598 · teambit/bit

luvkapur · 2026-08-11T18:59:05Z

Proposed Changes

Fix bit ci sync wrongly reporting "converged" when one git commit contains both a source edit and a .bitmap change. The edit never reached the lane, and the next lane update would overwrite it on the branch.

The bug

Sync decides "does the branch have new work?" by counting commits AFTER the last commit that touched .bitmap. When a single commit changes .bitmap AND source files, that count is zero — so sync reports noop (converged) even though the source edit was never snapped. The lane never gets the edit, and a later import-lane overwrites it.

This is exactly the commit shape sync's own conflict-resolution instructions produce (bit lane import, fix the files, commit once). Found live while testing the halt → resolve → resume flow.

The fix

Git alone can't tell whether the bundled files are already inside the recorded snap (a dev who snapped, exported, and committed everything at once) or were never snapped. So instead of guessing, sync checks with bit:

Nothing to snap → truly converged. Sync writes nothing, same as before.
Something to snap → real work. Sync exports it to the lane, like any other dev commit.

Where a wrong "no work" answer could lose something (branch deletion, divergence, first contact), the bundled commit counts as work — the safe direction. Sync's own ledger commits are exempt (they legitimately bundle merged sources).

Tests

New e2e reproducing the bug: red on master (noop (converged)), green with the fix (the lane gets the edit; the next run converges).
The three ci-sync-state.e2e.ts cells that had locked the old behavior are updated: the converged-dev case still ends with zero writes, the invisible-edit case exports immediately.
257 unit tests, all 59 ci-sync e2e cells (both suites, post-merge with fix(ci sync): adopt a branch whose lane exists but whose committed .bitmap has no lane state #10593), lint, prettier — green.

Note

Master (with #10593 merged) is merged in. At first contact, a bundled commit routes to adopt-branch — adoption already checks with bit before writing — and the different-lane guard now also fires for bundled commits.

…of trusting convergence a commit changing .bitmap AND sources is its own state commit, so the dev-commit count started after it and the edits read as converged — the exact shape the conflict-halt comment's resolve-by-hand recipe produces, stranding the resolution (and a later lane move would import-lane over it). git names alone cannot tell whether those sources are already inside the recorded snap, so the planner treats them as suspected work: a probe-only export-branch lets the snap decide — nothing pending settles as converged with zero writes, real work exports to the lane. suspected work counts as work on every path where a wrong 'no work' answer could lose something (deletion, divergence, first contact). the state-model cells that locked the old Stage-1 delta are rewritten to the new contract

qodo-free-for-open-source-projects · 2026-08-11T19:00:15Z

PR Summary by Qodo

Probe bundled .bitmap+source state commits to avoid false ci-sync convergence

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Detect when the state commit also touches sources and treat it as suspected work.
• Plan a probe-only export so snap decides if anything is actually pending.
• Update unit/e2e suites to cover bundled commits, probing, and deletion safety paths.

Diagram

graph TD
  A[("Remote git branch")] --> B["readBranchSyncState"] --> C["planLaneSync"] --> D["LaneSyncExecutor"] --> E["snapPrCommit"] --> F[("Remote lane")]
  B --> G{"
state commit bundles sources?\n(suspected work)
"}
  G --> C
  C --> H{"probeOnly export?"} --> D

  subgraph Legend
    direction LR
    _repo[("Git repo")] ~~~ _svc["Service/module"] ~~~ _dec{"Decision"}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Always treat bundled state commit as dev commits

➕ Simpler mental model: bundled commit always triggers export.
➕ Avoids needing a probe-only execution mode.
➖ Would force exports (and ledger commits) even when the developer truly already converged via snap/export.
➖ Higher noise: unnecessary lane updates and reconciler commits.

2. Deep-compare snap contents vs workspace tree

➕ Could deterministically classify whether bundled sources are already snapped.
➕ Avoids probe runs that do no work.
➖ Higher complexity and likely slow/fragile (needs object/model inspection, not just git metadata).
➖ More coupling to Bit internals and snapshot representation.

3. Record extra metadata in the state commit trailer

➕ Makes the ambiguity explicit (e.g., include a content hash of tracked files).
➕ Planner could make a deterministic decision without running a probe snap.
➖ Requires changing the write format/contract and handling back-compat.
➖ Doesn’t help for already-existing ambiguous commits in the wild.

Recommendation: Keep the probe-only approach: it preserves safety (never fake convergence) while avoiding unnecessary writes when the developer already converged. The added signal (stateCommitBundlesSources) is narrowly scoped (tip-only, non-sync-authored) to prevent reconciler-generated merge/ledger commits from causing perpetual re-exports.

Files changed (9) +233 / -66

Bug fix (3) +124 / -39

lane-sync-executor.tsImplement probe-only export and structured snap/export outcomes +35/-32

Implement probe-only export and structured snap/export outcomes

• Propagates 'stateCommitBundlesSources' into planning/execution logging and supports 'probeOnly' exports. Refactors 'snapAndExportOntoLane' to return explicit noop/exported/error results and settles probe-only runs as noop with zero writes when nothing is pending.

scopes/git/ci/sync/lane-sync-executor.ts

sync-planner.tsPlan probe-only export when the state commit bundles sources +20/-6

Plan probe-only export when the state commit bundles sources

• Adds 'stateCommitBundlesSources' to inputs and treats it as potential work ('mayCarryWork') for safety on divergence/deletion/first-contact paths. When everything else reads converged, plans 'export-branch' with 'probeOnly: true' instead of noop.

scopes/git/ci/sync/sync-planner.ts

sync-state.tsDetect state commits that bundle sources and expose NO_CHANGES_TO_SNAP +69/-1

Detect state commits that bundle sources and expose NO_CHANGES_TO_SNAP

• Introduces 'NO_CHANGES_TO_SNAP' as the shared sentinel for snap/noop. Enhances 'readBranchSyncState' to flag tip state commits (not sync-authored) that also touch non-'.bitmap' files via 'diff-tree', reporting this as 'stateCommitBundlesSources' for probe-based planning.

scopes/git/ci/sync/sync-state.ts

Refactor (1) +4 / -3

ci.main.runtime.tsCentralize NO_CHANGES_TO_SNAP message via sync-state constant +4/-3

Centralize NO_CHANGES_TO_SNAP message via sync-state constant
• Reuses the shared 'NO_CHANGES_TO_SNAP' constant for consistent snap/noop detection and messaging. Avoids string duplication across CI snap paths.
scopes/git/ci/ci.main.runtime.ts

Tests (5) +105 / -24

ci-sync-state.e2e.tsRewrite Stage-1 delta tests to the new probe/export contract +20/-21

Rewrite Stage-1 delta tests to the new probe/export contract

• Updates the state-model v2 e2e assertions to expect probe-based convergence (noop with zero writes) and immediate export of previously-invisible bundled edits. Tightens the deletion-guard fixture to be '.bitmap'-only to isolate marker semantics.

e2e/harmony/ci-sync-state.e2e.ts

ci-sync.e2e.tsAdd e2e coverage for bundled '.bitmap' + source edit state commits +45/-0

Add e2e coverage for bundled '.bitmap' + source edit state commits

• Introduces a new scenario where a single commit contains a source edit plus a '.bitmap' write, previously misread as converged. Verifies the first run exports the edit to the lane and the second run converges.

e2e/harmony/ci-sync.e2e.ts

lane-sync-executor.spec.tsAdjust executor spec for new snap/export result contract +2/-3

Adjust executor spec for new snap/export result contract
• Updates the mocked 'snapAndExportOntoLane' return value to match the new '{status: ...}' result shape. Keeps the ledger-race wording test aligned with the updated execution flow.
scopes/git/ci/sync/lane-sync-executor.spec.ts

sync-planner.spec.tsAdd planner test rows for bundled-state suspected work paths +24/-0

Add planner test rows for bundled-state suspected work paths
• Extends the action table to cover probe-only export when otherwise converged, escalation to merge-diverged when the lane moved, keeping branches on deletion paths, and halting on first-contact ambiguity.
scopes/git/ci/sync/sync-planner.spec.ts

sync-state.spec.tsAdd unit tests for 'touchesBeyondBitmap' classification +14/-0

Add unit tests for 'touchesBeyondBitmap' classification
• Validates that empty and '.bitmap'-only name lists are not treated as bundled work, and that any additional file is. Ensures the new suspected-work detector is covered without depending on real git history.
scopes/git/ci/sync/sync-state.spec.ts

qodo-free-for-open-source-projects · 2026-08-11T19:05:04Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Empty diff treated clean~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

commitTouchesBeyondBitmap() is documented to fail-safe (unreadable ⇒ true), but if git.raw()
resolves with an empty string it returns false via touchesBeyondBitmap(''). Given this module
already notes that simple-git can resolve empty output on non-zero exits, this can incorrectly
clear stateCommitBundlesSources and let a bundled state commit be declared converged without
probing/exporting.

Code

scopes/git/ci/sync/sync-state.ts[R218-221]

+    ]);
+    return touchesBeyondBitmap(names);
+  } catch {
+    return true;

Evidence
The code explicitly states unreadable outputs must keep the export path open, but the implementation
only handles the thrown-error case; an empty resolved string will be treated as “no files besides
.bitmap” because touchesBeyondBitmap('') is false. The same file already documents that
simple-git can resolve empty output on non-zero exits, making this a realistic failure mode.
scopes/git/ci/sync/sync-state.ts[145-153]
scopes/git/ci/sync/sync-state.ts[176-199]
scopes/git/ci/sync/sync-state.ts[201-223]
scopes/git/ci/sync/sync-state.ts[225-232]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`commitTouchesBeyondBitmap()` promises “Unreadable answers `true`”, but currently only treats thrown errors as unreadable. If `git.raw([...diff-tree...])` returns an empty string (which this module already documents as possible for `git.raw` on non-zero exits), the function will incorrectly return `false`.
### Issue Context
This boolean feeds `stateCommitBundlesSources`, which is used to decide whether to probe/export rather than declare convergence.
### Fix Focus Areas
- scopes/git/ci/sync/sync-state.ts[201-223]
### Suggested change
In `commitTouchesBeyondBitmap()`:
- After `git.raw(...)`, add a guard:
- if `!names.trim()` return `true` (unknown/unreadable)
- optionally also if the output doesn’t include `.bitmap` (unexpected for a “state commit”), return `true`
- Consider adding/adjusting a unit test by factoring the guard into a small exported helper (or otherwise making it testable) to cover the empty-string case without relying on real git execution.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. CiMain uses chalk.yellow 📘 Rule violation ⚙ Maintainability

Description

This PR modifies CLI output code but still formats the message with direct chalk.yellow(...)
instead of using the shared @teambit/cli output formatting toolkit/style guide. This can lead to
inconsistent CLI output styling and harder-to-maintain formatting across commands.

Code

scopes/git/ci/ci.main.runtime.ts[R1146-1147]

+        this.logger.console(chalk.yellow(NO_CHANGES_TO_SNAP));
+        return NO_CHANGES_TO_SNAP;

Evidence
PR Compliance ID 1 requires using the shared CLI output formatting toolkit/style guide when
modifying CLI output. The changed lines still apply direct chalk.yellow(...) formatting for the
user-facing message, bypassing the shared formatter utilities.
CLAUDE.md: Use shared CLI output formatting toolkit and follow CLI output style guide: CLAUDE.md: Use shared CLI output formatting toolkit and follow CLI output style guide: CLAUDE.md: Use shared CLI output formatting toolkit and follow CLI output style guide: CLAUDE.md: Use shared CLI output formatting toolkit and follow CLI output style guide
scopes/git/ci/ci.main.runtime.ts[1146-1147]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `bit` CLI output was modified, but the code still uses ad-hoc `chalk` formatting instead of the shared CLI output formatting toolkit required by the style guide.
## Issue Context
The compliance checklist requires using the shared formatting utilities (per `scopes/harmony/cli/cli-output-style-guide.md` and `@teambit/cli` output formatter utilities) when changing CLI output, to keep output consistent and maintainable.
## Fix Focus Areas
- scopes/git/ci/ci.main.runtime.ts[1146-1147]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

…n diff a state commit changed .bitmap by definition, so its first-parent diff is never legitimately empty — empty output is simple-git resolving on a non-zero exit, and must fail toward probing

…robe first contact with bundled sources routes to adopt-branch (adoption already probes via bit status); the different-lane guard's gate widens to suspected work so a bundles-only branch cannot be adopted over another lane's live claim; deletion cascade keeps master's shape with mayCarryWork feeding the unmerged-work check

luvkapur marked this pull request as draft August 11, 2026 18:59

qodo-free-for-open-source-projects Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread scopes/git/ci/ci.main.runtime.ts

Comment thread scopes/git/ci/sync/sync-state.ts

luvkapur added 2 commits August 11, 2026 15:56

fix(ci sync review): empty diff-tree output is unreadable, not a clea…

6dbf3c0

…n diff a state commit changed .bitmap by definition, so its first-parent diff is never legitimately empty — empty output is simple-git resolving on a non-zero exit, and must fail toward probing

Navigation Menu

Uh oh!

fix(ci sync): probe a state commit that bundles source edits instead of trusting convergence - #10598

fix(ci sync): probe a state commit that bundles source edits instead of trusting convergence#10598
luvkapur wants to merge 3 commits into
masterfrom
fix/ci-sync-bundled-state-commit

luvkapur commented Aug 11, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 11, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

luvkapur commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

The bug

The fix

Tests

Note

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 11, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

luvkapur commented Aug 11, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 11, 2026 •

edited

Loading