Skip to content

Fetch Release Snapshot Before The Blobless Ancestry Fetch In The Collector Runtime Updater - #1909

Open
jaywedgeworth22 wants to merge 1 commit into
mainfrom
zcode/um-1661-telemetry-install
Open

jaywedgeworth22 wants to merge 1 commit into
mainfrom
zcode/um-1661-telemetry-install

Conversation

@jaywedgeworth22

@jaywedgeworth22 jaywedgeworth22 commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Refs UM-1661.

Fetches the pinned release commit with a depth-1 snapshot fetch before the blobless main fetch in checkoutApprovedSource in scripts/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 under GIT_NO_LAZY_FETCH against 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 at scripts/update-local-collector-runtime.mjs:389: shallow snapshot may break ancestry checks for old SHAs (high); floating-main path with null SHA skips snapshot fetch and may leave checkout without blobs (high); unvalidated SHA triggers full blob fetch before ancestry validation (medium).

…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.
@jaywedgeworth22
jaywedgeworth22 enabled auto-merge (squash) November 7, 2026 23:31
@linear-code

linear-code Bot commented Nov 7, 2026

Copy link
Copy Markdown

UM-1661

@kody-ai

kody-ai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

​

@github-actions
github-actions Bot disabled auto-merge November 7, 2026 23:31
@github-actions
github-actions Bot disabled auto-merge November 7, 2026 23:31
@jaywedgeworth22
jaywedgeworth22 enabled auto-merge (squash) November 7, 2026 23:31
@github-actions
github-actions Bot disabled auto-merge November 7, 2026 23:31
Comment on lines +389 to 396
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"]);

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.

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.

Comment on lines +389 to 396
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"]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug high

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.

​

​

Comment on lines +389 to +395
if (sha !== null) {
try {
run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]);
} catch {
// The ancestry check below rejects unknown or unreachable SHAs.
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug high

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.

​

​

Comment on lines +389 to +395
if (sha !== null) {
try {
run("git", ["-C", path, "fetch", "--quiet", "--depth=1", "origin", sha]);
} catch {
// The ancestry check below rejects unknown or unreachable SHAs.
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Performance medium

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.

​

​

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant