Skip to content

feat(SIG-107): add language and config tool batch - #28

Merged
damartinezjulio merged 3 commits into
mainfrom
sig-107-next-tool
Jun 12, 2026
Merged

feat(SIG-107): add language and config tool batch#28
damartinezjulio merged 3 commits into
mainfrom
sig-107-next-tool

Conversation

@damartinezjulio

Copy link
Copy Markdown
Contributor

What

  • Add default-on selective wrappers for Flake8, golangci-lint, HTMLHint, Stylelint, and TFLint.
  • Keep heavier scanners default-off while making the language/config tools runner-owned, bounded, and non-fatal.
  • Split TruffleHog converter tests out of the main SARIF test file to preserve the 1k-line file cap.
  • Carry TypeScript npm pack filename/integrity hardening.

Why

SIG-107 is moving the runner toward CodeRabbit-style deterministic tool coverage while keeping caller CI safe. This batch adds tools that should improve useful recall today for language/config review signal without executing caller configs/plugins by default.

Safety notes

  • Flake8 is marker-gated on .flake8, but ignores config content via --isolated and only runs high-confidence PyFlakes/parse checks.
  • golangci-lint requires root go.mod, uses --no-config --default=standard, and verifies the downloaded Linux artifact SHA256.
  • HTMLHint and Stylelint use runner-owned configs, verify pinned npm tarballs, install with scripts disabled, and emit SARIF through the normal Sigilix contract.
  • TFLint is default-on but only runs when Terraform files exist, verifies the downloaded Linux artifact SHA256, and emits empty SARIF on unavailable paths.

Verified

  • Full helper suite: sigilix_sarif_test, eslint_ts_workflow_test, tsc_workflow_test, pylint_workflow_test, trufflehog_converter_test, language_config_tools_workflow_test, knip_workflow_test, biome_workflow_test, docs_config_tools_test, oxlint_workflow_test, ast_grep_workflow_test
  • python3 -m py_compile .github/scripts/*.py
  • YAML parse for .github/workflows/ci.yml and .github/workflows/scan.yml
  • actionlint v1.7.12
  • git diff --check
  • tracked file line-count check: all files <= 1000 lines
  • Empty-repo wrapper smoke for Flake8, golangci-lint, HTMLHint, Stylelint, and TFLint
  • Positive smokes for Flake8 F821 isolation, HTMLHint severity preservation, Stylelint conversion, and TFLint SARIF contract
  • Reviewer model pass returned SHIP after the final integrity-check refutation

Linear: SIG-107

@sigilix

sigilix Bot commented Jun 12, 2026

Copy link
Copy Markdown

Sigilix Overview

Effort: 4/5 (large)

Quality gates

  • ✅ PR title follows convention
  • ✅ PR description is complete
  • ℹ️ PR is linked to an issue — No Closes #N / Closes SIG-N keyword found in PR body or commit messages.

Summary — latest push

Adds default-on wrappers for Flake8, golangci-lint, HTMLHint, Stylelint, and TFLint, enforcing runner-owned configs and pinned integrity checks to keep caller CI safe, alongside SARIF converters and a comprehensive test suite. The TFLint wrapper has a control-flow bug where a failed discovery command writes an empty file list that passes the subsequent non-empty check, causing the script to proceed into the download/execution path instead of emitting empty SARIF early.

Important files

File Score Notes Next step
.github/scripts/run_tflint.sh 5/5 TFLint wrapper with strict integrity and version checks, but contains a logic flaw where discovery failure writes to the file list and falls through to the download path instead of exiting early. Fix the discovery failure branch to call emit_empty_sarif and exit immediately rather than writing to the file list and falling through to the elif chain.
.github/scripts/language_config_tools_workflow_test.py 4/5 Thorough integration and converter tests validating workflow inputs, script contents, and SARIF conversion logic for all new tools. Add a test case for the TFLint script simulating a discovery failure to catch the current fallthrough bug.
.github/scripts/run_stylelint.sh 4/5 New shell wrapper that discovers CSS files, verifies the Stylelint npm tarball SHA-512 integrity, installs with --ignore-scripts, and converts JSON output to SARIF. Extract the shared npm-pack-verify-and-install pattern into a sourced helper to reduce duplication across the HTMLHint and Stylelint wrappers.
.github/scripts/run_golangci_lint.sh 4/5 New shell wrapper that downloads a pinned golangci-lint Linux binary, verifies its SHA-256 checksum, and runs with --no-config --default=standard. Add a test confirming the script emits empty SARIF when go.mod is absent but .go files exist.
.github/scripts/run_htmlhint.sh 4/5 New shell wrapper that discovers HTML files, verifies the HTMLHint npm tarball SHA-512 integrity, installs with --ignore-scripts, and pipes native SARIF through the contract validator. Extract the shared npm-pack-verify-and-install pattern into a sourced helper to reduce duplication across the HTMLHint and Stylelint wrappers.

Confidence: 2/5

The TFLint discovery failure fallthrough bug is a logic error that will cause the wrapper to attempt a download and scan in a repository with no Terraform files when discovery commands fail, undermining the safety guarantees.

  • In run_tflint.sh lines 37-39, if discover_terraform_files fails (exits non-zero), the output is still redirected into $files_list, and the subsequent grep -qz . check can succeed on residual or improperly handled output, bypassing the intended empty SARIF early exit.
  • Verify the Flake8 --select=E9,F63,F7,F82 codes align exactly with the _level_for_code E9-prefix logic in flake8_to_sarif.py to ensure no unexpected severity mappings.
  • Confirm that the npm integrity verification in the HTMLHint and Stylelint wrappers correctly handles npm pack --json output format variations across npm versions.
  • Ensure the golangci-lint wrapper correctly handles the case where go.mod exists but contains no Go files, validating it emits empty SARIF rather than a scan error.

Suggested labels: bug

@sigilix sigilix Bot added the enhancement New feature or request label Jun 12, 2026
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@damartinezjulio, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 hour, 47 minutes, and 12 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5ed2ed1-8ee1-44c2-a98c-ad2cb1fdda97

📥 Commits

Reviewing files that changed from the base of the PR and between c32bb44 and 7d2ce2b.

📒 Files selected for processing (18)
  • .github/config/htmlhint-sigilix.json
  • .github/config/stylelint-sigilix.json
  • .github/config/tool-manifest.json
  • .github/scripts/flake8_to_sarif.py
  • .github/scripts/language_config_tools_workflow_test.py
  • .github/scripts/run_flake8.sh
  • .github/scripts/run_golangci_lint.sh
  • .github/scripts/run_htmlhint.sh
  • .github/scripts/run_stylelint.sh
  • .github/scripts/run_tflint.sh
  • .github/scripts/run_tsc.sh
  • .github/scripts/sigilix_sarif_contract.py
  • .github/scripts/sigilix_sarif_test.py
  • .github/scripts/stylelint_to_sarif.py
  • .github/scripts/trufflehog_converter_test.py
  • .github/workflows/ci.yml
  • .github/workflows/scan.yml
  • README.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sig-107-next-tool

Comment @coderabbitai help to get the list of available commands and usage tips.

sigilix[bot]
sigilix Bot previously requested changes Jun 12, 2026

@sigilix sigilix 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.

1 high-severity finding + 1 lower-severity inline · coverage: logic · security · performance · tests

⚠️ 6 files not reviewed — the diff exceeded this review's size budget, so they were not analyzed: .github/scripts/sigilix_sarif_test.py, .github/scripts/stylelint_to_sarif.py, .github/scripts/trufflehog_converter_test.py, .github/workflows/ci.yml, .github/workflows/scan.yml, README.md. Split the PR or re-run to cover them.

2 findings · 1 inline · 1 PR-level · Proof: 1 grounded · Quiet: 1 demoted to body

The PR introduces several new deterministic tool wrappers with robust version pinning and integrity checks. However, the Flake8 wrapper contains a logic bug in its rule selection that causes intended PyFlakes checks to be skipped.

Severity Location Issue
High (4) .github/scripts/run_flake8.sh:82 logic: Flake8 --select includes F7 which is not a valid Flake8 code prefix — intended F7xx checks are silently skipped

Severity ≥ 4 only. Rows without a line number are file-level or PR-level findings that cannot be posted as inline review comments.

Evidence & suggested changes for body-only blockers (score ≥ 4)
  • P1 LOGICMODEL .github/scripts/run_flake8.sh:82: Flake8 --select includes F7 which is not a valid Flake8 code prefix — intended F7xx checks are silently skipped score 4 · MODEL
    The wrapper's --select=E9,F63,F7,F82 includes F7 as a standalone prefix, but Flake8 error codes use a letter followed by three digits (e.g. F701, F702, F811). The string F7 matches no real code, so the intended F7* family of PyFlakes checks is silently excluded. The PR body states the wrapper runs 'high-confidence PyFlakes/parse checks' via --select=E9,F63,F7,F82; the F7 entry is a no-op. Fix: replace F7 with the specific F7xx codes intended (e.g. F701,F702,F704,F706,F707,F811,F821,F822,F823,F831,F841), or use F7 as a prefix wildcard if Flake8 supports it (it does not — Flake8 --select requires full codes).

    Example:

    input: a Python file with `for i in range(10): pass` (redefined built-in `range`)
    actual: Flake8 with `--select=E9,F63,F7,F82` emits no finding for F811 (redefinition of unused name)
    expected: Flake8 emits F811 because `F811` is a high-confidence PyFlakes check the wrapper intends to include
    

    Suggested fix:

    --select=E9,F63,F701,F702,F704,F706,F707,F811,F821,F822,F823,F831,F841,F82

Dismiss: @sigilix dismiss <reason> in thread (not-a-bug|bad-anchor|already-covered|too-minor|wrong-context) · Re-run: /sigilix review · Review #1 · 03c5deb

Comment thread .github/scripts/run_tflint.sh
sigilix[bot]
sigilix Bot previously requested changes Jun 12, 2026

@sigilix sigilix 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.

1 finding posted inline · since last review: +1 new · 2 carried-over · coverage: logic · security · performance · tests

⚠️ 6 files not reviewed — the diff exceeded this review's size budget, so they were not analyzed: .github/scripts/sigilix_sarif_test.py, .github/scripts/stylelint_to_sarif.py, .github/scripts/trufflehog_converter_test.py, .github/workflows/ci.yml, .github/workflows/scan.yml, README.md. Split the PR or re-run to cover them.

1 finding · 1 inline · Proof: 1 grounded

The PR adds five new deterministic tool wrappers (Flake8, golangci-lint, HTMLHint, Stylelint, TFLint) with pinned versions and integrity checks, along with comprehensive workflow and converter tests.

Dismiss: @sigilix dismiss <reason> in thread (not-a-bug|bad-anchor|already-covered|too-minor|wrong-context) · Re-run: /sigilix review · Review #2 · 03c5deb

Comment thread .github/scripts/flake8_to_sarif.py
@damartinezjulio

Copy link
Copy Markdown
Contributor Author

@Sigilix dismiss not-a-bug

The review-body Flake8 F7 blocker is a false positive. I verified the exact runner dependency and option behavior with Flake8 7.3.0:

$ python -m flake8 --isolated --select=F7 f701.py
f701.py:1:1: F701 'break' outside loop

So F7 is accepted as a Flake8 select prefix and does not silently skip F7xx findings. The inline TFLint cleanup finding was valid and is fixed in fb65d0e; validation was rerun after that commit.

@sigilix sigilix 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.

1 non-blocking finding posted inline · since last review: +1 new · 2 likely-fixed · coverage: logic · security · performance · tests

⚠️ 6 files not reviewed — the diff exceeded this review's size budget, so they were not analyzed: .github/scripts/sigilix_sarif_test.py, .github/scripts/stylelint_to_sarif.py, .github/scripts/trufflehog_converter_test.py, .github/workflows/ci.yml, .github/workflows/scan.yml, README.md. Split the PR or re-run to cover them.

1 finding · 1 inline · Proof: 1 grounded

The PR adds five new language and configuration tool wrappers with robust integrity checks and SARIF conversion. The implementation is secure and logically sound, with one minor gap in unit test coverage for the Flake8 error classification logic.

Re-run: /sigilix review · fb65d0e

Comment thread .github/scripts/flake8_to_sarif.py
@sigilix

sigilix Bot commented Jun 12, 2026

Copy link
Copy Markdown

Dismissed — not a bug. The F7 prefix is a valid --select argument in Flake8 7.3.0 (the runner's pinned version); it matches all F7xx codes like F701/F702/F703 and does not silently skip them. The asker's --isolated --select=F7 reproduction confirms this. The TFLint inline-cleanup finding was valid and already fixed in fb65d0e, with validation rerun after that commit. No code change needed for the Flake8 F7 flag.

sigilix[bot]
sigilix Bot previously requested changes Jun 12, 2026

@sigilix sigilix 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.

1 high-severity finding · since last review: +1 new · 1 likely-fixed · coverage: logic · security · performance · tests

⚠️ 6 files not reviewed — the diff exceeded this review's size budget, so they were not analyzed: .github/scripts/sigilix_sarif_test.py, .github/scripts/stylelint_to_sarif.py, .github/scripts/trufflehog_converter_test.py, .github/workflows/ci.yml, .github/workflows/scan.yml, README.md. Split the PR or re-run to cover them.

1 finding · 1 inline · Proof: 1 grounded

The PR introduces five new language and configuration tool wrappers with strong integrity guards and version pinning. However, there is a logic bug in the Flake8 SARIF converter that misclassifies certain critical parse errors as warnings.

Severity Location Issue
High (4) .github/scripts/flake8_to_sarif.py:40-42 logic: Flake8 _level_for_code misclassifies E9xx codes that do not start with E9 (e.g. E901) as warnings instead of errors

Dismiss: @sigilix dismiss <reason> in thread (not-a-bug|bad-anchor|already-covered|too-minor|wrong-context) · Re-run: /sigilix review · Review #3 · 7d2ce2b

Comment thread .github/scripts/flake8_to_sarif.py
@damartinezjulio
damartinezjulio dismissed stale reviews from sigilix[bot], sigilix[bot], and sigilix[bot] June 12, 2026 17:10

Dismissed after Sigilix acknowledged this as not-a-bug in PR comment: Flake8 7.3.0 accepts F7 as a select prefix and direct reproduction reports F701.

@damartinezjulio
damartinezjulio merged commit 157449b into main Jun 12, 2026
6 checks passed
@damartinezjulio
damartinezjulio deleted the sig-107-next-tool branch June 12, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant