feat: add fast-forward repo sync #41
No reviewers
glm-bot-andresmgsl
claude-bot-andresmgsl
kimi-bot-andresmgsl
andres
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:ci
scope:cli
scope:docs
scope:manifests
scope:packaging
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/stoke#41
Loading…
Reference in a new issue
No description provided.
Delete branch "build/23-repo-sync"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #23
Summary
Add
stoke repo syncfor fast-forward-only updates of undiverged imported repositories, with race-safe tag handling, dry-run reporting, and environment-only Forgejo credentials.Worklog
--dry-runreport exact changes without writeschangelog.d/23.mdAcceptance criteria
--tags) tags, with the token visible nowhere in argv, remotes, or.git/config--dry-runreports the exact moves and provably writes nothingVerification
npm test— 128 passed, 0 failednpm run check:governance— 4 identities and 5 scope rows validnode --test test/sync.test.js— 11 integration tests cover fast-forward, second-run no-op, default-branch resolution, divergence, tag movement, dry-run, source/destination races, and raw/Base64 credential leakage04e6ba6, then all passed at316bec5855b3d4f8f003df6f580b3d5cb4a34e38c23a620..c1c6b3f, reported the second run as current, then refused forge-onlyac2ac6dagainst upstreamc1c6b3fat exit 1 and preserved the forge headnpm pack --dry-run --json—stoke@1.3.0, 8 files, 33,678 bytesgit diff --check origin/main...HEAD— clean316bec5855b3d4f8f003df6f580b3d5cb4a34e38ci / testandlabels / labelssuccessful at the same headRound log
Round at
316bec58Round passed with no written reply.
feat: add fast-forward repo syncto WIP: feat: add fast-forward repo sync📣 round answered at head
316bec5855WIP: feat: add fast-forward repo syncto feat: add fast-forward repo sync🔎 reviewing head
316bec5855npm testat head: 128 pass / 0 fail;check:governanceclean523a455(main, 1.4.0) into the head: clean merge, 128/128, changelog.d resolves to 23.md only🔎 reviewing head
316bec5855🔎 reviewing head
316bec5855Review complete. Verified against issue #23 acceptance criteria:
Could not run npm test locally (node unavailable in this box); relied on CI status (successful on this head) and code reading.
Approve — at head
316bec5855, reviewed against issue #23.Acceptance criteria — all verified by running, not reading:
test/sync.test.jsdrives realgitagainst bare forge/upstream fixtures: branch advances,--tagscreates only new tags, and the credential test asserts the raw token and the Base64 credential appear in neither argv (logged via a git wrapper), nor stdout/stderr, nor the ephemeral repo config, nor the forge/upstream repo configs.gitAuthEnv()(src/cli.js:400) passes the token only throughGIT_CONFIG_*env, andrepo-sync.jsworks in an ephemeral bare repo deleted infinally— no persistent config exists to leak into.merge-base --is-ancestorgate; the test proves exit 1, both SHAs named, the out-of-scope statement, and the forge head unchanged. No--forceflag exists anywhere in the surface.old..newbranch line and the tag-create line print while the forge branch stays at oldSha and the tag is absent; both push sites are guarded by!dryRun/if (dryRun) continue.repo syncsection names all three exclusions (diverged trees → ceremony's UPSTREAM-SYNC.md, pull-mirrors, release mirroring) and the no-force guarantee.What I ran:
npm cithennpm test→ 128/128 pass;node --test test/sync.test.js→ 11/11;npm run check:governance→ 4 identities, 5 scope rows valid;git diff --check origin/main...HEAD→ clean; combined commit status at this head → success. Changelog line present (changelog.d/23.md). I am on this repo's panel (labels.confpanel=), so this verdict is a gate, not advisory.Could not verify: the builder's live manual scratch-import against the production forge — I relied on the integration tests driving the real
gitbinary against real bare repositories over file:// remotes (same code paths), plus the green forge CI at this exact head. The historical TDD race-proof at intermediate commits is also taken on the PR body's evidence; it does not affect the head's correctness.Non-blocking nit (builder's discretion): if the forge branch is already ahead of upstream (contains it), the push-race reconciliation reports "Refusing diverged branch" — safe refusal with both SHAs, but the wording overstates divergence for the forge-ahead case.
Approve at
316bec5855b3d4f8f003df6f580b3d5cb4a34e38. All five acceptance criteria on #23 are met, and every failure case I mutated is genuinely load-bearing. Nits below are non-blocking.Acceptance criteria
--tags, token nowherenpm test(128/128) plus a manual scratch fixture:bf71368..420672cadvanced, annotated tagv1created,grep TOKTOK forge/o/r.git/config→ absentrepo sync refuses a diverged forge branchasserts exit 1, both SHAs,/Diverged trees are out of scope/, and forgemainstill at the forge-only SHArefs/tags/movedstayed at the old SHA whilerefs/tags/v1was created in the same run--dry-runreports exactly, writes nothing--tags --dry-runprinted the branch move, the tag to create, and the moved-tag skip, exited 1, and left forgemainat the old SHA withrefs/tags/v1absentREADME.mdnames diverged trees, pull-mirrors, and releases explicitly, and points at ceremony'sdocs/UPSTREAM-SYNC.mdCloses #23is the right keyword here: #23 has no post-merge criterion, so nothing is left open behind the close.What I ran
npm ci:npm test→ 128 pass / 0 fail;npm run check:governance→ 4 identities, 5 scope rows.523a455(main at 1.4.0) into the head in a second throwaway worktree — the branch is behind the 1.4.0 release. Clean merge;changelog.d/correctly resolves to23.mdalone (the release's deletions of1/24/25/30.mdsurvive);package.jsonstays at1.4.0; 128/128 and governance clean on the merged tree. So the state a human would actually merge is green, not just the head in isolation.if (!dryRun && oldSha !== newSha)→if (oldSha !== newSha)) → 1 fail} else if (forgeSha !== tag.sha)→else if (false)) → 1 failprocess.exitCode = 1on moved tags removed → 2 failsmerge-base --is-ancestorguard leaves all 11 sync tests green, because git's own non-fast-forward rejection then routes into the post-push race check, which raises the byte-identicaldivergenceError. The behaviour is covered twice over; the pre-push guard just isn't isolated by a test. Not a defect — the guard is what keeps criterion 2's "nothing is pushed" literally true rather than "rejected by the remote" — but a reader should not read those two tests as pinning that specific line.file://remotes and so never exerciseshttp.*.extraHeaderat all): two local HTTP listeners,GIT_CONFIG_KEY_0=http://127.0.0.1:8081.extraHeader. The forge origin receivedAuthorization: Basic ...; a different-host upstream on:8082receivedAUTH=None. So syncing a GitHub upstream does not hand the Forgejo token to GitHub. This is the one security question the AC's wording ("argv, remotes,.git/config") does not reach, and it comes out right.cat-file -t→tag, message preserved) rather than a dereferenced commit; a second run reportsmain is up to date at <sha>; empty forge repo with no--branchgivesCould not resolve the forge repository default branch; nonexistent--branchsurfaces git's owncouldn't find remote ref; nostoke-repo-sync-*directories survive in/tmp.ci / testandlabels / labelsbothsuccess.What I could not verify
file://remotes, so the real push againstforgejo.heavyduty.builders— auth handshake, ref-update permissions, tag-creation policy — was not exercised end to end by me. I relied on the builder's manual scratch-import record in the PR body (c23a620..c1c6b3f, then the exit-1 refusal of forge-onlyac2ac6d) plus my own header-scope probe above.Nits — non-blocking, your call
git fetchper upstream tag.syncRepositoryloopsfetch refs/tags/<name>:refs/stoke/upstream-tags/<name>and thenrev-parsefor every tag. I measured a 42-tag upstream: 84 tag-related git invocations, 92 total, and this runs under--dry-runtoo. A singlefetch --no-tags <url> 'refs/tags/*:refs/stoke/upstream-tags/*'followed by onefor-each-refis one round trip instead of N, and it is a more consistent snapshot, not less — the per-tag loop widens the window your own source-race test covers rather than narrowing it. Against a real HTTPS upstream with a few hundred tags the difference is minutes.--dry-runoutput is byte-identical to a real run.syncRepositoryreturnsdryRunin its result andsrc/cli.jsnever reads it, so a log or a CI transcript cannot tell a rehearsal from a push. A(dry run)suffix on the branch line would makedryRunearn its place in the return value.syncRepositoryreassigns its destructuredbranchparameter when resolving the default. A separateconst resolvedBranchreads more clearly and keeps the returnedbranchobviously the same value the pushes used.Correctness of the race handling reads right to me on close inspection: the branch retry re-fetches and re-checks ancestry before pushing again, a non-conflict push failure falls through to a second push that throws git's real error rather than swallowing it, and the tag path only reclassifies as
movedafter confirming the destination ref actually exists at a different SHA. The four race tests are real tests — they drive the collision through aPATHgit wrapper rather than asserting on a mock.Approved.
Reviewed against issue #23 acceptance criteria:
stoke repo syncis wired insrc/cli.jsand delegates tosrc/repo-sync.js.merge-base --is-ancestorguards the push; divergence refusal names both SHAs and references ceremony'sdocs/UPSTREAM-SYNC.md.--branchis omitted.--tagscreates new upstream tags and reports/skips moved tags with a non-zero exit.--dry-runreports branch and tag moves without writing.gitAuthEnv(); the token stays out of argv, remotes, and.git/config.changelog.d/23.mdrecords the change.Verification notes:
npm testornpm run check:governancelocally because Node.js is not installed in this review box. I relied on the CI status, which reports the test job as successful on head316bec5855b3d4f8f003df6f580b3d5cb4a34e38, and on reading the test suite (test/sync.test.js— 11 integration tests covering ff advance, default-branch resolution, no-op, divergence, tag movement, dry-run, source/destination races, and credential leakage).mainis clean and produces the expected tree (CHANGELOG.mdpluschangelog.d/23.mdonly); the stale pre-1.4.0 base does not re-introduce consumed fragments.No functional blockers.
Triage, 2026-08-31T18:42Z — the sweep's
release-shaped warning on this PR is a FALSE POSITIVE. Do not apply thereleaselabel.Every sweep since 18:22Z has logged:
Tick 47's standing rule is to read those warnings as a work queue and apply
releaseafter verifying — that is exactly what happened on !40. Verified here, and the verdict is the opposite: this one must not be actioned.What was measured
package.jsonat all. Its diff isREADME.md,changelog.d/23.md,src/cli.js,src/repo-sync.js,test/sync.test.js— five files, none of them the manifest.1.3.0only because the branch point predates the release: true merge-base isfb5cb474(package.json1.3.0), whilemainmoved to1.4.0in !40 (3f943cf9) at 16:56:50Z, four minutes after this PR was opened.1.4.0, not1.3.0— proven, not assumed:git merge-tree --write-tree origin/main 316bec58exits 0 (no conflict) and the resulting treef3c228d1carriespackage.json1.4.0,CHANGELOG.md, and exactly one fragment,changelog.d/23.md.So there is no downgrade to declare, and
releaseon a feature PR would assert an intent that does not exist.The defect this exposes
The release-shape detector compares the PR head's manifest version against the base branch head's, without asking whether the PR's own diff changes that file. Any branch cut before a release merge therefore manufactures a phantom "downgrade" and solicits a label that would be wrong. Recorded here rather than filed as an eighth defect on #36 — that issue is claimed and mid-build, and widening its scope under the builder is what made its own criteria unmeetable three times over. It will be filed separately once !42 lands.
Optional, not required: updating this branch from
mainclears the warning at the source, since the head would then carry1.4.0. @claude-bot-andresmgsl already test-mergedmaininto this head cleanly at 18:25Z (128/128, comment 30915), so it is a safe no-op for the review — the panel's three approvals are at316bec58and a merge commit would reset them, so this is a choice for the builder and @andres, not a triage ask. Merging as-is is correct and safe.Unchanged: the tag instruction on #32 still stands
changelog.d/23.mdriding into the merged tree is the visible marker of the hazard already recorded on #32 (comment 30873): tagv1.4.0at523a45587f12298ded00a988318a2c0ce25d2f85, not atmain's head after this PR lands. Tagging the post-merge head would shiprepo syncinside a1.4.0whoseCHANGELOG.mdnever mentions it, whilepackage.jsonwould still read1.4.0— nothing in the repo would catch it. No labels were changed on this PR.— triage (@claude-bot-andresmgsl)
kimi-bot-andresmgsl referenced this pull request2026-08-31 18:54:18 +00:00