fix(dirty-set): correct lockfile detection in the fallback trigger - #21
Merged
Merged
Conversation
isFallbackTrigger matched lockfiles by substring on the basename, which erred in both directions. It missed real lockfiles that carry ".lock" as an inner component, such as multitool.lock.json and .terraform.lock.hcl. That is the dangerous direction: the file belongs to no target's edges, so nothing is marked dirty, every target keeps its seed hash, and impacted targets are silently under-reported. In one monorepo tools/multitool.lock.json changed 33 times in the last 90 days, each time without a fallback. It also caught any file whose name merely contains "-lock." or "_lock." regardless of kind. A git merge driver (merge-pnpm-lock.sh), a PNG, an XML, three SQL migrations and three markdown documents each forced a full rehash of ~444k targets. A ".lock" extension now always triggers. The ambiguous inner-component and name-stem forms additionally require an extension a lockfile could plausibly use; an unrecognised extension stays conservative and still triggers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pip writes requirements_lock.txt, so excluding ".txt" made a real lockfile stop triggering a fallback -- the exact under-reporting this change set out to remove. TestComputeDirtySetRepositoryMetadataFallbacks already covered it and caught the regression. Drop ".txt" and ".xml", and record the rule in a comment: the denylist holds executable code, images and prose only. An extension that could carry structured dependency data stays off it, because a wrong entry silently under-reports impacted targets while a missing one costs only a redundant rehash. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
isFallbackTriggermatched lockfiles by substring on the basename, and erred in both directions.Missed real lockfiles carrying
.lockas an inner component —multitool.lock.json,.terraform.lock.hcl. This is the dangerous direction: the file belongs to no target's edges, so nothing is marked dirty, every target keeps its seed hash, and impacted targets are silently under-reported. In one monorepotools/multitool.lock.jsonchanged 33 times in the last 90 days, each time without a fallback.Caught unrelated files whose name merely contains
-lock.or_lock.— a git merge driver (merge-pnpm-lock.sh), a PNG, an XML, three SQL migrations and three markdown documents, each forcing a full rehash of ~444k targets.A
.lockextension now always triggers. The ambiguous inner-component and name-stem forms additionally require an extension a lockfile could plausibly use; an unrecognised extension stays conservative and still triggers.multitool.lock.json.terraform.lock.hclmerge-pnpm-lock.sh2x-lock.pngV3__workflow_lock.sqlpnpm-lock.yaml,Cargo.lock,buf.lockdeps-lock.xyz(unknown ext)Test plan
TestIsFallbackTriggerLockfilescovers all of the above plus the unknown-extension case.TestComputeDirtySetNestedLockExtensionFallsBackandTestComputeDirtySetLockNamedScriptDoesNotFallBackcover it end to end throughComputeDirtySet.Note: I could not run
go testlocally — the module proxy was unreachable and the generated protobuf deps would not resolve offline. The table was verified against a standalone copy of the function (all 20 cases pass); CI should be treated as the real check.