Skip to content

Fix malware verdict formatter Bugbot findings from managed-files rollout - #154

Merged
hoffmang9 merged 5 commits into
mainfrom
cursor/fix-verdict-formatter-bugbot-7c91
Sep 3, 2026
Merged

Fix malware verdict formatter Bugbot findings from managed-files rollout#154
hoffmang9 merged 5 commits into
mainfrom
cursor/fix-verdict-formatter-bugbot-7c91

Conversation

@hoffmang9

@hoffmang9 hoffmang9 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Motivation

Today's Cursor Bugbot reviews flagged the newly rolled-out format_malware_review_verdict / VERDICT_RE post-processing from #153 on consumer "Update Managed Files" PRs. The source of truth is templates/dependency-cursor-review.yml; this PR fixes the template only.

Consumer reviews that motivated this work:

Follow-up Bugbot reviews on this PR:

Original consumer-repo findings (fixed)

# Finding Status
1 Verdict regex steals earlier bold markers (Low) Fixed
2 Formatter picks wrong label (Medium) Fixed
3 Formatter inverts verdict (Medium) Fixed

PR 154 Bugbot follow-ups

# Finding Commit Status
4 Verdict regex skips valid tokens (Medium) 9d7318d Fixed
5 Tests write leftover output file (Low) 9d7318d Fixed
6 Quoted verdicts can invert headline (Medium) 5c8f830 Fixed
7 Unpaired bold breaks comment markdown (Low) 5c8f830 Fixed
8 Cleanup strips unrelated bold markers (Medium) bfa300c Fixed
9 Orphan cleanup skips indented verdicts (Medium) 26976b7 Fixed_orphan_trailing_bold_span now lstrip leading whitespace before unpaired-bold detection

CI / Bugbot status (HEAD 26976b7)

All checks green, including Cursor Bugbot (pass / SUCCESS on 26976b7). No new Bugbot issue threads on HEAD after the indented-orphan fix. Latest review with findings was on bfa300c (1 issue, fixed in 26976b7).

Checks passing: test, prettier, builds (all platforms), installers, release, dependency-review, commit signing, Socket Security, Cursor Bugbot.

Changes

  • templates/dependency-cursor-review.yml: VERDICT_RE, formatter helpers, targeted orphan cleanup.
  • internal/workflowscripts/test_malware_verdict_formatter.py: 18 unit tests loading the inline snippet from the template.
  • Makefile: Python tests wired into make test.

Testing

  • python3 -m unittest discover -s internal/workflowscripts -p 'test_*.py'
  • make test-short (Go tests + Python tests)
Open in Web Open in Cursor 

Replace VERDICT_RE (optional ** with cross-line \s*) with a
standalone-line pattern that only matches own-line Verdict: lines.
Use the last standalone match as the official verdict so incidental
or prompt-echo mentions cannot override the model conclusion.

Add unit tests that load the inline workflow snippet from the template
and wire them into make test.

Co-authored-by: Gene Hoffman <hoffmang9@users.noreply.github.com>
@hoffmang9
hoffmang9 marked this pull request as ready for review September 3, 2026 20:03
Comment thread templates/dependency-cursor-review.yml Outdated
Comment thread internal/workflowscripts/test_malware_verdict_formatter.py Outdated
Restore PR-153-style token capture for line-start and punctuation-glued
Verdict: labels while keeping last-match-wins and horizontal whitespace
only (no cross-line ** stealing). Stop test loader from exec'ing the
full workflow snippet that writes cursor_output.json.

Co-authored-by: Gene Hoffman <hoffmang9@users.noreply.github.com>
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Addressed both Bugbot follow-ups from review #5106299608 in 9d7318d:

Finding 1 — Verdict regex skips valid tokens (d8368c8d) — confirmed & fixed. The full-line $ anchor was too strict and regressed PR-153's token-capture behavior for Verdict: benign because … and next.Verdict: benign. Updated VERDICT_RE to match at line start or after punctuation glue ((?<=[^\w \t\n*])), still capturing only the token, still preferring the last match, and still using [ \t]* (not \s*) so earlier ** cannot bind across newlines. Mid-sentence Consider Verdict: malicious … with no later valid verdict remains unchanged by design.

Finding 2 — Tests write leftover output file (d079e78c) — confirmed & fixed. Importing the test module did exec the full workflow snippet and write cursor_output.json. The loader now execs only the helper definitions (through format_malware_review_verdict) and a new test asserts no file is written in a temp directory.

Original consumer-repo findings (bold-marker stealing, wrong/inverted verdict from first-match-anywhere) remain fixed; this follow-up narrows the regex without reverting those protections.

Comment thread templates/dependency-cursor-review.yml Outdated
Comment thread templates/dependency-cursor-review.yml Outdated
Restrict punctuation glue to sentence terminators (.!?:;) so quoted,
backticked, and parenthesized Verdict: mentions are not official tokens.
Match either fully paired **Verdict: token** or token-only forms;
strip orphan trailing ** from lines after removal. Add tests for both
Bugbot follow-ups on PR 154.

Co-authored-by: Gene Hoffman <hoffmang9@users.noreply.github.com>
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Addressed both Bugbot follow-ups from review #5106460243 in 5c8f830:

Finding A — Quoted verdicts can invert headline (39f9eec7) — confirmed & fixed. The (?<=[^\w \t\n*]) glue lookbehind treated ", `, and ( as glue, so a later "Verdict: malicious" citation could become the last official match and invert the headline. Glue is now limited to sentence punctuation (?<=[.!?:;]) (keeps next.Verdict: benign), while line-start and properly paired bold verdicts still match.

Finding B — Unpaired bold breaks comment markdown (407ecfad) — confirmed & fixed. Optional (?:\*\*)? / (?:[ \t]*\*\*)? wrappers could consume an opening or closing ** without its pair, leaving orphan markers (e.g. **Verdict: benign because ….** → body contained because ….**). The regex now accepts either fully paired **Verdict: token** or token-only forms (including line-start **Verdict: token when the closer comes after same-line reasoning), and a post-removal pass strips orphan trailing ** at line ends. Same-line reasoning stays in the body.

All prior fixes (last valid match wins, no cross-line \s*/** stealing, no mid-sentence space-glue, test loader side effects) remain in place. 16 Python tests pass via make test.

Comment thread templates/dependency-cursor-review.yml Outdated
Replace blanket trailing-** stripping with targeted removal of orphan
closers on lines where an unpaired **Verdict: token match was spliced
out. Preserves legitimate **heading** and end-of-line emphasis in the
review body.

Co-authored-by: Gene Hoffman <hoffmang9@users.noreply.github.com>
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Addressed Bugbot follow-up from review #5106583328 in bfa300c:

Finding — Cleanup strips unrelated bold markers (ae3547cb) — confirmed & fixed. The post-removal re.sub(r"(?m)\*\*[ \t]*$", …) ran on every remaining body line, which stripped closing ** from legitimate headings and end-of-line emphasis (e.g. **Important finding****Important finding).

Replaced the blanket pass with _orphan_trailing_bold_span, which removes a trailing ** only on the same line as an unpaired **Verdict: token removal. Unrelated body bold is preserved; the prior unpaired-verdict orphan fix still applies on the affected line only. Added test_body_bold_markers_are_not_stripped_by_cleanup (17 tests, make test green).

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bfa300c. Configure here.

Comment thread templates/dependency-cursor-review.yml
_lstrip leading whitespace before detecting unpaired bold verdict
matches so indented **Verdict: token lines still get targeted orphan
trailing ** removal.

Co-authored-by: Gene Hoffman <hoffmang9@users.noreply.github.com>
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Addressed Bugbot follow-up from review #5106643567 on bfa300c:

Finding — Orphan cleanup skips indented verdicts (9cd86e42) — confirmed & fixed. _orphan_trailing_bold_span checked matched.startswith("**"), but VERDICT_RE includes leading ^[ \t]* in the match slice, so indented unpaired **Verdict: benign because …** lines skipped orphan cleanup and left trailing ** in the body. Now lstrip horizontal whitespace before the unpaired-bold check. Added test_indented_unpaired_bold_verdict_still_cleans_orphan_markers.

@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Babysitting complete on HEAD 26976b7. All GitHub checks are green, including Cursor Bugbot (pass/SUCCESS).

During babysitting, Bugbot on bfa300c reported one additional issue — Orphan cleanup skips indented verdicts (9cd86e42) — which was confirmed and fixed in 26976b7 by lstrip-ing horizontal whitespace before the unpaired-bold check in _orphan_trailing_bold_span. Re-pushed; CI and Bugbot re-ran clean with 0 new findings on HEAD.

Ready for human review. Not merging per instructions.

@hoffmang9
hoffmang9 merged commit 08e967c into main Sep 3, 2026
16 checks passed
@hoffmang9
hoffmang9 deleted the cursor/fix-verdict-formatter-bugbot-7c91 branch September 3, 2026 21:03
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.

3 participants