Skip to content

t3275: fix quality-debt review feedback for milestone-validation-worker.sh#4633

Closed
alex-solovyev wants to merge 1 commit intomainfrom
bugfix/issue-3275-milestone-validation-quality-debt
Closed

t3275: fix quality-debt review feedback for milestone-validation-worker.sh#4633
alex-solovyev wants to merge 1 commit intomainfrom
bugfix/issue-3275-milestone-validation-quality-debt

Conversation

@alex-solovyev
Copy link
Copy Markdown
Collaborator

Summary

Addresses all 5 findings from the CodeRabbit review on PR #2519 for .agents/scripts/milestone-validation-worker.sh.

Changes

CRITICAL fixes

  • Reject --max-retries 0: Updated regex from ^[0-9]+$ to ^[1-9][0-9]*$ — zero was accepted and caused the validation loop to be skipped entirely, returning a false PASS with no checks run.
  • Avoid early abort on dep install failure: Changed return 1 to return 0 in check_dependencies so a failed pkg install records the failure via record_fail and continues to report generation and fix-task creation, rather than aborting the script under set -e.

HIGH fix

  • ShellCheck covers nested scripts: Removed -maxdepth 1 from the find invocation — now uses -type f to recurse into all subdirectories under .agents/scripts/, matching the project guideline .agents/scripts/**/*.sh.

MEDIUM fixes

  • Lint/type-check is advisory: Changed record_failrecord_warning for all three linter paths (JS/TS run lint, TypeScript tsc --noEmit, Python ruff check). Lint issues are now warnings that appear in the report but do not flip VALIDATION_PASSED to false.
  • --json output mode: Added JSON_OUTPUT flag and --json CLI option. When active, all human-readable log functions (log_info, log_error, log_success, log_warn) are suppressed. At the end of the run, generate_json_report emits a single JSON object to stdout:
    {"mission_id":"...","milestone":1,"total_checks":5,"failures_count":0,"warnings_count":1,"skipped_count":1,"failures":[],"warnings":[{"message":"..."}],"exit_code":0}
    Exit status behaviour is unchanged — the script still exits non-zero on failures.

Verification

  • ShellCheck: zero new violations (pre-existing SC1091 info note on sourced file unchanged)
  • All 5 CodeRabbit findings addressed

Closes #3275

…tion-worker.sh

- CRITICAL: reject --max-retries 0 to prevent false PASS with zero checks
- CRITICAL: return 0 on dep install failure to avoid early abort before report/fix-task creation
- HIGH: remove -maxdepth 1 from ShellCheck find to cover nested .agents/scripts/**/*.sh
- MEDIUM: demote lint/type-check failures to record_warning (advisory, not blocking)
- MEDIUM: add --json flag for machine-readable orchestrator output (suppresses human logs, emits JSON summary with total_checks, failures_count, warnings_count, failures[], warnings[], exit_code)

Closes #3275
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 14, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 14, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • no-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ac7e637b-a182-49a2-bb87-61007c6d7eab

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/issue-3275-milestone-validation-quality-debt
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 413 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Sat Mar 14 05:58:58 UTC 2026: Code review monitoring started
Sat Mar 14 05:58:59 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 413

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 413
  • VULNERABILITIES: 0

Generated on: Sat Mar 14 05:59:01 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link
Copy Markdown

@marcusquinn
Copy link
Copy Markdown
Owner

Closing as duplicate — PR #4631 targets the same issue (t3275) and was opened first. Merging #4631 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Auto-created from TODO.md tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quality-debt: .agents/scripts/milestone-validation-worker.sh — PR #2519 review feedback (critical)

2 participants