feat(git): add no-cone support for sparse patterns#24
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces a structured Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Pull request overview
This PR adds support for git sparse-checkout's no-cone mode to handle advanced glob patterns like ** (double-star) and literal file patterns that cone mode doesn't support. Previously, patterns with ** were not eligible for sparse checkout; now they use no-cone mode with gitignore-style pattern matching.
Changes:
- Refactored sparse-checkout eligibility logic from
isSparseEligible/extractSparsePathsto a unifiedresolveSparseSpecfunction that determines mode and patterns - Added support for
--no-coneflag when patterns include**or literals - Added test coverage for no-cone mode with mixed literal and glob patterns
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/git/fetch-source.ts | Replaced sparse eligibility logic with new resolveSparseSpec function that chooses between cone and no-cone modes based on pattern types; added --no-cone flag support in three git operation functions |
| tests/fetch-source-file-protocol.test.js | Added test verifying no-cone mode is used for mixed include patterns (literal file + double-star glob) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/git/fetch-source.ts`:
- Around line 292-309: resolveSparseSpec currently truncates patterns at the
first "*" only (see the starIndex calculation) which lets patterns containing
"?" or "[" slip through and be treated as cone-mode; update the truncation to
locate the first occurrence of any glob character (e.g., "*", "?", "[") when
computing base paths so patterns like "docs/??.md" or "docs/[ab].md" are
detected as non-cone and handled accordingly; keep the rest of the flow
(normalizeSparsePatterns, patternHasGlob check, trimming trailing slashes,
uniquePaths creation) the same but replace the single-character indexOf("*")
logic with a search for the first index among the full set of glob chars and use
that index to slice the base.
🧹 Nitpick comments (1)
src/git/fetch-source.ts (1)
292-296: Avoidas const/as string[]by typing the sparse spec.
A smallSparseSpectype and explicit return type removes the assertions and aligns with the “avoid type casts” guideline.♻️ Suggested refactor
+type SparseSpec = { + enabled: boolean; + mode: "cone" | "no-cone"; + patterns: string[]; +}; + -const resolveSparseSpec = (include?: string[]) => { +const resolveSparseSpec = (include?: string[]): SparseSpec => { const normalized = normalizeSparsePatterns(include); if (normalized.length === 0) { - return { enabled: false, mode: "cone" as const, patterns: [] as string[] }; + return { enabled: false, mode: "cone", patterns: [] }; } const hasDoubleStar = normalized.some((pattern) => pattern.includes("**")); const hasLiteral = normalized.some((pattern) => !patternHasGlob(pattern)); if (hasDoubleStar || hasLiteral) { - return { enabled: true, mode: "no-cone" as const, patterns: normalized }; + return { enabled: true, mode: "no-cone", patterns: normalized }; } // ... - return { enabled: true, mode: "cone" as const, patterns: uniquePaths }; + return { enabled: true, mode: "cone", patterns: uniquePaths }; };
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/git/fetch-source.ts`:
- Around line 307-335: resolveSparseSpec currently treats patterns with
glob/wildcard as cone-eligible by extracting the base path, causing broad
matching; change the logic so any pattern where patternHasGlob(pattern) is true
(including ones that matched the '**' check) sets coneEligible = false and
breaks out of the loop so cone mode is disabled for globs; update the loop in
resolveSparseSpec to only push to conePaths when a pattern is a pure directory
literal (isDirectoryLiteral returns true) and otherwise mark coneEligible =
false (using the existing variables patternHasGlob, isDirectoryLiteral,
conePaths, and coneEligible).
🧹 Nitpick comments (1)
src/git/fetch-source.ts (1)
598-629: Consider extracting a shared sparse-checkout helper to avoid duplication.The same
sparse-checkout setblock appears incloneRepo,addWorktreeFromCache, andcloneOrUpdateRepo. A small helper would reduce drift risk and keep option wiring consistent.♻️ Suggested refactor
+const applySparseCheckout = async ( + repoDir: string, + sparseSpec: ReturnType<typeof resolveSparseSpec>, + options?: { + timeoutMs?: number; + logger?: (message: string) => void; + allowFileProtocol?: boolean; + }, +) => { + if (!sparseSpec.enabled) return; + const sparseArgs = ["-C", repoDir, "sparse-checkout", "set"]; + if (sparseSpec.mode === "no-cone") { + sparseArgs.push("--no-cone"); + } + sparseArgs.push(...sparseSpec.patterns); + await git(sparseArgs, options); +};- if (sparseSpec.enabled) { - const sparseArgs = ["-C", outDir, "sparse-checkout", "set"]; - if (sparseSpec.mode === "no-cone") { - sparseArgs.push("--no-cone"); - } - sparseArgs.push(...sparseSpec.patterns); - await git(sparseArgs, { - timeoutMs: params.timeoutMs, - allowFileProtocol: true, - logger: params.logger, - }); - } + await applySparseCheckout(outDir, sparseSpec, { + timeoutMs: params.timeoutMs, + allowFileProtocol: true, + logger: params.logger, + });
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/git/fetch-source.ts`:
- Around line 307-338: The resolveSparseSpec function currently uses inline type
assertions (`as const`, `as string[]`) in its return objects; remove those
assertions and instead add an explicit return type to the function signature
that models the discriminated union (e.g. { enabled: false; mode: "cone";
patterns: string[] } | { enabled: true; mode: "cone" | "no-cone"; patterns:
string[] } or your existing SparseSpec type). Update the function declaration
for resolveSparseSpec(include?: string[]): <ExplicitReturnType> and return plain
objects (no `as const`/`as string[]`) so the compiler infers the discriminant
and patterns as string[] correctly; keep the existing control flow and values
(conePaths/uniquePaths/normalized.map(toNoConePattern)) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/git/fetch-source.ts`:
- Around line 292-305: The current logic in isDirectoryLiteral treats
extensionless filenames (e.g., LICENSE, Makefile) as directories by checking for
absence of a dot in the last segment; change this to a conservative rule that
only treats a pattern as a directory literal when it explicitly ends with '/'
(i.e., make isDirectoryLiteral return true only for pattern.endsWith('/')), and
keep toNoConePattern unchanged so no trailing slash is appended for ambiguous
extensionless patterns; if you want later disambiguation, add a separate pass
after clone (e.g., using git ls-tree) rather than inferring by lack of a dot.
Summary
Refactor sparse checkout logic to support no-cone mode for glob patterns.
Changes
isSparseEligibleandextractSparsePathswith unifiedresolveSparseSpecfunctionpatternHasGlobto determine sparse mode (no-cone vs cone)--no-coneflag to git sparse-checkout when patterns contain globs or literal pathsSummary by CodeRabbit
New Features
Bug Fixes
Tests