Repository navigation
Fetch Release Snapshot Before The Blobless Ancestry Fetch In The Collector Runtime Updater - #1909
jaywedgeworth22 wants to merge 1 commit into
Conversation
…fetch The detached updater could never complete a fresh install against a filter-honoring server. checkoutApprovedSource fetched origin main with --filter=blob:none (which registers the staging clone as partial and excludes every blob), then detached-checked-out the release commit with GIT_NO_LAZY_FETCH=1 hard-set on every git call — so the checkout's missing blobs could never be fetched and the install always died with "could not fetch <blob> from promisor remote". Found live against GitHub during the UM-1661 ZCode/Kimi collector install; the fixture server in test-local-collector-runtime ignores filters, which is why the suite stayed green. The fix fetches the release snapshot first with --depth=1 (one commit's full tree, no filter registered yet), then fetches main blobless for the ancestry check. A failed snapshot fetch is swallowed so an unknown SHA still reaches the ancestry check and fails with its canonical refusal (the unapproved-pin rollback case keeps its assertion). The fixed origin, no-inherited-config and no-lazy-fetch posture is unchanged. Verified live: the updater now installs an exact origin/main release into a fresh runtime root. Refs UM-1661.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| if (sha !== null) { | ||
| try { | ||
| run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]); | ||
| } catch { | ||
| // The ancestry check below rejects unknown or unreachable SHAs. | ||
| } | ||
| } | ||
| run("git", ["-C", path, "fetch", "--quiet", "--filter=blob:none", "origin", "main"]); |
There was a problem hiding this comment.
Bug: checkoutApprovedSource fails during detached checkout when sha is null because GIT_NO_LAZY_FETCH=1 prevents fetching blobs missing from the tree-only fetch.
Severity: LOW
Suggested Fix
Ensure that required blobs are fetched when sha is null before performing git checkout --detach <sha>, avoiding reliance on lazy-fetching under GIT_NO_LAZY_FETCH=1.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: scripts/update-local-collector-runtime.mjs#L389-L396
Potential issue: When `checkoutApprovedSource(path, null)` is called during rollback
migration fallback, the `if (sha !== null)` check skips fetching the target commit with
full blob objects. Instead, it runs `fetch --filter=blob:none origin main`, resulting in
missing blobs. Because git invocations execute with `GIT_NO_LAZY_FETCH=1`, the
subsequent `git checkout --detach <sha>` fails to lazily fetch missing blobs and errors
out with a promisor-remote failure.
Did we get this right? 👍 / 👎 to inform future reviews.
| if (sha !== null) { | ||
| try { | ||
| run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]); | ||
| } catch { | ||
| // The ancestry check below rejects unknown or unreachable SHAs. | ||
| } | ||
| } | ||
| run("git", ["-C", path, "fetch", "--quiet", "--filter=blob:none", "origin", "main"]); |
There was a problem hiding this comment.
Shallow snapshot fetch breaks ancestry check for old SHAs
A genuine old release in sha fails validation with Requested SHA is not an ancestor and blocks rollback because the --depth=1 fetch truncates history for the later merge-base validation. Fetch --unshallow after validation so the check runs on full history.
Prompt for LLM
File scripts/update-local-collector-runtime.mjs, lines 389-396:
Shallow snapshot fetch breaks ancestry check for old SHAs
The depth-1 snapshot fetch creates a shallow boundary that the later blobless main fetch inherits, so merge-base ancestry can falsely fail. A valid old release SHA then fails with Requested SHA is not an ancestor and rollback/migration is refused even though the SHA is genuine. Unshallow or deepen after validation (e.g. fetch with --unshallow or fetch ancestry before shallowing) so the ancestry check runs on full commit history.
Suggested code:
if (sha !== null) {
try {
run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]);
} catch {
// The ancestry check below rejects unknown or unreachable SHAs.
}
}
run("git", ["-C", path, "fetch", "--quiet", "--filter=blob:none", "origin", "main"]);
try {
run("git", ["-C", path, "fetch", "--quiet", "--filter=blob:none", "--unshallow", "origin", "main"]);
} catch {
// Complete repository; ancestry check already has full history.
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (sha !== null) { | ||
| try { | ||
| run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]); | ||
| } catch { | ||
| // The ancestry check below rejects unknown or unreachable SHAs. | ||
| } | ||
| } |
There was a problem hiding this comment.
Floating-main path skips snapshot fetch leaving checkout without blobs
The default fresh install cannot materialize blobs because sha === null skips the snapshot fetch and checkout --detach runs on a --filter=blob:none clone with GIT_NO_LAZY_FETCH. Fetch depth-1 for the resolved FETCH_HEAD or main tip on this path as well.
Prompt for LLM
File scripts/update-local-collector-runtime.mjs, lines 389-395:
Floating-main path skips snapshot fetch leaving checkout without blobs
The depth-1 snapshot fetch is guarded by `if (sha !== null)`, so the floating-main path (sha === null) still does only the `--filter=blob:none` fetch before `checkout --detach`. That leaves the default fresh install with the exact original failure — a partial blobless clone plus GIT_NO_LAZY_FETCH on every git call makes the detached checkout unable to materialize blobs. Apply the same snapshot fetch to the resolved main tip (fetch depth-1 for FETCH_HEAD after resolving it, or fetch main unfiltered depth-1 first).
Suggested code:
if (sha !== null) {
try {
run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]);
} catch {
// The ancestry check below rejects unknown or unreachable SHAs.
}
} else {
run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", "main"]);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (sha !== null) { | ||
| try { | ||
| run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]); | ||
| } catch { | ||
| // The ancestry check below rejects unknown or unreachable SHAs. | ||
| } | ||
| } |
There was a problem hiding this comment.
Unvalidated SHA triggers full blob fetch before ancestry check
A bloated reachable sha can force an unbounded blob download that the ancestry validation then discards, wasting network and disk per attempt. Validate ancestry with a blobless fetch first and fetch the full snapshot only after sha is proven an ancestor.
Prompt for LLM
File scripts/update-local-collector-runtime.mjs, lines 389-395:
Unvalidated SHA triggers full blob fetch before ancestry check
The snapshot fetch pulls the full tree/blobs of an unvalidated SHA before the ancestry allowlist check. A bloated or malicious reachable commit forces a full unbounded blob download that is discarded when the ancestry check then rejects it, wasting network/disk per attempt. Check ancestry with a commit-only/blobless fetch first and fetch the full depth-1 snapshot only after the SHA is proven an ancestor.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Refs UM-1661.
Fetches the pinned release commit with a depth-1 snapshot fetch before the blobless
mainfetch incheckoutApprovedSourceinscripts/update-local-collector-runtime.mjs. This ensures the target release tree has blobs before the clone becomes partial, fixing fresh collector runtime installs that could not complete checkout underGIT_NO_LAZY_FETCHagainst filter-honoring servers (UM-1661). Snapshot fetch failures are ignored so unknown or unreachable SHAs still reach the existing ancestry check for canonical refusal, with the later blobless fetch supplying ancestry only. No tests were added in this change and no test runs were reported, so successful fresh install and ancestry enforcement remain unverified. Automated review reported 3 unresolved findings atscripts/update-local-collector-runtime.mjs:389: shallow snapshot may break ancestry checks for old SHAs (high); floating-mainpath with null SHA skips snapshot fetch and may leave checkout without blobs (high); unvalidated SHA triggers full blob fetch before ancestry validation (medium).