Skip to content

fix(dirty-set): correct lockfile detection in the fallback trigger - #21

Merged
honnix merged 2 commits into
mainfrom
honnix/amber-thicket
Sep 25, 2026
Merged

honnix merged 2 commits into
mainfrom
honnix/amber-thicket

Conversation

@honnix

@honnix honnix commented Sep 24, 2026

Copy link
Copy Markdown
Member

isFallbackTrigger matched lockfiles by substring on the basename, and erred in both directions.

Missed real lockfiles carrying .lock as 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 monorepo tools/multitool.lock.json changed 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 .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.

basename before after
multitool.lock.json false true
.terraform.lock.hcl false true
merge-pnpm-lock.sh true false
2x-lock.png true false
V3__workflow_lock.sql true false
pnpm-lock.yaml, Cargo.lock, buf.lock true true
deps-lock.xyz (unknown ext) true true

Test plan

  • TestIsFallbackTriggerLockfiles covers all of the above plus the unknown-extension case.
  • TestComputeDirtySetNestedLockExtensionFallsBack and TestComputeDirtySetLockNamedScriptDoesNotFallBack cover it end to end through ComputeDirtySet.

Note: I could not run go test locally — 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.

honnix and others added 2 commits September 24, 2026 15:31
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>
@honnix
honnix merged commit 6a05c88 into main Sep 25, 2026
2 checks passed
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