fix(source-control): exec-bit-check skips a copy destination whose source was never executable

[kyle-sexton]

Filed by AI. Found by a fresh-context adversarial verifier reviewing PR #2098's own discharges, and
reproduced independently before filing. It is a gap in #2098, which I authored — the verifier was
attacking my work and it was right.

The defect

plugins/source-control/skills/commit/scripts/exec-bit-check.sh:216-222 admits a C* (copy)
destination only when the source was 100755:

  R* | C*)
    IFS= read -r -d '' _source || break
    IFS= read -r -d '' path || break
    if [[ "$src_mode" == "100755" ]]; then
      added+=("$path")
    fi

So a copy off a 100644 shebang source is never reported — even though the destination is a path
that did not previously exist, staged 100644 with a shebang, which is exactly what this check
reports in every other configuration.

Reproduction

Identical staged content, only diff.renames differs. Against origin/main:

########## NON-exec source (the defect)
  diff.renames=false   raw=:000000 100644 … A    copy.sh          --list -> [copy.sh]
  diff.renames=copies  raw=:100644 100644 … C095 lib.sh copy.sh   --list -> []

########## exec source (control, proves the fixture discriminates)
  diff.renames=false   raw=:000000 100644 … A    copy.sh          --list -> [copy.sh]
  diff.renames=copies  raw=:100755 100644 … C095 lib.sh copy.sh   --list -> [copy.sh]

Fixture: commit lib.sh with a shebang, append a line to it, cp lib.sh copy.sh, stage both. (Copy
detection only binds against a source modified in the same change, which is why the source is
edited.) git 2.54.0.windows.1.

This is the exact failure mode #2098 says it exists to remove. From that PR's body: the fix is
there so that catching a file is a function of staged content "rather than of the consumer's
diff.renames setting." The first block above is identical staged content producing two different
answers.

Why the current behaviour is wrong, specifically

R* and C* share one predicate and one justification. The justification — in the script's own
comment at lines 206-208 — is:

100644 -> 100644 is a deliberately non-executable tracked file merely being moved, and reporting
it would flip a mode nobody changed — squarely outside this check's newly-added-only scope.

That is true for R and false for C. A rename destination is the same tracked file at a new
path. A copy destination is a path that did not exist before — it is newly added, squarely inside
the newly-added-only scope. One justification is covering two cases it does not both describe.

Proposed fix

Split the arms rather than widening the shared gate:

• R* — keep the src_mode == "100755" gate. repo19's case is legitimate: a deliberately
  non-executable sourced library being renamed must not be flipped to 100755.
• C* — drop the gate. The destination is always a new path, so admit it and let the existing
  100644-plus-shebang filter do the rest, exactly as the A branch does.

This makes C behave identically to the A the same staged content produces with copy detection
off, which is the stated goal.

Test changes required

exec-bit-check.test.sh case group 21 (repo21) currently asserts "a copy whose SOURCE was never
executable is NOT reported" — it pins the defect. I added that case in #2098 in response to a
reviewer asking for a copy-branch negative twin of repo19's rename negative. The reviewer's ask was
reasonable and I implemented it without noticing that R and C are not symmetric here, so the
test encodes the asymmetry as intended behaviour.

• Invert repo21: a copy off a 100644 shebang source is reported.
• Keep repo19 as-is: a rename off a 100644 shebang source is still not reported. Once the
  arms are split, these two cases stop being twins and each pins its own arm — which is what the
  original reviewer wanted from a copy negative, correctly aimed.
• Add the diff.renames=false / diff.renames=copies pair above as a single case asserting the two
  configurations agree. That is the property the whole finding is about, and nothing currently pins
  it.
• Every case needs a control that fails against the current implementation. A fixture returning the
  same answer on both trees proves nothing — that trap has produced one worthless reproduction and
  one false "already discharged" reading on this sweep already.

Status of the source thread

PRRT_kwDOTCGFQM6T4klg on #1590 is left unresolved and points here. The reviewer's literal ask
was "treat the C* destination as an added candidate"; only the exec-source half of that shipped.

Related

• feat(source-control): harden /commit — exec-bit backstop, trailer SSOT, composition resilience #1590 — where the finding was filed
• fix(source-control): discharge four stranded P2 review findings #2098 — the partial discharge
• Triage the remaining stranded P2 review findings from the post-merge sweep #1938 — the stranded-findings sweep this came out of