feat(ci sync): PR run summary; decide convergence from a status probe, not commit messages - #10581
Conversation
17b0c80 to
4669f5d
Compare
…of trusting convergence A converged branch/lane pair is not proof there is no work: a developer can commit their own `.bitmap` write together with source edits, which reads as the reconciler's own state commit. Detect a state commit that bundles source changes and probe it rather than short-circuiting to noop. The probe records the sync ledger whichever way it answers. Skipping the ledger on a clean answer left the developer's commit as the tip, so the same probe -- a checkout, a status and a snap -- ran again on every later run and never settled. The ledger commit moves the tip, which is what lets the existing sync-authored-tip check settle the next run before any workspace work, the same way a commit touching no bit-tracked file settles.
Each sync run upserts one comment on the branch's pull request: the lane it mirrors, the branch tip it was computed from, and the components whose versions the run moved. Kept as a single maintained comment (RUN_SUMMARY_MARKER + upsertComment) rather than one post per push, since the snap step can move dependency ranges the PR's own files never touched.
c961235 to
699aefe
Compare
4669f5d to
fe30ea9
Compare
… from commit messages
luvkapur
changed the title
PR Summary by QodoCI sync: decide convergence via
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 3bb8512 |
…h an exhausted page budget
|
Code review by qodo was updated up to the latest commit c3a98ec |
|
Code review by qodo was updated up to the latest commit bf2141e |
…g a nothing-new merge exported
|
Code review by qodo was updated up to the latest commit c1aa3e4 |
|
Code review by qodo was updated up to the latest commit ae2c3f9 |
|
Code review by qodo was updated up to the latest commit 6a3aa5a |
…he lane moves mid-run
|
Code review by qodo was updated up to the latest commit 8ea415b |
|
Code review by qodo was updated up to the latest commit c8b8842 |
…th the plan-freshness guard
|
Code review by qodo was updated up to the latest commit 431dcdf |
|
Code review by qodo was updated up to the latest commit a107a46 |
|
Code review by qodo was updated up to the latest commit f7cd39a |
…d, never a refetched tip
|
Code review by qodo was updated up to the latest commit 88d4fbc |
|
Code review by qodo was updated up to the latest commit 9cda922 |
|
Code review by qodo was updated up to the latest commit bc710f2 |
2 participants
Proposed Changes
bit ci synccould report a branch as converged and silently drop its work.bit statusand compare the branch's files with the lane.(Replaces #10598 — this branch contains its commit and reworks its mechanism.)
Bug 1: a commit that bundles source edits with a
.bitmapwrite was invisibleThe sync reads "work on the branch" as "commits after the commit that last wrote
.bitmap". A single commit that carries a source edit and a.bitmapwrite is that state commit itself, so the count is zero. The run reported converged and the edit never reached the lane. The conflict-resolution recipe (import, fix by hand, one commit) produces exactly this shape.The fix detects this shape: a state commit that also touched other files is suspected work, and the run must check it instead of trusting the fingerprints.
Bug 2: the check trusted commit messages, and a squash-merge forges them
The first version of that check wrote a marker commit and recognized it later by its message (
[bit-sync]on its own line). But a squash-merge copies the messages of the squashed commits into the new commit's body, and a synced branch always contains sync commits with that line. So a squashed developer commit looked machine-written. The run reportednoop (converged). The developer's edits never reached the lane. There was no halt, no label, and no warning.The fix
The run no longer reads commit messages to decide convergence. It asks the workspace instead.
Each run, the planner picks
export-branchwhen the lane did not move but the branch may hold new work. Git metadata alone cannot confirm the work, so the executor now checks first:bit status. This compares the branch's actual files with the versions the lane records.If status shows nothing, the branch and the lane hold the same content. The run reports converged and writes nothing: no snap, no commit, no push. If status shows changes, the run snaps and exports exactly as before.
A clean check writes nothing, so it is safe to repeat on every run. This removes the old settle mechanism: an empty ledger commit pushed to the developer's branch, recognized on the next run by its own message. The marker check remains in one place only — branch deletion after the lane is gone, where no content exists to compare.
Two behavior changes, both intended:
import-lanecreated) re-checks on every run. This is a read. It pushes nothing and triggers no CI run.merge-diverged, notimport-lane. An import would write the lane's files over the branch's bundled work. That is the same bug in a second shape. A merge that finds nothing new says so in its summary instead of claiming it exported.The run-summary comment
A sync run can change a lane component — a new snap, a dependency range — while the branch's own diff shows nothing. The reviewer cannot see this in the pull request.
After a successful export (plain or merge), the run now writes one comment on the branch's pull request: the components it snapped, with their new versions, and the synced branch/lane anchors. The run updates the same comment in place on every export. It never adds a second one.
This uses a new optional provider capability,
GitHostProvider.upsertComment. A provider without it is skipped. A missing git host, a missing pull request, or a comment API error never fail the run. The comment search follows GitHub's pagination but never follows aLinktarget off the API host (the bearer token must stay home), stops at the first marker match, and treats an exhausted page budget as "unknown" rather than "absent" — so it never posts a duplicate.Tests
github-client.spec.ts:upsertCommentposts, patches in place, honorscreateIfAbsent: false, follows pagination, refuses off-hostLinktargets, and never posts through an exhausted page budget.lane-sync-executor.spec.ts: the summary-comment helpers, and merge-diverged's exported vs nothing-new split.e2e/harmony/ci-sync.e2e.ts: 52 passing.