Skip to content

JS-1513 peach-check skill: filter ANSI script-preview lines, clarify parallelism rule#6701

Open
francois-mora-sonarsource wants to merge 36 commits intomasterfrom
fix/skill-peach-check
Open

JS-1513 peach-check skill: filter ANSI script-preview lines, clarify parallelism rule#6701
francois-mora-sonarsource wants to merge 36 commits intomasterfrom
fix/skill-peach-check

Conversation

@francois-mora-sonarsource
Copy link
Copy Markdown
Contributor

Summary

  • Add /\[36;1m/b to Phase 1 and Phase 2 sed scripts to skip GitHub Actions ANSI-colored script-preview lines, preventing false-positive matches (e.g. echo "All 3 attempts failed" being mistaken for a clone timeout)
  • Add a paragraph clarifying that issuing multiple independent Bash calls in a single response is the correct way to parallelize — it is separate from and does not violate the no-chaining rule
  • Add .codex and Claude MCP config files to .gitignore

Test Plan

  • Run /peach-check and verify Phase 1 output no longer includes spurious ANSI-colored script-preview lines

🤖 Generated with Claude Code

francois-mora-sonarsource and others added 15 commits March 13, 2026 09:56
- Fix description to use triggering conditions only (not workflow summary)
- Remove invalid disable-model-invocation frontmatter field
- Flatten step numbering (remove single-phase wrapper)
- Expand grep pattern to cover clone timeouts, project misconfiguration, and dep failures
- Make parallel agents opt-in for ambiguous cases only (classify directly when obvious)
- Compress agent prompt template (remove verbose XML block)
- Fix cluster check to handle null completedAt by falling back to log timestamps

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… new categories

- Add Step 3b: when ≥80% of jobs fail, sample 5 jobs and apply the dominant
  verdict to all instead of triaging every job individually
- Replace Phase 1 tail-based approach with grep: cleanup steps that run after
  scan failures push the exit code out of the tail window; grep is more reliable
- Add two new IGNORE categories to docs/peach-main-analysis.md:
  - Peach server unreachable (HTTP 502/503 at scan start)
  - Artifact expired (HTTP 410 during JAR download)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ass failure verdict rules

- Replace one-liner grep with multi-line sed -n scripts for readability
- Add allowed-tools header and prerequisites section
- Clarify mass failure verdict rules: CRITICAL takes priority when stack trace
  originates in org.sonar.plugins.javascript, even for mass failures
- Add exit code 3 disambiguation: always run Phase 2 for exit code 3 failures
- Rename "SonarJS Analyzer Crash" to "SonarJS Plugin Failure" to cover
  plugin initialization failures (not just sensor-level crashes)
- Add example log excerpt for plugin initialization failure
- Fix step numbering (3b → 4, renumber remaining steps accordingly)
- Move cluster check to its own Step 7, after classification

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…lure

- Fix forward ref in Step 2: completedAt null handling is in Step 7, not Step 8
- Add explicit flowchart branch for plugin initialization failure (no sensor
  name but org.sonar.plugins.javascript in stack trace → CRITICAL), preventing
  it from falling through to the IGNORE branch

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…d detection patterns

- Save logs to target/peach-logs/ instead of piping, enabling Phase 2 without re-download
- Inline --jq filter in gh api calls so only failed jobs are returned (no client-side filtering)
- Add sed --sandbox flag to prevent e command injection from untrusted log content
- Add Step 1b: auto-rerun cancelled runs and stop early
- Add ETARGET/notarget/503 patterns to Phase 1 filter
- Fix Phase 3 to use Read tool on saved log instead of re-fetching via gh api
- Add command discipline section (no &&/;/| chaining)
- Add mass failure CRITICAL-takes-priority rule
- Add Detection patterns to every failure category in docs/peach-main-analysis.md
- Add canonical sed pattern reference section to docs/peach-main-analysis.md

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…-preview lines

- Clarify that the no-chaining rule (no &&/;/|) and parallel execution
  (multiple Bash calls in one response) are orthogonal concerns
- Add /\[36;1m/b to Phase 1 and Phase 2 sed scripts to skip GitHub
  Actions script-preview lines, preventing spurious pattern matches
  on lines like `echo "All 3 attempts failed"` that appear in every
  job log as part of the pre-execution script display

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Keep HEAD changes: ANSI script-preview line filter (/\[36;1m/b) in
Phase 1 and Phase 2 sed scripts, and the paragraph clarifying that
parallel Bash calls are distinct from chaining.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@francois-mora-sonarsource francois-mora-sonarsource requested a review from a team March 27, 2026 09:10
@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha bot commented Mar 27, 2026

Summary

This PR refines the peach-check skill for triaging SonarJS Peach Main Analysis workflow failures, with emphasis on filtering ANSI artifacts and clarifying failure classification.

Core technical fix: Added /\[36;1m/b to Phase 1 and Phase 2 sed scripts to skip GitHub Actions ANSI-colored script-preview lines (cyan bold). This prevents false-positive matches—for example, an echo statement printing "All 3 attempts failed" is no longer mistaken for an actual clone timeout.

Structural improvements:

  • Restructured job collection (Step 2) to use paginated gh api with jq -s instead of three separate calls, then filter failed jobs in a single query
  • Shifted to metadata-first classification before log download: determine failure phase and step context from API metadata, then download logs only for jobs needing log-based triage
  • Added explicit handling for diff-validation-aggregated (excluded entirely) and Diff Val monitoring steps (classified as IGNORE)
  • Enhanced failure categories with new patterns: Node heap exhaustion, Diff Val monitoring, upstream repo removal, scanner bootstrap timeouts, and post-scan report-upload timeouts
  • Updated output format guidance: group findings by shared cause instead of one-row-per-job table

Clarifications added:

  • "Last Sensor Wins" rule: the most recent active sensor before a stack trace owns the failure, not necessarily the first sensor that appeared
  • Parallel execution is separate from the no-chaining rule—multiple independent Bash calls in one response is correct parallelization
  • Guidance on distinguishing pre-scan, analyze, and post-scan phase boundaries (e.g., a failure in the Analyze project step may actually be post-scan if JS analysis completed before report upload failed)
  • Removed auto-rerun of cancelled runs; now only recommends rerun and stops triage

What reviewers should know

Start here: Review .claude/skills/peach-check/SKILL.md for the step-by-step procedure changes and sed script updates. The /\[36;1m/b line is the most critical addition—test it by running /peach-check on a recent run to confirm Phase 1 output no longer includes spurious ANSI-colored lines.

Key decision points:

  • Step 2 now uses jq -s to slurp paginated responses. Reviewers should verify the JSON merging logic is correct when the API returns multiple pages.
  • Metadata-first classification (Step 5) assumes the job API provides enough context (conclusion, step names, completedAt) to classify many jobs without log inspection. Confirm this matches real Peach API responses.
  • The diff-validation-aggregated exclusion is intentional—it should not appear in failure counts, findings, or ignored-list outputs. Check for any hardcoded references that might need updating.

Testing notes:

  • The test plan mentions verifying Phase 1 output with /peach-check. Run this against both a recent run (with ANSI escapes present) and an older run (without, to ensure no regressions).
  • If you have access to a Peach run with post-scan failures (e.g., ReportPublisher.upload timeouts), verify Phase 2 correctly surfaces them as post-scan rather than analyzer failures.
  • The "Last Sensor Wins" rule is critical for multi-sensor runs—if a non-SonarJS sensor appears after JS sensor completes, confirm it is correctly attributed to the later sensor.

Non-code changes: .gitignore additions (.codex/, .mcp.json) are housekeeping for tooling artifacts—low risk.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title peach-check skill: filter ANSI script-preview lines, clarify parallelism rule JS-1513 peach-check skill: filter ANSI script-preview lines, clarify parallelism rule Mar 27, 2026
@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod bot commented Mar 27, 2026

JS-1513

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Ruling Report

No changes to ruling expected issues in this PR

sonar-review-alpha[bot]

This comment was marked as outdated.

@sonarqube-next
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
No data about Coverage
No data about Duplication

See analysis details on SonarQube

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as resolved.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

README Freshness Check

The rules README is out of date.

A fix PR has been created: #6752

Please review and merge it into your branch.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

…uidance

- Fix missing spaces in allowed-tools frontmatter between Bash() entries
- Add explicit callout of the echo && command anti-pattern in the
  Command discipline section
- Clarify Phase 1 instructions: write job name as plain text, then
  issue sed as a standalone Bash call with no prefix

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

- Fix path inconsistency: create target/peach-logs upfront in Step 2
  and write jobs.json to target/jobs.json; remove redundant mkdir in Step 5
- Clarify completedAt is reliably null in paginated API; always use log timestamps
- Add SocketTimeoutException and ReportPublisher.upload to Phase 1 filter so
  report-upload timeout failures can be classified without needing Phase 2
- Sync docs/peach-main-analysis.md Phase 1 patterns section

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@sonarqube-next
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

🗣️ Give feedback

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