Conversation
Adds sync/lane-sync-executor.ts: fetches the remote lane's content fingerprint and the branch's sync state, runs them through planLaneSync, and executes the resulting action (import-lane, export-branch, close-pr, halt; merge-diverged is stubbed for the next change). Also wires MergeLanesAspect into CiMain (needed by merge-diverged) and exposes switchToLaneForSync so the executor can reuse the PR flow's lane-switch semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e lane
Review fixes on the lane sync executor:
CRITICAL: import-lane wrote no file content. switchToLane defaults to
forceOurs, and applyVersion checks forceOurs first and short-circuits --
every file marked unchanged, filesystem untouched, only .bitmap ids moved.
The sync commit's Bit-Lane-Head trailer then permanently claimed the branch
mirrored the lane. The same short-circuit also threw "applyVersion expect to
get componentFromFS" for any lane component absent from the branch's .bitmap.
The import direction now switches with { forceOurs: false, forceTheirs: true }
via a new materializeLane helper.
IMPORTANT: "already checked out" was swallowed as success. switchLanes throws
it from throwForSwitchingToCurrentLane before doing any work, so switching
onto the lane we already sit on materialized nothing yet still recorded a
trailer. materializeLane now steps off to main first and verifies the landing
lane by name+scope. (bit checkout head is not sufficient -- it derives ids
from workspace.listIds(), so lane components missing from the branch's
.bitmap would be skipped.)
IMPORTANT: export-branch could destroy the lane it mirrors. snapPrCommit's
--keep-lane stale-lane recovery deletes the remote lane and re-forks it from
main; fine for a throwaway PR lane, catastrophic when the lane is the authored
artifact. Adds an opt-in noDestructiveRecovery flag threaded through
snapPrCommit into snapAndExportReusingLane that throws instead. The executor
passes it and turns the throw into a halt, so one failing lane no longer
aborts the rest of the run. bit ci pr behavior is unchanged.
Minor: drop the workspaceOnly option (existingOnWorkspaceOnly is declared in
SwitchProps but never read by LaneSwitcher) and its incorrect comment; log the
"GitHub client present but no open PR" case in close-pr.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements the merge-diverged path of the lane sync executor: 1. attempt the export path (snap the branch's tree onto the lane with keepLane/noDestructiveRecovery); snapPrCommit's adopt-on-conflict recovery converges most divergence in one step. 2. classify a failed export: only the "could not adopt the branch onto the lane" class (applyVersion's missing componentFromFS, checkout's "use --auto-merge-resolve") falls back to a merge; build failures, stale lanes, merge-pending components and export rejections halt. 3. the fallback merges the lane into the branch's working tree with "bit checkout head --manual" (a same-id lane cannot be passed to mergeLanes.mergeLane, and mergeLane refuses to run while components are modified). No conflicts -> export + trailer commit + push; conflicts -> discard the marker writes and halt with the ids. 4. anything unexpected halts; the path never throws and never pushes a partially merged tree. Also adds CiMain.reloadWorkspaceFromDisk() so the merge reads the branch's .bitmap (lane pointer + merge base) rather than the copy the process loaded at startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fixes on the merge-diverged path: - F1 (critical): the export-first attempt was not a merge. snapPrCommit switches with forceOurs, so checkout returns no propsForMerge and applyVersion leaves the filesystem untouched — the snap recorded the branch's tree against the new lane head, reverting every lane-side file edit on the lane tip while the trailer asserted convergence. merge-diverged now runs the three-way merge (bit checkout head --manual) FIRST and unconditionally, then snaps+exports the merged tree, then records the trailer. Conflicts still discard the marker writes and halt with the ids. The export-first structure and its error classification (incl. the unreachable --auto-merge-resolve trigger) are gone; snap/export failures halt. - F2: scope `git clean` with -e .bit -e node_modules so a workspace whose scope lives at <workspace>/.bit (or a .git-as-file worktree) without Bit's gitignore block can't have objects/ and scope.json wiped mid-run. Corrected the JSDoc: simpleGit() has no baseDir, so clean runs in process.cwd(). - F3: restoreWorkspace now reloads the consumer after checking out the default branch, so a branch's .bitmap can't leak into the next lane of a multi-lane run. - M2: removed the now-dead MergeLanes dependency from CiMain (aspect dep, ctor param, provider tuple) — mergeLane cannot express this merge (same lane id, and it refuses to run with modified components). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the main-scope reconciler (MainSyncExecutor), the `bit ci sync` command and the CiMain.sync orchestration that routes lane/branch/main targets and aggregates their summaries (throwing on any HALTED line so CI exits non-zero). Also extracts the git primitives both executors share into sync/git-ops.ts (scoped clean, exclusion-aware `git add -A`, ls-remote branch probe, git identity), which closes the flagged `git add -A .` hazard on the lane path too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hing The reconciler force-checkouts branches and removes untracked files, which is correct in a CI clone and destructive interactively — name the files at stake before touching them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…promising in the sync docs I2: the main-scope checkout now runs with mergeStrategy 'theirs'. Without a strategy, ordinary unexported source drift on the default branch makes `checkout head` throw 'automatic merge has failed ... please use "--auto-merge-resolve"', which landed verbatim in the HALTED summary and named a flag `bit ci sync` does not have. 'theirs' materializes the exported truth, which is what the sync PR is for; the reversion of unexported drift is visible in the PR diff for a human to reject. 'ours' is forbidden: it would advance .bitmap while keeping the old files. I1: `sync.mode` is resolved but never read, so it now gets the same honest treatment as autoMergeMainSyncPr — a runtime warning when a non-default mode is configured, and a docs row marked reserved. No mode gating is implemented. Also: reject --all combined with [lane]/--main instead of silently ignoring it; align the docs with the CLI on --main and with reality on --dry-run (it writes locally and can leave a local sync branch) and on the catch-up merge (only when the sync branch already exists); reword the catch-up justification to the real reasons (PR mergeability + checkout clobbering merged-up files) rather than a claim about PR diffs, which are three-dot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… git client `simpleGit().clean(CleanOptions.FORCE)` is not configuration: simple-git's task methods queue and run their command, so merely importing scopes/git/ci/git.ts executed `git clean -f` in process.cwd() — on any bit command that loaded this aspect — silently deleting untracked files at the repository root. Verified empirically: constructing that chain in a scratch repo removed an untracked file with no explicit call (untracked directories survived, since `-d` was not part of the chain). Nothing relied on the side effect: all six importers of this module issue explicit git commands, and the only deliberate cleaning goes through sync/git-ops.ts's scoped `git clean -fd -e .bit -e node_modules`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion-aware helper `commitAndPushBitmapChanges` staged with `git add .`, which in a workspace whose .gitignore lacks Bit's block commits the local object store (.bit/) onto the default branch. It now uses sync/git-ops.ts's addAllExceptScopeAndModules, the same helper both sync executors use, so all three commit paths agree on what counts as workspace content. Verified: e2e/harmony/ci-commands.e2e.ts "bit ci merge workflow" (5 passing), "--skip-push flag" (6 passing) and "components are new to the lane" (5 passing) — the blocks that exercise this staging path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… at risk Found by running `bit ci sync --branch main` in a workspace with no .gitignore: the new dirty-tree warning claimed 51,535 uncommitted changes and listed `.bit/cache/...` paths as "will be discarded" — but the executors exclude those two paths from both the clean and the staging, so the claim was false and the message unreadable. `isNonContentPath` in sync/git-ops.ts is now the single definition of "not workspace content", applied wherever a `git status` is interpreted: the warning and MainSyncExecutor.driftFiles (which had its own local copy of the prefix list). After the fix the same command reports 2 changes and names them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The help said --main reconciles "against the main branch" (the docs say default branch, which is what the code resolves), promised --dry-run wrote nothing "to the remote" without saying the working tree is written and restored, and did not mention that --all cannot be combined with a lane argument or --main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vider `bit ci sync` was written against `GitHubClient` directly. The engine now knows only the `GitHostProvider` contract (find/create/close PR, comment, label), and GitHub is one implementation registered into a Harmony slot — same mechanics as a package manager registering into dependency-resolver's `packageManagerSlot`. - `sync/git-host-provider.ts`: the `GitHostProvider` interface (owning `PrInfo`, re-exported from github-client for existing importers) plus the pure `selectGitHostProvider(providers, remoteUrl)` selection helper: host match + configured wins, sole configured provider is the fallback, otherwise PR-less. - `sync/github-client.ts`: `GitHubClient implements GitHostProvider`, and a new `GitHubHostProvider` shell that resolves `fromEnv` lazily so it is registrable at aspect-load time (before any env/remote is known) and simply reports `isConfigured() === false` without credentials. - `ci.main.runtime.ts`: `static slots = [Slot.withType<GitHostProvider>()]`, public `registerGitHostProvider` / `listGitHostProviders`, the ci aspect self-registers GitHub through that same public API, and `sync()` selects the active provider instead of constructing a GitHub client. - both executors take `gitHost?: GitHostProvider`; their PR-less logs now name the missing provider/credentials rather than "GitHub client". - `index.ts` re-exports `GitHostProvider` / `PrInfo` / `CiMain` so a future gitlab aspect can implement and register one from out of tree. `isConfigured` takes an optional `remoteUrl`, a refinement over the sketched signature: GitHub derives `owner/repo` from the origin remote when `GITHUB_REPOSITORY` is unset, so "configured" is a function of the remote too — without it, that (pre-existing) non-Actions CI path would regress. 12 new tests for selection and the lazily-configured GitHub provider; the existing 27 (github-client.spec included) are unchanged and green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review found the sole-configured fallback both contradicted the documented
guarantee ("registering a provider never disturbs a repository it doesn't
serve") and enabled wrong-host routing: a GitHub-hosted workspace *without*
GITHUB_TOKEN plus an installed GitLab provider *with* GITLAB_TOKEN silently sent
PR create/close/comment to GitLab.
Selection is now claim-exclusive: if any registered provider claims the remote,
only a claimant can be selected — the first configured one, or nothing. The
sole-configured fallback survives only where it is unambiguous: no remote URL, or
a remote nobody claims. `selectGitHostProvider` returns `{ provider?, reason? }`
so the skip is never silent — `sync()` warns once, up front, naming the provider
that claimed the remote without being configured (the executors' per-action lines
can only say that PR operations were skipped, not why). Docs and the
`registerGitHostProvider` JSDoc now state the real, complete rule.
Adjacent fixes from the same review:
- `fromEnv` only parses `owner/repo` out of a remote that `isGitHubRemote`
accepts: `parseGitHubRepo` is unanchored, so `https://mygithub.com/acme/shop`
used to mint a client aimed at api.github.com for another host's repository.
- `GitHubHostProvider` remembers the remote URL it was asked about, so a PR
method reached without a prior `isConfigured(remoteUrl)` keeps the
origin-parse path instead of reporting itself unconfigured.
- dropped a dead `.flat()` in `listGitHostProviders` (one provider per
registration) and the array-registration implication in its doc.
- `isConfigured`'s contract now warns implementers it may be called more than
once per run and must stay cheap, idempotent and I/O-free.
- the reserved `mode` config row no longer says merges happen "in GitHub".
Selection specs updated to lock the new semantics (the old test that asserted
gitlab won a github.com remote now asserts the opposite); 42 tests green, and
github-client.spec.ts is still byte-identical to pre-task-14.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drives one lane/branch pair through a full lifecycle against a local bare git remote with no git-host credentials (the PR-less path): import-lane, converged no-op, export-branch, merge-diverged (clean and conflicting), and close-pr after the remote lane is removed. Plus the main-scope paths: --all --dry-run must leave every remote ref untouched, and --main must push the scope's state onto bit-sync/main without halting on unexported source drift. Assertions are on file bytes on the pushed branch and at the lane tip, not just on commit existence, because both bugs these paths had produce a well-formed [bit-sync] commit with a valid Bit-Lane-Head trailer and only the content wrong: - scenario A locks the forceOurs materialization fix (the branch tree must hold the lane's content, not just a .bitmap bump); - scenario D1 locks the merge-before-snap fix (the lane-side edit must survive on the lane tip after a diverged reconcile); - scenario E locks the 'theirs' resolution on main sync (unexported git drift must not turn into an "--auto-merge-resolve" halt). The suite unsets GITHUB_TOKEN/BIT_GITHUB_TOKEN/GITHUB_REPOSITORY/GITHUB_HEAD_REF for its duration so a developer's shell cannot flip the run onto a real repository, and asserts the specific PR-less line each path logs. Known failing until scopes/git/ci/sync/git-ops.ts is fixed: the shared staging helper runs `git add -A -- . :(exclude).bit :(exclude)node_modules`, and git exits non-zero when a pathspec element names a gitignored path — including a negative one. Every write path of `bit ci sync` (and of `bit ci merge`, which shares the helper) therefore fails in a workspace whose .gitignore contains Bit's recommended block. See task-9-report.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rkspaces
`git add` exits non-zero ("The following paths are ignored by one of your
.gitignore files … use -f") when ANY pathspec element names a gitignored path —
and it applies that rule to negative `:(exclude)` elements too. So
git add -A -- . :(exclude).bit :(exclude)node_modules
staged the right set but failed with exit 1 in the *normal* setup, where
.gitignore carries Bit's block (node_modules/, .bit/). simple-git turns that exit
code into a throw, so every commit path that shares this helper broke:
- bit ci sync: import-lane, export-branch and merge-diverged (the trailer commit)
- bit ci sync --main: the sync commit
- bit ci merge: commitAndPushBitmapChanges
Only the --dry-run paths survived, because they never stage anything. The
regression reached `bit ci merge` too, whose e2e describe in ci-commands.e2e.ts
was failing in its before hook with this exact error.
The error cannot be suppressed: advice.addIgnoredFile=false drops the hint but
keeps the exit code, and --ignore-errors does not cover it. `-f` does make it
exit 0, but it is unsafe — it force-adds every OTHER ignored path (dist/, *.log,
…) into the sync commit, which is the class of accident the exclusions exist to
prevent (verified: with -f, a gitignored dist/out.js gets staged).
Stage normally and then unstage the two paths instead. `git add -A -- .` already
skips them when they are ignored, and the `reset` covers the pathological
workspace whose .gitignore lacks Bit's block — which is the only reason the
exclusions were there. `git reset -- <paths>` is a no-op (exit 0) for paths
absent from the index, including on an unborn HEAD, so neither command can fail
on a shape the sync engine can reach. SYNC_EXCLUDED_PATHS is unchanged, so all
three commit paths still agree on what counts as workspace content.
Verified: e2e/harmony/ci-sync.e2e.ts 49/49 and the ci-commands.e2e.ts
"bit ci merge workflow" describe 5/5, both against the compiled aspect; unit
suite 43/43; bit lint clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ll of history `syncLane` documents that one unreconcilable lane must not abort the others, but only `executeMergeDiverged` honoured it: every git call around the action dispatch (fetch, ls-remote, the log probes, `checkout -B`, push, `executeClosePr`, `executeHalt`) could throw straight out and take the rest of an `--all` run with it. `syncLane` is now a wrapper around a private `reconcileLane`; on any throw it routes to `executeHalt`, and if the halt itself throws it logs and returns a HALTED line rather than propagating. Both `git checkout -B` call sites now force (`-f`) and reload `.bitmap`, matching `resetToRemoteBranch` and `MainSyncExecutor.resetToStartPoint`. Without `-f` a single tracked modification left by an earlier lane made the lane *abort* rather than halt; without the reload the following bit operation resolved "current lane" and per-component versions against the checkout the process started on. `restoreWorkspace` now cleans untracked files, as the main executor already did. A lane component absent from the default branch is materialized untracked, so a lane that halted after materializing left its files for the next lane's `git add -A` to commit under a `Bit-Lane-Head` trailer that does not describe them. `readBranchSyncState` no longer walks a 200-commit window. Merging the default branch into a long-lived lane branch brings in all of that branch's history, so the branch's own sync commit ends up arbitrarily deep and the window reported `lastSyncedHead: undefined` for a branch synced many times — making the planner halt with "branch has commits but no Bit-Lane-Head trailer", which was simply false. It now finds the commit with `--grep=Bit-Lane-Head:`, scanning every match (the grep matches the string anywhere in a message; the parser only accepts it at the start of a line, so a commit quoting a previous sync could otherwise mask the real one). The tip's message is returned as part of `BranchSyncState`, which removes the second `git log` `syncLane` used for the loop-guard probe, and the never-read `syncCommitSha` is dropped. Also: `main -> converged` now names an open sync PR, because the checkout ran on the sync branch and the default branch has not converged until that PR merges; a halt with a git host but no PR says so instead of falling silent; the merge-diverged snap message uses `SYNC_COMMIT_MARKER` instead of the bare literal, whose declaration now records that the action repo's event router duplicates the same string as its loop guard; and the marker plus `hasSyncMarker` are exported from the aspect index so a trigger can share the one definition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`listLanesToSync` enumerated only the default scope's remote lanes. A lane that
was merged, archived or deleted on bit.cloud is by definition absent from that
list, so its branch was never visited and `close-pr` — the action that closes the
PR and deletes the branch — could never fire for the one state it exists for.
Every lane that completed its life cycle left an orphan branch and an open pull
request behind, permanently.
The enumeration is now the union of the remote's matching lanes and the
lane-mapped branches on `origin` (`git ls-remote --heads`, `refs/heads/` stripped,
mapped back through `branchToLaneName`, skipping the default branch and
`mainSyncBranch`), deduplicated by lane name. A branch-only entry resolves
`laneHead === undefined`, which is already exactly the input `planLaneSync` turns
into `close-pr` — no action logic changed.
`listRemoteBranches` uses `ls-remote` rather than `git branch -r` so the answer
cannot be narrowed by this checkout's fetch refspec; a single-branch clone is
precisely the case where a stale lane branch would go unnoticed.
`listLanesToSync` now returns `{ lanes, errors }`. With two independent sources, a
failure in one must not discard the other: each failure becomes its own HALTED
line (so the run still exits non-zero) while the half that did enumerate is still
reconciled. Previously any failure skipped every lane.
`summarizeSync` throws `BitError` instead of `Error`. A halt is user-actionable —
resolve a conflict, remove a label — and bit reports a plain `Error` as an
internal failure, with a stack trace and a Sentry event at FATAL.
The e2e suite gains a two-lane `--all` scenario driving three runs plus a re-run:
both lanes import; lane A then halts on a real file conflict while lane B, after
it, still syncs; lane A is removed from the remote and the run reports
"Reconciling 2 mapped lane(s)", retires its branch and leaves lane B converged;
the re-run drops the retired lane entirely. The "2 mapped lane(s)" assertion is
the non-vacuous lock — only one of the two lanes still exists on the remote there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`isGitHubRemote` tested for `github.com` followed by `:` or `/` anywhere in the URL, so it claimed `https://gitlab.example.com/mirrors/github.com/acme/repo.git`. Because a claim is exclusive, that means the GitHub provider owns a remote it cannot serve: every PR operation for that repository is skipped, or — with a token in the environment — aimed at api.github.com for a repository hosted elsewhere. The authority is now parsed out and compared for equality, which accepts the scp-like, `ssh://` and `https://` forms (plus in-URL credentials and an explicit port) and rejects path-embedded mirrors, `mygithub.com`, `github.acme.com` and bare local paths. `parseGitHubRepo` stays unanchored, and stays guarded behind this test. Also drops the dead `PrInfo` re-export from `github-client.ts`, whose comment claimed every consumer imported it from there; every consumer imports it from `git-host-provider.ts`, which is where the contract lives. Adds a spec for `laneHeadFingerprint`: single 40-hex token (so it survives `parseLaneHeadTrailer`'s `(\S+)`), stable under reordering, sensitive to a moved head and to added/removed components, and independent of the lane object. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e2e timing Documents that the sync engine needs a complete checkout: full history (`actions/checkout` with `fetch-depth: 0`) and all branches. Shallow (`--depth=1`) and single-branch clones are unsupported, and the reason is given in each case — the sync state is a trailer on a commit that may be arbitrarily deep and the dev-commit count is a revision range, while `--all` has to enumerate every lane-mapped branch plus the default branch and `mainSyncBranch`. Describes which targets an `--all` run visits, now that the enumeration is the union of the remote's lanes and the lane-mapped branches on `origin`, and why the branch half is not redundant. The `--all` flag row mentions retiring a deleted lane's branch and PR, and the halt paragraph states that the isolation is unconditional. Adds a hand-estimated entry for `e2e/harmony/ci-sync.e2e.ts` to the e2e timing manifest that `split-e2e-tests.js` balances CircleCI nodes with. The suite measures ~4m locally with the new two-lane scenario. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The union enumeration added in 20408a0 visits every lane-mapped branch on `origin`. Under the documented defaults (`branchPrefix: ''`, `lanes: ['*']`) that is *every* branch, so an ordinary developer branch with no lane arrived at the planner with `laneHead === undefined` and `branchExists === true` — the exact input for `close-pr`, whose effect is `git push origin --delete`. An unmerged `origin/feature-x` was destroyed, silently, with exit 0. The `getDefaultBranchName` fallback to 'master' amplified it: on a repo whose default branch is `develop` or `trunk`, the real default branch was not excluded from the enumeration either. The absence of a lane is not evidence that a branch was ours. `LaneSyncInput` gains `wasLaneManaged` — true iff a `Bit-Lane-Head` trailer exists in the branch's own history, which the executor already has in `lastSyncedHead`, so no extra git call. `close-pr` now requires it; without it the branch is a no-op reported as "branch maps to no lane and has no sync history; ignoring". That also defangs the default-branch amplifier: an unexcluded default branch has no trailer, so it is ignored rather than deleted. The adopt-an-existing-branch path is unaffected — it requires `laneHead` to be defined, so the lane exists and the pairing comes from the mapping rather than from history. `readBranchSyncState`'s `--grep` walk now passes `--first-parent`. `git log` orders by commit date across all parents, so a `Bit-Lane-Head` commit that arrived through a *merge* — the default branch carrying another lane's sync commit, or an old sync branch merged in — outranked the branch's own sync commit by being newer. The adopted `lastSyncedHead` was then another pair's fingerprint, which never equals this lane's head, so every run read the lane as moved and re-planned work already done. First-parent traversal restricts the walk to this branch's own line of development, the only line whose sync commits describe this pair. It also strengthens the guard above: without it, an ordinary branch that merely merged main would have looked lane-managed, and therefore deletable. E2E: the two-lane scenario gains an ordinary `feature-x` branch (no lane, no trailer, unmerged work) asserted to survive both the first run and the run that deletes a real lane branch, and a final block that forges a newer trailer commit on a second parent via `git merge --no-ff` and asserts the branch's own sync commit still wins — `export-branch`, not `merge-diverged`, then a converged no-op on the re-run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…trailer The `wasLaneManaged` guard added in 07f8b43 asked "does this branch carry a `Bit-Lane-Head` trailer". That is satisfied by trailers the branch never earned: when a sync PR is squash-, rebase- or fast-forward-merged, its trailer lands on the DEFAULT branch's own first-parent line, so every branch cut from the default branch afterwards inherits one. Reproduced against the shipped compiled code: an ordinary `feature-x` forked after a rebase-merged sync commit was routed to close-pr and deleted, exit 0. A branch forked off a lane's sync branch inherits its trailer the same way. Ownership now needs two independent pieces of evidence, replacing the boolean with a `LaneOwnershipEvidence` union so the states are named and unit-testable (the boolean also duplicated `lastSyncedHead` and admitted contradictory inputs): - **attribution** — the sync commit's subject must name THIS lane. `buildSyncCommitMessage` writes the lane id there, so `parseSyncCommitLaneId` recovers it and it is compared against the lane being reconciled. This is what an inherited trailer cannot fake. - **reachability** — either that commit is not yet in the default branch (a live lane branch of ours), or the branch tip already is (nothing on the branch is missing from the default branch, so deletion cannot lose work). The four outcomes: `own-live` and `own-merged` close the PR and delete the branch; `own-superseded` — the PR was merged and then more commits were pushed — closes the PR and KEEPS the branch, because those commits are in no other ref; `inherited-or-none` does nothing at all. `close-pr` therefore carries a `deleteBranch` flag rather than always deleting. That also settles the residual "close-pr ignores hasDevCommits" concern: unmerged commits are exactly what `own-superseded` detects. `isAncestor` in git-ops answers reachability via the merge-base comparison, not `merge-base --is-ancestor`, whose answer is an exit code that simple-git's `raw` resolves rather than rejects — the same trap `isBranchBehindDefaultBranch` documents. An unanswerable comparison degrades to `inherited-or-none`: every other answer permits deleting a branch, and a failed merge-base is evidence of nothing. `BranchSyncState` now returns the located `syncCommit` (hash, message, trailer, subject lane id) instead of a pre-digested `lastSyncedHead`, so both the last-synced head and the ownership question are derived from one record. E2E: a new self-contained block walks one lane branch through all three owned outcomes and pairs it with the reviewer's repro — a sync-shaped commit forged on the default branch's own line naming another lane, with `feature-x` cut from that tip. It survives two `--all` runs including the one that legitimately deletes a branch, and a non-vacuity assertion proves the inherited trailer really is on its first-parent line. Docs: the over-claimed "positive evidence = a trailer" paragraph is replaced by the actual two-part rule with an outcome table, the `[lane]` flag row notes that a retired lane whose branch lacks its own sync history is a no-op, and `readBranchSyncState` records the known cost of `--first-parent` — a developer's `git pull` merge can leave the newest sync commit on the second parent, which plans `merge-diverged` instead of `export-branch` and converges with one round of churn rather than a wrong outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… reads the subject only
own-live meant the sync commit never reached the default branch — the PR was
never merged — so dev commits above it exist in no other ref: not on the lane,
not in the default branch. Deleting the branch there destroyed never-exported
work, contradicting the documented invariant ('deletes the branch unless it
holds unmerged commits'). own-live now splits on hasDevCommits: with dev
commits the PR is closed and the branch kept (same keep path own-superseded
uses); without them it is deleted as before.
parseSyncCommitLaneId dropped its /m flag: a sync-shaped line pasted into a
commit BODY satisfied attribution and could mark an ordinary developer branch
own-live. Only the subject line — which is where buildSyncCommitMessage writes
the lane id — may attribute a sync commit now.
Belt-and-braces at the one 'git push origin --delete' site: executeClosePr
unconditionally refuses to delete the default branch or cfg.mainSyncBranch
(isProtectedBranch), whatever the ownership evidence concluded.
e2e: the reconcile-cycle deletion scenario (whose tip was D2's unexported dev
commit) is now the own-live keep lock, including idempotent re-runs; the
two-lane deleted-lane scenario converges lane A first so its tip is a sync
commit and the genuine own-live deletion path stays covered. Docs: the
outcome table gains the own-live-with-dev-commits row and a note that kept
branches re-log close-pr on later runs until deleted by hand.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A lane is an org-global change set: it is *hosted* on one scope but may *contain* components from many, while "bit ci sync" maps one repository to one scope and reconciles a lane whole — fingerprint, materialize, snap and export every component on it. Mirroring a lane with foreign content would write another repository's components into this one, propose them in a PR nobody in their scope reviews, and export them from here. Stage 0 makes that impossible and makes the hosting/content distinction addressable: - Relevance/purity check in LaneSyncExecutor, after the lane and branch state are read and before anything is planned or written, so every trigger (bare lane argument, --branch, --all) is covered. A cross-scope lane is a legitimate thing to create, so the outcome depends on how this repo reached it: enumerated -> skipped and the run stays green; named explicitly -> refused with the reason, exit non-zero, no PR machinery; became cross-scope after its branch existed -> halted, labelled and commented, because that pair is mid-flight and can no longer converge. - "bit ci sync [lane]" accepts a scope-qualified id (other.scope/my-lane). The branch mapping stays keyed on the lane NAME; every bit-facing use — reading the lane, snap/export, and the id in the sync commit subject — uses the lane's real hostScope/name id, so attribution round-trips. --all enumeration is unchanged (defaultScope-hosted lanes and lane-mapped branches only). - Reserved-name guards now read the NAME part, and also refuse a lane whose branch would be the default branch or the main sync branch. Docs state the v1 boundary (content in one scope, hosting may differ), the three-way cross-scope semantics, and point at Stages 1-2 of the design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ver alias a branch
Review follow-ups on the Stage 0 cross-scope work.
- The mid-flight halt is now read after the PR, so `bit-sync-conflict`
suppresses it exactly like every other halt. Without that, a standing
became-cross-scope lane posted a fresh comment on the same PR on every
scheduled run and the documented way to silence it did nothing.
- Mid-flight attribution now requires REACHABILITY, not just a matching
subject: a merged sync PR leaves its commit on the default branch's own
first-parent line, so every branch cut from it afterwards carries one.
The claim is computed once (own-live) and reused for the lane-gone path,
which keeps that path's answer identical while making the new one honest.
The comment and docs claimed this immunity; now the code has it.
- Two lanes with the same name in different scopes map to one branch. If
the branch is another lane's live mirror, reconciling this one halts
instead of hijacking it — import-lane would otherwise materialize one
lane over the other's content under a trailer asserting the opposite.
Gated on this lane existing: with no lane there is nothing to write, and
close-pr already refuses without attribution, so halting there would turn
a foreign-hosted lane's branch met by name into a permanently red run.
- The reserved-branch guard moved into the per-lane reconciler. It was only
in the explicit path, and --all calls the executor directly, so a
`branches: {release: 'main'}` override would have force-checked-out the
default branch, committed and pushed.
- The refusal no longer claims "no branch was created" when one exists.
- The purity predicate states what it does not see: soft-deleted components
and updateDependents cascade entries, neither of which is ever
materialized, snapped or exported.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n halt The branch-aliasing halt added in 97319b3 called executeHalt unconditionally, so `bit ci sync <name> --dry-run` labelled and commented on a PR — and not even the refused lane's PR, but the one belonging to the lane that *owns* the branch. A flag that promises to write nothing froze a healthy lane's syncs until a human removed the label. Its sibling mid-flight halt guarded exactly this, three lines away. Both halts now go through one `haltOrReport`, so the guard cannot be present at one site and missing at the other. The comment posted on that PR is also wrong by default: the standard resolution steps say "bit lane import <lane>" naming the REFUSED lane, which is precisely the overwrite the halt just prevented. `executeHalt` takes an optional note that replaces them, and the aliasing halt supplies one that says whose PR this is, that its own lane is fine, how to resolve the collision, and not to run the usual steps. Comment/doc corrections this round falsified: - the branch-claim cost is per lane per run (up to 2N on --all), not per run; - the refusal no longer claims there is never a branch or PR; - crossScopeRefusal and crossScopeOutcome docblocks carried pre-I2 wording; - assessBranchOwnership's failure warning said "rather than retired" though it now also gates two non-retirement halts; - docs state whose PR the aliasing halt annotates, and the accepted residual: an own-superseded branch is nobody's live mirror, so a same-named lane will still plan from the other lane's trailer. Pre-existing; tightening it means attributing on subject alone, which is what labels developer branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The claim test lower-cases and port-strips its host; the parse was case-sensitive and port-blind, so a GitHub.com or :443 remote was claimed exclusively yet parsed to nothing — the whole run silently degraded to PR-less mode. The parse is now case-insensitive, authority-anchored (never a path segment of another host), and strips :digits only under a scheme — in scp form the colon starts the path and an all-digits owner is a legal GitHub username.
|
Code review by qodo was updated up to the latest commit 3d1b72c |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
238 rows -> 182 across the same invariant families: repetitive validation, selector-guard, parse and argv rows become one iterating test each; facets of a single behavior collapse into one it. Every invariant that locks real behavior is retained (planner decision table, ownership evidence, authored-commit predicate, reset-guard shapes, runner-behavior rows, identity and token precedence, YAML parse, scope-qualified routing).
One cell per reconcile run: facets of a single `bit ci sync` invocation are now expects inside one `it` rather than a describe of one-assertion cells, and the "re-running right after X -> converged no-op" blocks fold into their parent as a second run at the end of that cell. Scenario B (`re-run with nothing moved`) stays standalone as the suite's idempotency cell. Repeated fixture idioms move into ci-sync-support.ts: setupSyncWorkspace, createLaneWithSnap, syncRun, seedSync. Deleted: the two flag-combination cells (`--all --main`, `--init --dry-run`) — pure argv validation with no bit/git side effect, already locked by sync-orchestrator.spec.ts. 161 cells -> 41; 2523 -> 1682 lines.
…onfig
`ensureGitIdentity()` persisted `user.name`/`user.email` into the repository's
git config whenever they were unset, so the developer's own later commits in
that repo were authored as bit-sync[bot].
`resolveGitIdentity()` replaces it: it reads the config to honour a configured
identity but never writes, keeping the precedence (configured > GIT_USER_NAME /
GIT_USER_EMAIL > bit-sync[bot]). `gitWithIdentity(args)` prefixes
`-c user.name=… -c user.email=…` so the identity applies to that invocation
only; `commitWithIdentity(message, { extraArgs })` is the commit form.
Every authoring path moves over: lane-sync-executor.commitAllAndPush,
main-sync-executor's drift commit, and catchUpWithDefaultBranch's `git merge`
(a non-fast-forward merge writes a merge commit, so it needs the identity too).
…e ci.docs.mdx ref-name.ts folds into sync-config.ts and bitmap-state.ts into sync-state.ts (four files fewer, specs merged with them), and the aspect docs drop narrative duplication while keeping every config option.
|
Code review by qodo was updated up to the latest commit 1dbeefa |
| ); | ||
| } | ||
| } | ||
| await run(startPoint ? ['checkout', '-f', '-B', branch, startPoint] : ['checkout', '-f', branch]); |
There was a problem hiding this comment.
This line is running also for dry-run? (also the next line). if so, a user might loose local (uncommitted) changes unexpectedly
There was a problem hiding this comment.
Good catch — you're right, and thank you. Fixed in 221cf6c.
I audited every write reachable under dryRun: true:
- Main-scope path — the offending one.
syncMainrunscheckoutPristine(checkout -f -B+ scoped clean), thencatchUpWithDefaultBranch(a local merge commit), thenbit checkout head, all before its dry-run return — because main drift is computed as a diff, so the tree has to be written to learn the answer.restoreWorkspacerewinds to HEAD, so anything uncommitted was already gone. - Lane path — clean. The dry-run early return precedes every tree move (
checkoutPristine/materializeLaneare only reachable from the three action executors behind theswitch). Everything before it is reads plus lane-object imports into.bit. - Orchestrator — reads only;
--initis already refused together with--dry-run.
Fix: a dry run now refuses when the working tree has uncommitted changes (tracked modifications or untracked non-.bit/node_modules files), naming up to 10 offending paths and telling the user to commit or stash — instead of discarding them. With a clean tree the write-then-restore loses nothing, and that reasoning is recorded at the site. The guard sits in the orchestrator's existing status block, so it fires for every target shape (uniformly, including lane-only dry runs that provably write nothing — the promise 'a dry run never discards' then holds without the user having to know which shape writes).
Covered by unit rows plus an e2e cell asserting --main --dry-run over an uncommitted modification exits non-zero and leaves the modification intact.
| try { | ||
| // `:refs/heads/<branch>` rather than `--delete <branch>`: the full delete refspec means the | ||
| // branch name can never be read as an option however it was configured. | ||
| await git.push(['origin', `:refs/heads/${branch}`]); |
There was a problem hiding this comment.
This command deletes the branch without checking if it was progressed in the remote. Can be risky because every input to the deletion decision (tip-is-sync-commit, no-dev-commits, ownership) is computed from origin/* refs fetched once per run (fetchOnce, :1097).
In case like --all it can take some time until completing one lane and moving to the other, which, in the meanwhile, things can change.
Better to refetch at this point. or use a flag that "--force-with-lease=:"
There was a problem hiding this comment.
Right on both counts — fixed in f3cb357 with both belts you suggested.
- Re-verify.
readBranchSyncStatenow also returns thetipShait read every ownership input from. Before anything (including the PR close),executeClosePrre-fetches just that branch (git fetch origin +refs/heads/<b>:refs/remotes/origin/<b>) and compares. If the tip moved — or cannot be re-read — the outcome downgrades to{deleteBranch:false; keepReason:'tip-advanced-during-run'}, so the PR comment and the run summary both say the branch was kept because it advanced. - Lease. The delete itself is now
git push origin --force-with-lease=refs/heads/<b>:<sha> :refs/heads/<b>— the server refuses a racing update rather than losing it. Argv order verified against a real bare repo (git 2.39.5); the full:refs/heads/<b>refspec is kept so the branch name can never be read as an option. A lease rejection is classified and reported as the same 'kept' outcome, never a crash.
One thing the e2e taught us: the server-side refusal wording differs from git's client-side stale info (cannot lock ref … is at X but expected Y / [remote rejected]), so the predicate matches both — a stale info-only match would have fallen through as 'left in place' instead of 'kept'.
Coverage: an e2e cell races the window directly (a pre-push hook advances the branch in the bare repo while the delete is in flight) and asserts the branch survives with the racing tip and the summary explains why, plus unit rows for the argv, the moved/unreadable tip paths, and both refusal wordings.
The main-sync path computes drift by diff, so it force-checkouts the sync branch and cleans untracked files BEFORE the dry-run return — a dry run in a dirty tree destroyed uncommitted tracked changes and untracked files, which "--dry-run writes nothing" forbids. Refuse the run up front instead, naming the paths; with a clean tree the write-then-restore loses nothing. The lane path's dry-run early return already precedes every tree move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tip-is-sync-commit, dev commits and ownership are all read from refs fetched once per run (fetchOnce); an --all run can spend minutes on earlier lanes, so a branch may have advanced by the time close-pr deletes it. Two belts: re-fetch that one head and refuse unless the tip is still the sha the evidence was read from, then delete under --force-with-lease on that sha so the server refuses a racing update. Either refusal is a "kept" close-pr outcome, never a crash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dry-run early return built the summary line from the action TYPE only, with no HALT_SUMMARY_PREFIX, so summarizeSync could not see a planned halt and the run exited 0 on a plan that needs a human. Report the same prefix and reason the real run would. Refusals already return before planning, prefix included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bit lane names admit `$` and `!`, so the copy-pasteable commands in the halt PR comment shell-expanded when pasted. Single-quote every interpolated value, escaping a quote inside one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every error/halt (chalk.red) and warning (chalk.yellow) line in the two executors now goes through the shared formatWarningSummary. No message text changed. chalk.blue progress detail and chalk.green per-step success detail have no faithful primitive and stay as they are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review by qodo was updated up to the latest commit a69cf60 |
…rence The local/file transport words a refused lease as 'incorrect old value provided' — a third wording the predicate missed, so CI reported a raced delete as a failed one instead of a kept branch. The cli reference picks up the --dry-run help sentence the dirty-tree refusal extended.
|
Code review by qodo was updated up to the latest commit 60aff5b |
…and at its source The default branch is derived from the remote, not from validated config, so a name like "-x" could reach 'git checkout' as a flag. checkoutPristine now asserts the name before building any argv (every caller inherits the guard), and getDefaultBranchName validates each candidate — an unusable answer falls through to the next source rather than into the argv.
|
Code review by qodo was updated up to the latest commit 997bbbd |
The lane grammar permits '-', so a developer branch called '-x' mapped to a valid LANE name while the branch mapping then asserted on it — turning one such branch into a halt that failed the whole --all run. syncableLaneNameForBranch now round-trips: a mapped lane name counts only when its derived branch name is usable in a git invocation.
| /** | ||
| * A file's content at a git ref, or undefined when it isn't there. simple-git's `raw` can resolve with | ||
| * empty output instead of rejecting on a missing path; `:./` keeps the path cwd-relative in case the | ||
| * workspace is a subdirectory of the repo. | ||
| */ | ||
| async function readFileAtRef(revision: string, filePath: string): Promise<string | undefined> { | ||
| try { | ||
| return await git.raw(['show', `${revision}:./${filePath}`]); | ||
| } catch { |
There was a problem hiding this comment.
1. Wrong .bitmap ref path 🐞 Bug ≡ Correctness
readBranchSyncState() reads the committed .bitmap via git show origin/<branch>:./.bitmap, but it never prefixes the workspace’s repo-relative subdirectory. In repositories where the Bit workspace is not at the repo root (explicitly supported by --init and orchestrator comments), this can make .bitmap look “missing”, which then makes the lane planner treat the branch as having no state and may incorrectly choose import-lane for an existing mirrored branch.
Agent Prompt
### Issue description
`sync-state.readFileAtRef()` uses `git show ${revision}:./${filePath}` while callers pass `filePath` as `BIT_MAP` (`.bitmap`). This does not incorporate the Bit workspace’s repo-relative directory, so in non-root workspaces the `.bitmap` at the branch tip may be read from the wrong location (or not found), causing `branchState.bitmap` to be `undefined` and the planner to select actions as if the branch has no sync state.
### Issue Context
This PR explicitly supports workspaces that are subdirectories (init scaffold `ws-dir`, orchestrator mentions workspace may be subdirectory). The sync logic must read the `.bitmap` that is committed **at the workspace path**, not necessarily at repository root.
### Fix Focus Areas
- scopes/git/ci/sync/sync-state.ts[155-166]
- scopes/git/ci/sync/sync-orchestrator.ts[223-226]
### What to change
1. Compute the workspace’s repo-relative prefix once (e.g. via `git rev-parse --show-prefix` or by combining `gitRepoRoot()` with `workspace.path` to derive a repo-relative path).
2. Pass that prefix into `readBranchSyncState()` / `readFileAtRef()` and use it when building the `rev:path` argument, e.g. `${revision}:${prefix}${filePath}` where `prefix` is `''` at repo root or like `packages/my-workspace/`.
3. Add/adjust unit tests in `sync-state.spec.ts` to cover both root and subdirectory workspace cases (ensure `git show` arguments include the prefix).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const claimants = remoteUrl ? providers.filter((provider) => provider.matchesRemote(remoteUrl)) : []; | ||
| if (claimants.length) { | ||
| const configured = claimants.find((provider) => provider.isConfigured(remoteUrl)); | ||
| if (configured) return { provider: configured }; |
There was a problem hiding this comment.
2. Ambiguous host provider pick 🐞 Bug ☼ Reliability
selectGitHostProvider() returns the first configured provider among those that matchesRemote(), even if multiple configured providers claim the same remote. This makes selection order-dependent and can route PR operations through the wrong provider, despite the function’s documented assumption that claims are exclusive.
Agent Prompt
### Issue description
When multiple providers claim the same `remoteUrl` and more than one is configured, `selectGitHostProvider()` picks the first configured claimant via `.find(...)`. This is order-dependent and conflicts with the documented “claim is exclusive” routing rule.
### Issue Context
Providers are registered via a public slot (`ci.registerGitHostProvider(...)`). In real deployments, it’s plausible to have overlapping `matchesRemote()` implementations (e.g., a generic GitHub provider plus an org-specific wrapper) or accidental double-registration.
### Fix Focus Areas
- scopes/git/ci/sync/git-host-provider.ts[46-85]
### What to change
1. When `claimants.length > 0`, compute `configuredClaimants = claimants.filter(p => p.isConfigured(remoteUrl))`.
2. If `configuredClaimants.length === 1`, return it.
3. If `configuredClaimants.length > 1`, return `{ reason: "...too ambiguous..." }` (or throw a `BitError` that instructs users to remove/disable one provider).
4. Add a unit test in `git-host-provider.spec.ts` covering: two claimants, both configured => ambiguity (no provider selected).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit d38e892 |
What this PR adds
This PR adds one command:
bit ci sync. The command keeps Bit lanes and git branches equal. It works in both directions. A lane on bit.cloud becomes a branch and a pull request on the git host. A commit on that branch becomes a snap on the lane.The command completes the
bit ciset.bit ci prandbit ci mergemove changes from git to Bit.bit ci syncalso moves changes from Bit to git.Why teams need this
Use case 1: git is the source of truth. Some organizations must review every change in a pull request. Their policy blocks lane merges on bit.cloud. With this command, a lane export creates a pull request. The team reviews the pull request in GitHub. When the pull request merges,
bit ci mergereleases the components. The audit trail stays in git.Use case 2: git is a compliance mirror. Some teams merge lanes on bit.cloud. They also keep a git repository for audit. When the scope moves ahead of the repository, the command opens a sync pull request. A reviewer approves it. The repository then shows the true state of the scope.
How to use it
Triggers decide when the command runs. They never decide what it does. A webhook, a push, a cron job, or a person can start it. The result is the same. A converged pair is a no-op. For a fully working GitHub setup,
--initwrites two workflow files and prints a checklist.How a sync works
The command reads three facts for each lane and branch pair:
.bitmapfile at the branch tip. Bit writes this file. It records the lane pointer and each component version. No extra state store exists.A pure planner compares these facts and picks one action: import the lane onto the branch, export the branch commits to the lane, merge a diverged pair, close a retired pair, skip, or halt. The executor performs the action with Bit APIs and plain git pushes. The command never force-pushes.
Safety rules
bit-sync-conflict, and writes the recovery steps as a comment. Remove the label to resume..bitmapat the tip must name the same lane. The commits must be safe to delete. A developer branch never qualifies.Other changes in this PR
scopes/git/ci/git.tsno longer runsgit clean -fwhen the module loads. This side effect ran on everybitinvocation and could delete untracked files.bit ci mergeand the sync paths now stage files with an exclusion list. They can no longer commit the local scope directory (.bit/) in a workspace without a.gitignore.ci sync.Tests
Known follow-ups
bit ci prhas one cold-runner gap at its lane-switch probe. The sync paths are fixed. The shared path needs its own change.BitMapwould benefit from aparse(content, { defaultScope })API. The current no-filesystem entry point needs two placeholder arguments.