Plan skip restoration leaves false-positive findings with stale reopened metadata

Open
  • Is something wrong?
#855 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
python
Domain
backend

Research direction

Start with the reproduction steps, which use only in-memory state: call upsert_issues in desloppify/engine/_state/merge_issues.py, then reconcile_plan_after_scan in desloppify/engine/_plan/scan_issue_reconcile.py, and compare the row's note, reopen_count, and resolution fields. The maintainers have not decided which side owns the fix, so confirm the contract with them before editing. Done means a regression test asserts the agreed terminal fields for a plan-skipped false positive.

Written by the indexing model from the issue text.

Description

Problem

A finding with a durable plan skip --false-positive disposition can be returned by the next scan, briefly reopened by scan merge, and then restored to false_positive during post-scan plan reconciliation. The terminal state can retain the scan's Reopened (×N) note, a higher reopen_count, and null resolved_at/resolution_attestation even though the final status is false_positive and the plan still contains the accepted skip evidence.

This makes the terminal state ambiguous: it looks both accepted and unresolved, and the reopen counter reports churn that was immediately superseded by the durable plan disposition.

Minimal in-memory reproduction

No project files or persisted state are required:

  1. Create one synthetic test_coverage work item with status false_positive, a prior resolved_at, and a resolution_attestation.
  2. Add a matching plan entry under plan["skipped"] with kind false_positive, note accepted, and attestation reviewed.
  3. Pass the same finding through engine._state.merge_issues.upsert_issues.
  4. Pass the plan and state through engine._plan.scan_issue_reconcile.reconcile_plan_after_scan.

Observed after upsert_issues:

status = open
reopen_count = 1
note = "Reopened (×1) — reappeared in scan (was false_positive)"
resolved_at = null
resolution_attestation = null

Observed after reconcile_plan_after_scan:

status = false_positive
reopen_count = 1
note = "Reopened (×1) — reappeared in scan (was false_positive)"
resolved_at = null
resolution_attestation = null
plan["skipped"][id]["kind"] = "false_positive"
plan["skipped"][id]["note"] = "accepted"
plan["skipped"][id]["attestation"] = "reviewed"

Code path

  • desloppify/engine/_state/merge_issues.py::upsert_issues increments reopen_count, clears resolution metadata, and writes the Reopened note when a previously resolved finding is present in current detector output.
  • desloppify/engine/_plan/scan_issue_reconcile.py::_sync_skipped_issue_statuses restores an open skipped finding to the skip-kind status by assigning status only.
  • desloppify/app/commands/scan/workflow.py persists state after scan merge and again after post-scan reporting, so the restored terminal status can be persisted.

Expected behavior / question

Please clarify the supported contract for a durable plan skip:

  • If the plan skip is the canonical disposition, should scan merge avoid treating the accepted finding as a user-resolution reopen, or should reconciliation restore coherent state metadata from the plan skip?
  • If the reopen event is intentionally retained as history, should it be represented separately so a final false_positive row does not carry an unresolved-looking note and null resolution metadata?

A regression test should assert the chosen terminal contract for status, reopen count, note, resolved_at, resolution_attestation, and plan skip evidence.

Related work

  • PR836 preserves deliberate skip dispositions across scans and cleans stale queue/cluster references. This report concerns the state-row metadata left after status restoration.
  • PR798 persists reconciled scan scores and history. This report concerns per-finding terminal metadata, not score persistence.
  • Issue807 tracks conflicting scan scores in one invocation and is related to the pre/post-reconciliation visibility problem.
Dominant language
Python
Stars
3.8k
Forks
301
No merged PRs in 30d

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from peteromallet/desloppify

All issues in peteromallet/desloppify

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.