Skip to content

feat: Populate on-host mirrors from remote mirrors - #4161

Merged
pda merged 2 commits into
mainfrom
pda/remote-git-mirror-on-host-c275
Aug 4, 2026
Merged

feat: Populate on-host mirrors from remote mirrors#4161
pda merged 2 commits into
mainfrom
pda/remote-git-mirror-on-host-c275

Conversation

@pda

@pda pda commented Aug 1, 2026

Copy link
Copy Markdown
Member

Description

Populate and refresh the shared on-host mirror from the server-provided remote mirror before falling back to canonical. This is PR 2 based on #4160.

Context

The shared mirror’s durable origin and canonical heads remain canonical for mixed-version agents. Remote objects are pinned in a namespaced ref without moving canonical refs.

Changes

  • Create mirrors through shared-permission staging, canonicalize origin, and atomically publish.
  • Reclaim interrupted staging and fail open to canonical if optional cleanup fails.
  • Refresh by exact SHA with --no-tags and hidden URL prompts.
  • Require warm objects to be reachable from a durable ref, preventing failed ref writes from becoming retry hits.
  • Reconcile canonical origin on remote and warm hits before shared rename maintenance.
  • Skip the redundant canonical exact-SHA checkout fetch after an on-host hit.
  • Bypass mirrors containing an unpinned build object and dissociate reused checkouts from stale alternates before canonical fetch.
  • Continue canonical checkout without an on-host reference on lock timeout.
  • Preserve locks, snapshots, submodules, skip-update, cancellation, and reference/dissociate modes.

Testing

Real Git-over-HTTP tests cover atomic creation, permissions, staging, lag, timeout, namespaced refs, tag/head isolation, fallback, warm/remote collision repair, failed ref write plus canonical force-push plus reused-checkout retry, lock contention, prompt hiding, and cancellation. The retry regression verifies the stale alternate is removed before canonical fetch; test executors close checkout roots for Windows cleanup.

Disclosures / Credits

Implemented with Cursor under human direction. Independent review drove atomic publication, durable reachability, retry alternate cleanup, warm-hit maintenance, and contention fallback.

Open in Web Open in Cursor 

@pda
pda requested review from a team as code owners August 1, 2026 05:48

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find a concrete issue to flag inline. The change reaches the shared mirror clone/update and atomic-publication paths, so this still needs a human sanity-check.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10735, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

About buildsworth

Model: gpt-5.6-sol with xhigh thinking.

How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.

Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. To let it approve, grant L2 approval by mentioning @buildsworth-bk (see L2 approval grant):

  • L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
  • L2 — Standard risk (new UI, additive API fields, refactors). Approved only with an L2 grant; otherwise comment-only.
  • L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Propagated the parent plan correction in merge ae13c0fe. Documentation only; the on-host mirror layer remains unchanged against its updated base.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remote-hit early return needs to preserve the existing origin-reconciliation path; details inline.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10796, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

Comment thread internal/job/checkout_mirror.go Outdated
@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Merged foundation review fixes through 3e03ed15. This layer now inherits forced prompt hiding, ambient mirror masking, the corrected helper command, and the explicit Git 2.45+ support boundary.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Added the red-step regressions in a4a5ba47: a remote hit on a lossy directory-key collision must repair canonical origin, and a debug-mode creation clone must not print the mirror URL argv.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Fixed the on-host review issue in bca1477e: canonical origin is reconciled before the remote attempt, a hit skips only canonical fetch, and the existing rename fsck/gc and snapshot tail still runs. Remote creation clones also use the debug-proof hidden-prompt option.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Follow-up 0996648e adds the missing package import exposed by the first focused compile run.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Propagated the recursive-submodule eligibility foundation through f463a1ae; this does not change the on-host site because clone flags only gate fresh remote clones.

@buildsworth-bk-app
buildsworth-bk-app Bot dismissed their stale review August 2, 2026 00:32

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous origin-reconciliation concern is fixed. I found one new retry path that can leave a remote-fetched object unpinned; details inline.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10828, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

Comment thread internal/job/checkout_mirror.go Outdated

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous destination-ref retry concern is still open; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10839, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Propagated the final plan alignment through 22e43357; the on-host implementation diff remains scoped to its layer.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10848, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Added red-step contention and warm-cache regressions in d9b2c68e: an already-present commit must still repair a colliding canonical origin, and mirror lock timeout must degrade to canonical checkout rather than fail the job.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Implemented the on-host re-review fixes in 5185d16a: warm hits now reconcile canonical origin and run rename maintenance; lock timeout falls back to canonical checkout without a reference mirror; and staging cleanup failure is warned without blocking an independent canonical clone.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10852, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The merge updates only the parent documentation, so the destination-ref retry concern remains open. I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10862, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open because the regression’s second attempt does not match the production retry path. I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10869, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

Final on-host fix 04b2b0c0 requires a warm commit to be reachable from a durable ref, preventing a failed destination-ref write plus failed canonical fallback from becoming a false warm hit on retry.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern remains open after this foundation merge; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10875, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open after this foundation merge; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10888, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10892, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

@buildsworth-bk review

@buildsworth-bk approve L2

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open after this foundation merge; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10904, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

Resolved the stack conflict by merging the rebased #4160 foundation branch into this branch. The conflict in internal/job/checkout_mirror.go retained #4161’s on-host remote-mirror behavior while taking the latest foundation/main changes. Pushed as 5f2e28c8; focused verification will follow.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destination-ref retry concern is still open after the foundation merge; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 11124, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@pda pda added the feature New user-facing feature! label Aug 3, 2026
Base automatically changed from pda/remote-git-mirror-foundations-c275 to main August 3, 2026 06:12

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous destination-ref retry concern remains open; I found no new issues this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 11261, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>
@cursor
cursor Bot force-pushed the pda/remote-git-mirror-on-host-c275 branch from d78865c to 039ec85 Compare August 3, 2026 12:24
@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

Rebased onto current main and force-pushed the PR as one clean feature commit. The destination-ref retry fix now also dissociates a reused checkout from any stale Buildkite-managed alternate when the mirror is bypassed, so the canonical fetch cannot mistake an unpinned mirror object for a safe local hit. The regression covers the force-push/retry path end to end.

@buildsworth-bk-app
buildsworth-bk-app Bot dismissed their stale review August 3, 2026 12:35

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous concern is fixed, and I found no new issues. This still needs human review because it changes shared mirror publication and checkout alternate handling.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 11278, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>
@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

Follow-up for the rewritten CI run: Windows correctly exposed that the new retry regression left the executor's checkoutRoot open, preventing TempDir cleanup. The shared on-host mirror test executor now closes that root during cleanup. The product path was unaffected.

@buildsworth-bk-app
buildsworth-bk-app Bot dismissed their stale review August 3, 2026 12:57

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous destination-ref retry concern remains fixed after this test-only cleanup, and I found no new issues. This still needs human review because it changes shared mirror publication and checkout alternate handling.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 11290, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

@catkins catkins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@pda
pda merged commit 01c20f6 into main Aug 4, 2026
5 checks passed
@pda
pda deleted the pda/remote-git-mirror-on-host-c275 branch August 4, 2026 00:28
@zhming0 zhming0 mentioned this pull request Aug 4, 2026
@pda pda mentioned this pull request Aug 4, 2026
2 tasks
cursor Bot pushed a commit that referenced this pull request Aug 4, 2026
…-on-host-c275"

This reverts commit 01c20f6, reversing
changes made to 20df5d1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New user-facing feature!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants