Skip to content

fix: preserve deliberate skip dispositions across scans - #836

Open
eminos wants to merge 1 commit into
peteromallet:mainfrom
eminos:fix/preserve-accepted-scan-dispositions
Open

eminos wants to merge 1 commit into
peteromallet:mainfrom
eminos:fix/preserve-accepted-scan-dispositions

Conversation

@eminos

@eminos eminos commented Oct 10, 2026

Copy link
Copy Markdown

Problem

An ordinary plan skip <id> --false-positive --note ... --attest ... can survive one scan and reopen on the next matching scan, even when the source has not changed. Skipping removes the ID from the queue but retains cluster membership. Scan reconciliation restores the FP status, then supersedes its non-actionable cluster reference and deletes the skip entry. The following scan has no disposition to restore.

The same cleanup discards permanent skips. An ordinary re-skip cannot repair an affected FP durably because the old superseded tombstone deletes the new skip before status synchronization. This occurs in active and inactive clusters and is independent of explicit language selection.

Fix

Detach deliberate permanent/false-positive dispositions from queue, promoted IDs, cluster members, and step references while preserving their skip evidence. Clear an earlier superseded tombstone when a present finding has an explicitly restored durable skip. Keep existing superseding behavior for missing findings and unskipped resolved work.

The change is confined to scan-plan reconciliation and its regression tests. It does not add source fingerprints, config exclusions, review imports, or score overrides.

Validation

  • Twenty new regression cases verify three matching scans, FP and permanent skips, manual and auto clusters in all four execution states, ordinary re-skip recovery from existing tombstones, and disappearance cleanup. Before the fix, 18 new cases fail; afterward all 121 focused tests pass. Ruff and git diff --check also pass.
  • The full suite on the reviewed integration base reports 5,874 passed, 70 failed, 4 skipped. Its exact 70 failed node IDs match the unmodified 95205c1e base, whose result is 5,854 passed / 70 failed / 4 skipped. Fifty-five base failures explicitly concern parser grammar-download DNS; 15 parser assertions also reproduce on the base. Full-suite success remains unverified in this environment.
  • This PR contains only this reconciliation fix on freshly fetched upstream main (3a7735d5). Its 121 focused tests and Ruff pass; the full suite reports 5,761 passed, 70 failed, 4 skipped. The exact 70 failed node IDs match the already reproduced failures on that exact unmodified upstream base. Integration and upstream pass counts differ because the integration checkout contains separate pending tool fixes; this PR excludes them.
  • A read-only in-memory replay of real project disposition receipts retains all 124 original skips for three matching merge/reconcile cycles. A separate replay restores the 67 already reopened findings through ordinary resolution/skip engine paths and preserves them for three cycles. Live source, state, plan, and evidence files remain unchanged. This replay is not a fresh scan or score claim.
  • Two actual PHP scans and their immediate next commands exit0 in an owned coherent source snapshot of the project that exposed the bug. The scans redetect 67 and 68 accepted findings, respectively, while retaining all 124 FP statuses and original skip records, with zero superseded dispositions. The unchanged sample module is detected in both scans. The 846 archived source files preserve their accepted Git hashes; canonical scanner files remain byte unchanged. The snapshot uses an explicit isolated state path and an attested --force-rescan solely to exercise repeated scans outside the canonical queue. PHPStan reports reduced coverage and jscpd exits with errors, so neither PHPStan nor duplication coverage is claimed. The tested reconciliation source is byte-identical to this PR's source; no external review or score override was used.

This PR contains only the reconciliation fix; the separate queue-target/state-path PR #835 remains unchanged.

github-actions Bot added a commit to citizenadam/desloppify that referenced this pull request Oct 11, 2026

This branch has not been deployed

No deployments
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