Skip to content

Commit 089f994

Browse files
fix: exclude untracked files from record-review.sh overlap check (merge worktree-20260324-211622)
2 parents 9e382ee + 4805de1 commit 089f994

File tree

3 files changed

+22
-13
lines changed

3 files changed

+22
-13
lines changed

plugins/dso/hooks/record-review.sh

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -246,27 +246,18 @@ if [[ -n "$CFG_UNIT_SNAPSHOT_PATH" ]]; then
246246
_RR_EXCLUDE+=(":!${CFG_UNIT_SNAPSHOT_PATH}*.html")
247247
fi
248248

249-
# Build grep exclusion pattern for untracked files
250-
_RR_GREP_PATTERN='^\.checkpoint-needs-review$|^\.sync-state\.json$'
251-
if [[ -n "$CFG_VISUAL_BASELINE_PATH" ]]; then
252-
_VBP_ESC="${CFG_VISUAL_BASELINE_PATH//./\\.}"
253-
_RR_GREP_PATTERN="${_RR_GREP_PATTERN}|^${_VBP_ESC}.*\\.png$"
254-
fi
255-
if [[ -n "$CFG_UNIT_SNAPSHOT_PATH" ]]; then
256-
_USP_ESC="${CFG_UNIT_SNAPSHOT_PATH//./\\.}"
257-
_RR_GREP_PATTERN="${_RR_GREP_PATTERN}|^${_USP_ESC}.*\\.html$"
258-
fi
259-
260249
# Allow tests to inject changed files without writing to the repo.
261250
# isolation-ok: test-only override for overlap check
262251
if [[ -n "${RECORD_REVIEW_CHANGED_FILES:-}" ]]; then
263252
CHANGED_FILES="$RECORD_REVIEW_CHANGED_FILES"
264253
else
254+
# Only include tracked file changes (staged + unstaged). Untracked files are
255+
# excluded because compute-diff-hash.sh excludes them — the overlap check
256+
# must match the scope of the reviewed diff. (Bug dso-lm92)
265257
CHANGED_FILES=$(
266258
{
267259
git diff --name-only HEAD -- "${_RR_EXCLUDE[@]}" 2>/dev/null || true
268260
git diff --cached --name-only HEAD -- "${_RR_EXCLUDE[@]}" 2>/dev/null || true
269-
git ls-files --others --exclude-standard 2>/dev/null | { grep -v -E "$_RR_GREP_PATTERN" || true; }
270261
} | sort -u | { grep -v '^$' || true; }
271262
)
272263
fi

plugins/dso/scripts/acli-integration.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ def update_issue(
274274
transition_issue(jira_key, status, acli_cmd=acli_cmd)
275275

276276
if not kwargs:
277-
# Only a status change was requested — return the transition result
277+
# No editable fields remain (status/priority were already handled above)
278278
if status is not None:
279279
return {"key": jira_key, "status": status}
280280
return {"key": jira_key}

tests/hooks/test-record-review.sh

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,4 +149,22 @@ EXIT_CODE=0
149149
bash "$HOOK" "--reviewer-hash=${HASH}" 2>/dev/null || EXIT_CODE=$?
150150
assert_eq "test_record_review_equals_style_reviewer_hash: --reviewer-hash=VALUE accepted" "0" "$EXIT_CODE"
151151

152+
# ---------------------------------------------------------------------------
153+
# test_record_review_changed_files_excludes_untracked
154+
# Bug dso-lm92: CHANGED_FILES overlap check must not include untracked files.
155+
# The review diff (compute-diff-hash.sh) excludes untracked files, so the
156+
# overlap check in record-review.sh must match that scope. Including untracked
157+
# files can cause false-positive overlap matches against files not in the
158+
# reviewed diff.
159+
# ---------------------------------------------------------------------------
160+
# Check the CHANGED_FILES computation block (not the diagnostic dump which
161+
# legitimately uses ls-files --others for mismatch forensics).
162+
# The CHANGED_FILES block is between "CHANGED_FILES=$(" and the closing ")".
163+
if sed -n '/CHANGED_FILES=$(/,/^[[:space:]]*)/p' "$HOOK" | grep -q 'ls-files.*--others'; then
164+
actual="includes_untracked"
165+
else
166+
actual="excludes_untracked"
167+
fi
168+
assert_eq "test_record_review_changed_files_excludes_untracked" "excludes_untracked" "$actual"
169+
152170
print_summary

0 commit comments

Comments
 (0)