Skip to content

analysis: add all-security tier packs and onboarding/test coverage#813

Merged
PrzemyslawKlys merged 14 commits intomasterfrom
codex/reviewer-static-next-level-security-tiers
Feb 26, 2026
Merged

analysis: add all-security tier packs and onboarding/test coverage#813
PrzemyslawKlys merged 14 commits intomasterfrom
codex/reviewer-static-next-level-security-tiers

Conversation

@PrzemyslawKlys
Copy link
Member

Summary

  • add explicit cross-language security tiers: all-security-50, all-security-100, all-security-500
  • keep all-security-default as stable baseline alias and make security tiers build on it
  • update setup/onboarding examples (CLI + web) to advertise all-security-50
  • extend tests for security-tier pack resolution and analyze list-rules security-tier counts
  • update static-analysis and pack docs with new security tier IDs

Why

  • expands static-analysis pack contracts for security-focused adoption beyond our internal defaults
  • gives external users stable security-tier IDs consistent with existing *-50|100|500 tier patterns

Validation

  • dotnet run --project IntelligenceX.Cli/IntelligenceX.Cli.csproj --framework net8.0 -- analyze validate-catalog --workspace .
  • dotnet build IntelligenceX.Cli/IntelligenceX.Cli.csproj -c Release
  • dotnet build IntelligenceX.Tests/IntelligenceX.Tests.csproj -c Release
  • dotnet ./IntelligenceX.Tests/bin/Release/net8.0/IntelligenceX.Tests.dll
  • dotnet ./IntelligenceX.Tests/bin/Release/net10.0/IntelligenceX.Tests.dll
  • pwsh -NoLogo -NoProfile -File .agents/skills/intelligencex-analysis-gate/scripts/run-analysis-suite.ps1 -Mode fast

@intelligencex-review
Copy link

intelligencex-review bot commented Feb 26, 2026

IntelligenceX Review

Reviewing PR #813: analysis: add all-security tier packs and onboarding/test coverage
Reviewed commit: df5bea2

Merge blockers: items in Todo List ✅ and Critical Issues ⚠️ sections (if present). Other Issues 🧯 are suggestions.

Auto-resolve (AI): 0 resolved, 6 kept (candidates 6, assessments 6, attempts 0, evidence_rejected 3, resolve_failed 0, sweep_resolved 0, missing_ids 0, duplicate_ids 0).

Summary 📝

Adds new cross-language pack tiers (all-security-*, all-multilang-*), updates onboarding/docs, and introduces exclude-path: support for internal maintainability filtering with targeted tests. Overall direction is solid and test coverage is materially improved.

Todo List ✅

None.

Critical Issues ⚠️ (if any)

None.

Other Issues 🧯

  • Analysis/Catalog/rules/internal/IXLOC001.json: adding exclude-path:IntelligenceX.Cli/Setup/Web/Assets/wizard.js hard-codes a repo-specific exception into a generic LOC rule; this increases long-term maintenance risk because rule behavior now depends on one concrete file path.
  • IntelligenceX.Cli/Analysis/AnalyzeRunCommand.InternalMaintainability.SourceFiltering.cs: CollapseRepeatedPathSeparators uses a replace-in-loop pattern; while fine for short strings, a single-pass normalization would be easier to reason about and avoids repeated allocations.
  • TODO.md: PR-specific reviewer-churn notes are being accumulated in source control; this can become stale process metadata and reduce signal in a product backlog file.

Tests / Coverage 🧪

  • Good: added pack-resolution tests for new security and multilang tiers.
  • Good: added analyze list-rules tier-count assertions for new tiers.
  • Good: added focused normalization and end-to-end tests for exclude-path semantics, including exact-match behavior and rooted-path rejection.
  • No coverage gaps found in changed behavior that would block merge.

Next Steps 🚀

  • Merge as-is.
  • Consider moving the wizard.js LOC exclusion to pack/environment-specific config (or a narrower rule override path) to keep base internal rule definitions repo-agnostic.

Static Analysis Policy 🧭

  • Config mode: respect
  • Packs: All Essentials (50)
  • Rules: 108 enabled
  • Rule list display: up to 10 items per section
  • Enabled rules preview: CA2000 (Dispose objects before losing scope), CA1062 (Validate arguments of public methods), SA1600 (Elements should be documented), CA1016 (Mark assemblies with assembly version), CA1018 (Mark attributes with AttributeUsageAttribute), CA1041 (Provide ObsoleteAttribute message), CA1047 (Do not declare protected member in sealed type), CA1050 (Declare types in namespaces), CA1061 (Do not hide base class methods), CA1067 (Override Object.Equals(object) when implementing IEquatable<T>) (truncated)
  • Result files: 2 input patterns, 1 matched, 1 parsed, 0 failed
  • Status: pass
  • Rule outcomes: 0 with findings, 108 clean
  • Failing rules: none
  • Clean rules: CA2000 (Dispose objects before losing scope), CA1062 (Validate arguments of public methods), SA1600 (Elements should be documented), CA1016 (Mark assemblies with assembly version), CA1018 (Mark attributes with AttributeUsageAttribute), CA1041 (Provide ObsoleteAttribute message), CA1047 (Do not declare protected member in sealed type), CA1050 (Declare types in namespaces), CA1061 (Do not hide base class methods), CA1067 (Override Object.Equals(object) when implementing IEquatable<T>) (truncated)
  • Outside-pack rules: none

Static Analysis 🔎

  • Findings: 0 (no issues at or above configured severity)

IntelligenceX thread triage

Assessed commit: df5bea2

Diff range: PR base → head (78c4e63..df5bea2)

Needs attention:

  • PRRT_kwDORAe5k85w4DDx: The concern remains valid: excluded paths are still stored in a case-insensitive set, which conflicts with strict exact/case-sensitive semantics.
  • PRRT_kwDORAe5k85w4DEv: No diff evidence shows alignment to case-sensitive exact matching; membership check still depends on the case-insensitive set built earlier.
  • PRRT_kwDORAe5k85w4lx-: The implementation still uses iterative global replacement of "//"; no change shown to a segment-based/non-overlapping strategy as requested.
  • PRRT_kwDORAe5k85w30HT: The diff introduces separate exclude-path handling with exact path matching instead of prefix/root matching, directly addressing over-match concerns. (missing diff evidence)
  • PRRT_kwDORAe5k85w33m_: The test setup no longer creates a directory at the same path used for a file; it now uses a different subdirectory and validates exact-file exclusion behavior. (missing diff evidence)
  • PRRT_kwDORAe5k85w4Z6D: Normalization is now symmetric and explicit (slash unification + repeated separator collapse), and tests cover both file-side and tag-side normalization/mixed separators. (missing diff evidence)

Model & Usage 🤖

  • Model: gpt-5.3-codex
  • Length: medium
  • Mode: hybrid
  • Reasoning: not configured
  • Usage: 5h limit: 77% remaining | weekly limit: 19% remaining | code review weekly limit: 100% remaining | credits: 0

@PrzemyslawKlys PrzemyslawKlys merged commit 4f612a6 into master Feb 26, 2026
8 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the codex/reviewer-static-next-level-security-tiers branch February 26, 2026 15:51
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