fix(cachekey): include config fingerprint in graph keys, skip mapping cache for mutable refs (audit #7) - #263
Draft
sbalabanov wants to merge 2 commits into
Draft
fix(cachekey): include config fingerprint in graph keys, skip mapping cache for mutable refs (audit #7)#263sbalabanov wants to merge 2 commits into
sbalabanov wants to merge 2 commits into
Conversation
… cache for mutable refs (audit #7) Graph cache keys now include an md5 digest of graph-affecting RepositoryConfig fields (bzlmod_enabled, bazel_command, bazel_extra_args, bazel_startup_options, full_hash_repos, exclude_external_targets, excluded_files). When all fields are at their zero/default values the suffix is omitted, preserving all existing cache entries. The treehash-mapping fast path now requires base_sha to be a full 40-hex SHA. Mutable refs (HEAD, branch names, short SHAs) skip the mapping lookup and write so the workspace is materialized and the ref resolved fresh each time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
Pass cfg as ConfigProvider when constructing the controller in example/main.go so the controller's fast-path cache lookup uses the same config fingerprint the orchestrator writes under. Without this, any deployment with non-default RepositoryConfig would silently miss the graph cache on every request. Add a key-symmetry test asserting the controller read path and orchestrator write path produce identical graph keys for the same non-default config. Add godoc on Params.ConfigProvider documenting that it must be the same config source the orchestrator uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
3 participants
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.
Summary
Graph cache keys omitted graph-affecting RepositoryConfig fields, so a config change (e.g. enabling bzlmod, adding bazel args) could silently serve stale cached graphs. Additionally, the treehash-mapping cache accepted mutable refs like HEAD, which go stale once the ref moves.
HashGraphAffectingConfigto compute an md5 digest of the 7 graph-affecting config fields; emit empty suffix for default/zero config so existing cache entries remain validGetGraphByTreeHashwith aconfigHashparameter and update all call sites (controller, orchestrator)IsFullHexSHAguard: treehash mapping lookups and writes are skipped for non-40-hex base_sha values (HEAD, branch names, short SHAs)GetTreehashCachePathandProtoToBuildDescriptiongodocs to document the mutable-ref constraintTest plan
go build ./...passesgo test ./core/cachekey/ ./controller/ ./orchestrator/ ./internal/mapper/-- all passgo test ./...passes (integration tests skip due to missing env, pre-existing)make gazelle-- BUILD files in sync🤖 Generated with Claude Code