Skip to content

Honour cone mode on git older than 2.35 - #4130

Open
lox wants to merge 4 commits into
mainfrom
cursor/sparse-checkout-pin-cone-old-git-8562
Open

Honour cone mode on git older than 2.35#4130
lox wants to merge 4 commits into
mainfrom
cursor/sparse-checkout-pin-cone-old-git-8562

Conversation

@lox

@lox lox commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

On git < 2.35, sparse-checkout set has no --cone flag, so cone mode was not actually enforced after clone --sparse. Run sparse-checkout init --cone before set when that flag is unavailable; fail the checkout if pinning fails.

Behaviour change on git 2.27–2.34: top-level files appear, and paths match only at the top level.

Also fixes a pre-existing nil-deref race in ExecutorTester.Cancel that could panic TestPreExitHooksFireAfterCancel under load.

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

  • Cursor cloud agent implemented this change
Open in Web Open in Cursor 

@cursor
cursor Bot force-pushed the cursor/sparse-checkout-pin-cone-old-git-8562 branch from beab265 to c4f5f3c Compare July 28, 2026 22:58
@lox lox added the change Not a new feature, but a user observable non-breaking behavior change. label Jul 28, 2026 — with Cursor
@cursor
cursor Bot force-pushed the cursor/sparse-checkout-pin-cone-old-git-8562 branch 2 times, most recently from cdd333c to 6c55d58 Compare July 29, 2026 17:31
Base automatically changed from cursor/sparse-checkout-no-cone-8562 to main July 29, 2026 20:12
`git sparse-checkout set` only learned --cone/--no-cone in git 2.35. Below
that the interpretation comes from core.sparseCheckoutCone, and `git clone
--sparse` on those versions runs `sparse-checkout init` without --cone, so
a cone-mode request has been getting non-cone pattern matching: paths match
a directory of that name at any depth, and top-level files are left out of
the working tree.

Pin the mode with `sparse-checkout init --cone` (accepted since git 2.25)
when the mode option isn't available, so the paths are interpreted the way
the job asked on every supported git. Non-cone mode is unaffected: it still
falls back to a full checkout below 2.35, since it can't be requested
without the option.

This changes which files land in the working tree for sparse checkouts on
git 2.27-2.34: top-level files now appear, and a path only matches at the
top level. That's the documented cone behaviour the agent has always
claimed, but it is a change for anyone on those versions.

Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
@cursor
cursor Bot force-pushed the cursor/sparse-checkout-pin-cone-old-git-8562 branch from 6c55d58 to 8265e95 Compare July 30, 2026 15:31
cursoragent and others added 3 commits July 30, 2026 15:35
Rename pinSparseCheckoutMode to pinConeMode, drop the silent no-cone
no-op that would recreate the bug if the caller invariant ever broke,
derive --cone from SparseCheckoutModeCone, and assert the operator log.

Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
Cancel could run during Run's pre-Start setup while cmd/Process are
still nil, panicking TestPreExitHooksFireAfterCancel under load.
Wait briefly for the process before signalling.

Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
Under parallel load, Cancel could time out waiting for Start while
flushPendingLocalHooks still ran unlocked. Lock across setup+Start so
Cancel blocks until the process exists.

Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
@lox
lox marked this pull request as ready for review July 30, 2026 16:41
@lox
lox requested review from a team as code owners July 30, 2026 16:41

@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 old-Git behavior is narrow, but this changes the agent's checkout execution and failure path, so I'm leaving it for a human sanity-check. I didn't find any issues.

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 10119, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change Not a new feature, but a user observable non-breaking behavior change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants