Phase 0 Research: Sonar BUG and BLOCKER Remediation
The authoritative per-issue inventory (rule, file:line, message) is research/sonar-inventory.txt, pulled from the SonarCloud public API (https://sonarcloud.io/api/issues/search?componentKeys=Priivacy-ai_spec-kitty&issueStatuses=OPEN,CONFIRMED). This document records the approach decisions, not per-issue fixes (those land in implementation).
Decision: fix at the root, never suppress
NOSONAR is never used to clear an issue. A genuine false positive is resolved by a written rationale (PR body / inline comment explaining why the code is correct), not a suppression.
the failure rather than closing it.
quality gate and the tautological tests would still prove nothing.
- Decision: Every issue is resolved by a real code/test change. A
# noqa/# type: ignore/ - Rationale: Charter Sonar Expectations — "Prefer real fixes over suppression." Suppressions move
- Alternatives considered: bulk-suppress the test-quality rules — rejected: it green-washes the
Decision: mechanical vs investigate seam (the two-WP split)
asserts, S8998 empty parametrize) and an investigate stream (S2083 path-injection, S3516 always-same-return, S2583 always-true).
correct shape. The investigate stream needs per-case judgment (real bug vs intentional-but-smelly vs trusted-local false positive) and touches src control flow / security — a different risk profile and review depth. Operator pre-approved this split.
security/logic judgment, muddying review.
- Decision: Split the work into a mechanical stream (S5863 tautological asserts, S5779 swallowed
- Rationale: The mechanical stream is largely local, low-risk test-integrity repair with a clear
- Alternatives considered: one flat pass — rejected: mixes low-risk mechanical edits with
Decision: S2083 (path-injection) — determine trusted-local vs external-input per site
from external/user input reachable at runtime, validate/contain it (reject traversal, anchor under an allowed root) and add a test exercising the rejection. If it derives from trusted repo/mission internal data, keep the semantics and record a rationale (C-001) — do not add sanitization theatre.
is noise; real external vectors must be contained.
classify the source; decide contain-vs-rationale per site.
- Decision: For each of the 3 sites, trace the tainted path component to its source. If it derives
- Rationale: Charter loopback/local-only special case — forcing sanitization on trusted-local paths
- Method: read the call chain feeding
bookkeeping_projection.py:212,346andskills/verifier.py:402;
Decision: S3516 / S2583 (degenerate control flow) — classify real-bug vs intentional
vary but cannot — fix the logic, prove with a behavioral test) or an intentional constant (a protocol-conforming stub / defensive guard — remove the smell: drop the vacuous return, tighten the signature/return type, or make the invariant explicit) — never suppress.
charter requires the real fix, and every new branch needs a test (C-002).
- Decision: For each site, determine whether the degeneracy is a real defect (a branch that should
- Rationale: Sonar cannot tell intent; a blind "add a branch" could invent wrong behavior. The
Decision: red-first for recoverable assertions (S5863)
write the corrected assertion so it fails against the current (buggy) behavior first (red-first evidence), then make it pass. Where intent is unrecoverable, remove the assertion with a one-line rationale rather than guess a wrong comparison.
bite.
- Decision: For each tautological assertion whose intended comparison is recoverable from context,
- Rationale: NFR-003; a tautology "passing" tells us nothing — the corrected form must be shown to
Out of scope (recorded)
(doctrine/charter/sync), per operator sequencing. C-003 bounds this mission to the 41 BUG+BLOCKER.
- HIGH-severity maintainability (S3776 complexity, S1192 dup-literals) — separate per-module missions