can you take a look at:

Claude Code·Opus 4.8[1m]·Soph·2mo ago·49hr 9min·1 Checkpoint·2 file changes·+234/-2·78.1K tokens

can you take a look at:

Here's the git-sync issue, structured to mirror the entiredb one.

git-sync: batched bootstrap sends pack-less ref creates that some servers reject

Symptom

During a batched bootstrap, git-sync finalizes a "subsumed" branch by sending a ref-create command with no packfile. Against a receive-pack implementation that expects a pack for any non-delete command, this fails:

The bootstrap dies mid-run, leaving the target half-populated. First observed mirroring github.com/JUMPTOON/app → entiredb prod aws-us-east-2, 2026-06-15 07:36 (trace 433bad8f4ef654ac651f868ab3c2fa3e): ~15 batch pushes succeeded, then the subsumed-ref create EOF'd.

Root cause

In the batched bootstrap, the subsumed-branch path issues a ref-only push:

PushCommands → sendReceivePack(ctx, conn, req, nil, …) with packData=nil, so the request body is the command pkt-lines + flush and nothing else (internal/gitproto/push.go:585, body assembly at push.go:293-296).

That's valid git when the objects already exist, but it relies on the server tolerating a pack-less create. entiredb (and likely others) treats every non-delete as "a pack will follow" and errors reading the absent header. The interop contract is ambiguous and git-sync is on the fragile side of it.

Suggested fix

On the subsumed-ref / pack-less create path, send a valid empty packfile (12-byte header + trailer, 0 objects) instead of no pack, so servers that unconditionally read a pack header are satisfied. (Cheap, and strictly more compatible.) Alternatively, gate the pack-less optimization on a detected server capability.

Evidence

  • Trace 433bad8f4ef654ac651f868ab3c2fa3e — batched bootstrap, EOF on create subsumed branch ref for refs/heads/docs-rules.
  • Counterpart server-side cause (entiredb): commandsNeedPack returns true for any non-delete (server/githttp/receive_pack_connectivity.go:66), then io.ReadFull of the 12-byte header EOFs on the empty body (objstore/packfile_fanout.go:44).

Related (separate, larger git-sync issue): replicate strategy can't batch, so a half-bootstrapped repo never recovers

Why it matters

Once the bootstrap above fails partway, the target is non-empty, so runReplicate routes to executeReplicate instead of bootstrap (internal/syncer/syncer.go — replicateCanBootstrap false). The replicate strategy streams one pack for all updates with no subdivision:

For JUMPTOON/app the resulting pack is >10 GiB and the target rejects it (push rejected: body exceeded size limit 10737418240), 708× over the window — a permanent wedge.

Two gaps

  1. No body-limit fallback in replicate. The body exceeded size limit detection + checkpoint-batched relay (autoTargetMaxPackBytes, bodyLimitPattern) lives only in the bootstrap strategy (internal/strategy/bootstrap/bootstrap.go:33,1380). Replicate has none — it just returns push target refs: %w and fails.
  2. TargetMaxPackBytes isn't reachable via the public API. Options/SyncRequest/SyncPolicy don't expose it; internalbridge/config.go:75 hardcodes it to 0. So a caller (mirror-pipeline) can't proactively cap pack size even if it wanted to.

Suggested fix

  • Give the replicate strategy the same body-limit detection + checkpoint-batched relay the bootstrap path has (or route replicate's create/update plans through executeBatched).
  • Expose TargetMaxPackBytes on the public SyncPolicy/Options and thread it through SyncConfig so callers can set it up front (~5 GiB) and avoid the wasted 10 GiB round-trips.
  • Consider making bootstrap resumable after a mid-batch failure rather than stranding partial state (the scaffolding exists — BootstrapResumeIndex, temp refs — but it didn't re-engage here).

I kept the pack-less-create bug as the primary issue (it's the direct counterpart to the entiredb one) and split the replicate-batching work out, since it's a bigger, separable change. Want me to file these as two GitHub issues in git-sync, or fold them into one? I can verify the exact bootstrap.go line numbers in the real git-sync repo (I've been reading the vendored copy) before filing.

2mo ago·2m·3 calls

I dug into the actual git-sync source. Your report holds up well — line numbers are accurate and the mechanism is correctly described. Here's the verification, with the one correction worth fixing before you file.

Primary bug: pack-less subsumed-ref create — ✅ confirmed

internal/strategy/bootstrap/bootstrap.go:323-337 (the report didn't pin a line; here it is):

The subsumed field (struct doc at lines 121-126) explicitly says it "skips the commit-graph fetch, the pack fetch, the temp ref, and the pack push — emitting only a single ref create command." That matches your error string exactly.

internal/gitproto/push.go — your line cites verified:

  • push.go:585 — PushCommands calls sendReceivePack(ctx, conn, req, nil, …) with packData=nil. ✅
  • push.go:293-296 — body assembly is bytes.NewReader(header.Bytes()), and the pack is appended only if packData != nil. So with nil it's command pkt-lines + flush, nothing else. ✅

Empty-pack suggestion: confirmed there is no empty-pack construction anywhere in git-sync today (only packfile.NewEncoder for non-empty packs in PushObjects). So your suggested fix is genuinely new code, not a toggle of something existing — accurate framing.

Related issue: replicate can't batch — ✅ confirmed, with one correction

  • replicate.go:62-74 — one FetchPack, one PushPack, on failure returns fmt.Errorf("push target refs: %w", err). No fallback/retry/batching. ✅ (note it does call gitproto.LimitPackReader(packReader, p.MaxPackBytes), but that's a source-side read cap, not the target body-limit relay.)
  • Body-limit machinery is bootstrap-only: bodyLimitPattern at bootstrap.go:33, autoTargetMaxPackBytes at :1380, isTargetBodyLimitError at :1430, executeBatched at :255, and the fallback wiring at :180-189. Replicate has none of it. ✅
  • Routing in syncer.go: replicateCanBootstrap/allAbsent check at :916, bootstrap path at :935, executeReplicate at :982. Non-empty target → executeReplicate. ✅
  • BootstrapResumeIndex resume scaffolding exists at bootstrap.go:348 (temp refs + re-plan on hash-in-chain). ✅

⚠️ Correction — TargetMaxPackBytes is reachable, just not on the stable API

The report says it "isn't reachable via the public API." That's only true for the stable surface. It's actually exposed in two places:

  • unstable/client.go:43 — AdvancedOptions.TargetMaxPackBytes int64
  • CLI flag --target-max-pack-bytes (cmd/git-sync/bootstrap.go, syncplan.go)

What the report correctly identified is that internalbridge/config.go:75 (SyncConfig) never threads it through — it's left at 0 in the stable bridge layer (it does hardcode MaterializedMaxObjects nearby but not this). So the precise claim is: the stable SyncPolicy/Options/SyncRequest don't expose it and the bridge hardcodes 0 — not that it's entirely unreachable. mirror-pipeline going through the stable bridge can't set it; going through unstable or the CLI can.

I'd reword that bullet to "not exposed on the stable public API; internalbridge/config.go:75 leaves it 0" so a reviewer doesn't immediately point at unstable/AdvancedOptions and dismiss it.

On filing

Two issues is the right split — the pack-less create is a tight, self-contained fix; the replicate-batching work is a real refactor. Want me to:

  1. Draft the two GitHub issue bodies (with the corrected TargetMaxPackBytes wording and these verified line refs), and/or
  2. Go ahead and implement the primary fix — the empty-pack path is small and I can also add the server-capability gate as the alternative?

let's first fix the empty-pack

2mo ago·40m·39 calls

Lint clean, all tests pass. Here's a summary of the empty-pack fix.

What changed

internal/gitproto/push.go — the package-level PushCommands now attaches a valid empty packfile whenever the request contains a non-delete command:

Plus a new helper:

This fixes the subsumed-branch finalize path (bootstrap.go:324 and :643) and any other create/update routed through PushCommands — the request body now ends with a 12-byte header + checksum trailer instead of nothing, so a receive-pack that unconditionally reads a pack header (like entiredb) no longer EOFs.

Two correctness details I made sure of

  • Delete-only pushes stay pack-less. PushCommands is also used for temp-ref deletes (bootstrap.go:375/649, replicate.go:78). Gating on hasUpdates means deletes still send no pack, as git requires — I added a test asserting the body contains no PACK.
  • Object format is honored. git-sync supports SHA-256 (there's a convert-sha256 command), and a hardcoded SHA-1 empty pack would produce a wrong trailer for a SHA-256 target. The helper reads the advertised object-format capability and uses go-git's hash.New so the trailer is 20 or 32 bytes accordingly.

Tests added (push_test.go)

  • TestEmptyPackTrailerMatchesObjectFormat — SHA-1 (incl. the canonical golden hash 029d0882…) and SHA-256 trailers.
  • TestPushCommandsSendsEmptyPackForCreate — request body ends with a valid empty pack.
  • TestPushCommandsSendsNoPackForDeleteOnly — delete-only carries no pack.

go build ./..., go vet, the full internal/gitproto + internal/strategy suites, and mise run lint all pass.

This is the cheap, strictly-more-compatible option from your report. I did not add the alternative server-capability gate — sending an empty pack is unconditionally valid, so the capability detection isn't needed. Want me to commit this on a branch, and then move on to drafting the two GitHub issues?

/simplify → 4 cleanup agents in parallel → apply the fixes

You are improving the quality of the changed code, not hunting for bugs. Review it for reuse, simplification, efficiency, and altitude issues, then fix what you find. Do not look for correctness bugs — that is what /code-review is for.

Phase 0 — Gather the diff

Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Review (4 cleanup agents in parallel)

Launch 4 independent review agents via the Agent tool, all in a single message so they run concurrently. Pass each agent the diff and one of the four angles below. Each returns its findings with file, line, a one-line summary, and the concrete cost (what is duplicated, wasted, or harder to maintain).

Reuse

Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.

Simplification

Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.

Efficiency

Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.

Altitude

Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.

Phase 2 — Apply the fixes

Wait for all four agents to complete, dedup findings that point at the same line or mechanism, and fix each remaining one directly. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped (or confirm the code was already clean).

2mo ago·27m·13 calls

Done. /simplify complete — build, full internal/gitproto suite, and mise run lint all pass.

Fixed

  1. Precomputed the two empty packs (push.go) — the empty-pack bytes depend only on the hash algorithm, so there are exactly two possible outputs. They're now built once at package load (emptyPackSHA1, emptyPackSHA256) via a small buildEmptyPack(algo) helper, and emptyPack(adv) just selects between them after the capability check. Removes the per-call hash + allocation in bootstrap's per-branch/per-checkpoint loops, and confines the subtle Sum/append construction to one clearly-named builder that uses the unambiguous append(slices.Clone(header), h.Sum(nil)...) form.

  2. Deduplicated the 12-byte header literal (reuse) — emptyPackHeader now lives once in push.go and is shared by both production and the test (same package), replacing the copy that was in push_test.go.

  3. Extracted the test boilerplate (simplification) — added captureReceivePackBody(t) and awaitBody(t, bodies) helpers, collapsing the two near-identical ~30-line server/channel/select blocks in the new tests down to a few lines each.

Skipped (with reasons)

  • Generalizing the rule into sendReceivePack (altitude) — the altitude agent itself graded the current placement correct. sendReceivePack lacks adv/object-format, and PushObjects always encodes a real pack through the go-git encoder so it isn't exposed; moving the rule would rework a shared chokepoint's signature for no current gain.
  • A PreferredObjectFormat accessor (altitude) — single caller today; adding it now is speculative generality. Worth revisiting only if a second caller appears.
  • The var packData conditional (simplification) — already minimal and idiomatic.

Behavior is unchanged; this was quality-only. The changes remain uncommitted in the working tree.

can you create a branch, commit and open a PR?

2mo ago·8m·4 calls

Done. PR opened: https://github.com/entireio/git-sync/pull/74

  • Branch: fix/pack-less-ref-create-empty-pack (pushed, tracking origin)
  • Commit: 462a28b — gitproto: send empty pack on pack-less ref creates (2 files, +144/-4)
  • PR #74 against main, describing the symptom, root cause, fix, tests, and noting the separate replicate-batching follow-up.

The PR body flags the larger replicate-batching issue as a follow-up. Want me to file that as a separate GitHub issue now (with the corrected TargetMaxPackBytes wording we verified earlier)?

752e16cBatch ref-update commands to stay under receive-pack cap A sync of a repo with many refs sent every ref-update command in a single receive-pack request, which entire-server rejects past 25,000 commands ("too many ref-update commands: 55006 (limit 25000)"). The relay/materialize strategies (replicate, incremental, materialized) had no command-count batching. Batch inside the gitproto push primitives so every strategy benefits: - PushPack / PushObjects send the pack with the first batch and the remaining refs as ref-only follow-ups. The pack carries every object for the whole push, and receive-pack commits the entire received pack (entire-server via CommitQuarantinedFanout, canonical git via tmp_objdir_migrate — neither prunes objects unreachable from the pushed tips), so later batches only move ref pointers. - PushCommands chunks all commands under maxRefUpdatesPerPush (20_000, with headroom under the server's 25_000 cap). Works against both entire-server and canonical git/GitHub (GitHub's per-push branch/tag limit is opt-in, default unlimited). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 5d4b8f06380e+234/-2