ENG-1636: Detect re-published private-ingredient content in the depot#3820
Open
mitchell-as wants to merge 2 commits into
Open
ENG-1636: Detect re-published private-ingredient content in the depot#3820mitchell-as wants to merge 2 commits into
mitchell-as wants to merge 2 commits into
Conversation
Private ingredients use a timeless, mutable pointer: the same artifact ID can point at different content over time. A depot keyed by ID alone would serve the old plaintext forever. Record the build-plan content checksum on each private depot entry and, in the cache-hit decision, re-fetch and re-deploy a private artifact whose stored checksum no longer matches the build plan, undeploying the stale plaintext first. The comparison is gated on the depot's Private flag, so public artifacts (which are content-addressed and immutable) and projects without private ingredients are unaffected. The artifact directory is also cleared before unpacking so a re-fetch replaces stale content rather than merging into it. ENG-1636 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A private artifact re-published under the same artifact ID keeps the project's commit hash unchanged, so the hash short-circuits would report the runtime as up to date and never run the depot content check. When the runtime has private artifacts, state refresh and state pull now bypass those short-circuits (the NeedsUpdate gate, the runbits hash check, and the pkg/runtime hash check) to re-solve and re-check content. Routine commands keep the fast path, and projects without private artifacts are unaffected. Also renames the depot's HasPrivateArtifacts helper and standardizes comment terminology on "private artifact". ENG-1636 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds content-aware cache invalidation for private artifacts so that when a private artifact is re-published under the same artifact ID (with an unchanged commit hash), the runtime detects the content change and re-fetches/re-deploys it. It also adjusts state refresh and state pull to perform the extra re-check only when the runtime actually has private artifacts deployed.
Changes:
- Store a build-plan checksum alongside depot cache entries for decrypted private artifacts, and use it to detect re-published content for a fixed artifact ID.
- Force reinstall/re-download of detected-stale private artifacts during runtime setup/update.
- Add an opt-in update path for refresh/pull that re-checks private artifact content even when the runtime hash is unchanged, gated by whether the runtime has private artifacts.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/runtime/setup.go | Detect stale private artifact content and force re-download/reinstall; record checksum on MarkPrivate; clears existing unpack path before unpacking. |
| pkg/runtime/runtime.go | Add HasPrivateArtifacts() and gate hash short-circuit based on the new opt + runtime contents. |
| pkg/runtime/options.go | Add runtime option to enable private-artifact content re-checks on unchanged hash. |
| pkg/runtime/depot.go | Persist checksum for private cache entries; add HasPrivateArtifacts + PrivateContentChanged. |
| pkg/runtime/decrypt_test.go | Add tests for checksum storage, checksum mismatch detection, and private-artifact deployment detection. |
| pkg/runtime_helpers/helpers.go | Add project-level helper to query whether the runtime has private artifacts. |
| internal/runners/refresh/refresh.go | If commit hash is unchanged, still refresh when private artifacts are present; pass new update option. |
| internal/runners/pull/pull.go | Pass new update option so pull triggers private-artifact re-check behavior. |
| internal/runbits/runtime/runtime.go | Thread new option through runbit opts; adjust “already up-to-date” check accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
499
to
+505
| unpackPath := s.depot.Path(artifact.ArtifactID) | ||
| if fileutils.DirExists(unpackPath) { | ||
| // Clear prior private artifact that is being updated. | ||
| if err := os.RemoveAll(unpackPath); err != nil { | ||
| return errs.Wrap(err, "could not clear private artifact directory") | ||
| } | ||
| } |
Comment on lines
+221
to
+242
| // HasPrivateArtifacts reports whether any artifact deployed at path is a private | ||
| // artifact. A runtime with private artifacts cannot be trusted as up-to-date | ||
| // by commit hash alone, since the same artifact ID may point at re-published | ||
| // content. | ||
| func (d *depot) HasPrivateArtifacts(path string) bool { | ||
| d.mapMutex.Lock() | ||
| defer d.mapMutex.Unlock() | ||
|
|
||
| resolved := fileutils.ResolvePathIfPossible(path) | ||
| for id, deploys := range d.config.Deployments { | ||
| info, ok := d.config.Cache[id] | ||
| if !ok || !info.Private { | ||
| continue | ||
| } | ||
| for _, dep := range deploys { | ||
| if fileutils.ResolvePathIfPossible(dep.Path) == resolved { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ENG-1636: Detect re-published private-ingredient content in the depot
Part of the private ingredient work (ENG-1563). A private artifact can be re-published with new content under the same artifact ID (the timeless, mutable-pointer model). A cache keyed by ID alone would serve the old plaintext forever; this makes the State Tool notice the content changed and re-fetch it.
Each private depot entry now records the build-plan checksum its content was stored under. The cache-hit decision compares that against the build plan and, on a mismatch, re-fetches and re-deploys the artifact (undeploying the stale plaintext first). The comparison is gated on the depot's
Privateflag, so public artifacts and non-private projects are untouched.Because a re-publish keeps the commit hash unchanged, the hash short-circuits would otherwise skip the check entirely. So
state refresh/state pullnow re-solve and re-check content only when the runtime actually has private artifacts — routine commands keep the fast path.Two commits: the depot checksum + content-aware cache-hit, then the refresh/pull re-check.
Base branch: targets
mitchell/eng-1638(where this was branched from, for the checksum primitive); GitHub will retarget toversion/0-48-1-RC2as the upstream PRs land.Tested for the checksum comparison (fresh/stale/public/absent/empty), the stored-checksum write, and private-artifact detection by deployment path.
🤖 Generated with Claude Code