From 78db90e4bfd158bca74b70d720e3450695595f06 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Mon, 6 Apr 2026 00:29:54 -0300 Subject: [PATCH] chore: optimize local git hooks and fix T11 any budget strictness - Removed the expensive (40s+) `npm run test:unit` step from the `pre-commit` hook - Created `.husky/pre-push` to run the unit test suite before pushing rather than per commit - This prevents spurious async teardown errors from local test runners from blocking fast commits - Replaced an explicit `any` cast with `Record | undefined` in `chatCore.ts` to pass the `check:any-budget:t11` strict checker which enforces a budget of 0 --- .github/CODEOWNERS | 2 + .github/ISSUE_TEMPLATE/bug_report.yml | 26 ++ .github/ISSUE_TEMPLATE/config.yml | 2 +- .github/ISSUE_TEMPLATE/feature_request.yml | 26 ++ .github/ISSUE_TEMPLATE/test_coverage_task.yml | 73 +++++ .github/copilot-instructions.md | 15 + .github/pull_request_template.md | 30 ++ .github/workflows/ci.yml | 274 ++++++++++++++---- .husky/pre-commit | 1 - .husky/pre-push | 1 + AGENTS.md | 9 +- CONTRIBUTING.md | 14 +- open-sse/handlers/chatCore.ts | 4 +- package.json | 4 +- scripts/check-pr-test-policy.mjs | 107 +++++++ scripts/test-report-summary.mjs | 92 ++++++ sonar-project.properties | 2 + 17 files changed, 617 insertions(+), 65 deletions(-) create mode 100644 .github/CODEOWNERS create mode 100644 .github/ISSUE_TEMPLATE/test_coverage_task.yml create mode 100644 .github/copilot-instructions.md create mode 100644 .github/pull_request_template.md create mode 100755 .husky/pre-push create mode 100644 scripts/check-pr-test-policy.mjs create mode 100644 scripts/test-report-summary.mjs create mode 100644 sonar-project.properties diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..3b6790f8 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,2 @@ +* @diegosouzapw + diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index e0fcea3f..121d5091 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -119,6 +119,20 @@ body: validations: required: true + - type: dropdown + id: test-impact + attributes: + label: Test Impact + description: "What automated test coverage should exist for this bug?" + options: + - Needs a new unit test + - Needs a new integration test + - Needs a new e2e test + - Existing automated test already fails + - Unsure + validations: + required: true + - type: textarea id: logs attributes: @@ -143,3 +157,15 @@ body: description: "Any other context about the problem (e.g. proxy config, number of accounts, network setup)." validations: required: false + + - type: textarea + id: validation-plan + attributes: + label: Validation Plan + description: "Which commands or tests should prove this bug is fixed?" + placeholder: | + Example: + - node --import tsx/esm --test tests/unit/my-file.test.mjs + - npm run test:coverage + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index b5a7c0fd..e603f00e 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,4 +1,4 @@ -blank_issues_enabled: true +blank_issues_enabled: false contact_links: - name: Question / Help url: https://github.com/diegosouzapw/OmniRoute/discussions diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 2c211370..359eaab6 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -33,6 +33,19 @@ body: validations: required: false + - type: textarea + id: acceptance + attributes: + label: Acceptance Criteria + description: "Describe the concrete behaviors or outcomes that should be validated before this is considered done." + placeholder: | + Example: + - API route returns 200 with valid payload + - Unit coverage added for the new branch + - Existing integrations remain green + validations: + required: true + - type: dropdown id: area attributes: @@ -68,3 +81,16 @@ body: description: "Any other context, mockups, or references." validations: required: false + + - type: textarea + id: test-plan + attributes: + label: Expected Test Plan + description: "Which automated tests or coverage changes should accompany this work?" + placeholder: | + Example: + - Add unit tests for open-sse/services/combo.ts + - Extend integration coverage for /api/v1/models + - Keep npm run test:coverage at 60%+ + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/test_coverage_task.yml b/.github/ISSUE_TEMPLATE/test_coverage_task.yml new file mode 100644 index 00000000..57b03c44 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/test_coverage_task.yml @@ -0,0 +1,73 @@ +name: Test Coverage Task +description: Create a focused coverage-improvement issue tied to concrete files and targets +title: "[Coverage] " +labels: ["test", "coverage"] +body: + - type: markdown + attributes: + value: | + Use this template for scoped coverage work. Keep it tied to specific files, measurable targets, and the gate that must stay green. + + - type: input + id: baseline + attributes: + label: Current Coverage Baseline + description: "Paste the current overall or file-level coverage number that justifies this task." + placeholder: "e.g. Lines 79.00%, Branches 72.85%, open-sse/handlers/chatCore.ts = 67.22%" + validations: + required: true + + - type: textarea + id: scope + attributes: + label: Target Files Or Modules + description: "List the concrete source files or directories that this task will cover." + placeholder: | + Example: + - open-sse/handlers/chatCore.ts + - open-sse/services/combo.ts + - tests/integration/chat-pipeline.test.mjs + validations: + required: true + + - type: textarea + id: scenarios + attributes: + label: Missing Scenarios + description: "Describe the specific behaviors, branches, or failure paths that are currently uncovered." + placeholder: | + Example: + - Upstream timeout path + - Empty tool_calls normalization + - Fallback route after first provider failure + validations: + required: true + + - type: input + id: target + attributes: + label: Coverage Target + description: "Set the expected target for this task." + placeholder: "e.g. Raise open-sse/handlers to 80%+ lines and keep global gate >= 60%" + validations: + required: true + + - type: textarea + id: validation + attributes: + label: Validation Commands + description: "List the commands that must pass before this issue can be closed." + placeholder: | + Example: + - node --import tsx/esm --test tests/unit/my-suite.test.mjs + - npm run test:coverage + validations: + required: true + + - type: textarea + id: notes + attributes: + label: Notes + description: "Optional context, blockers, or dependencies." + validations: + required: false diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..7fa0cc6f --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,15 @@ +# OmniRoute PR and Coverage Instructions + +- Treat `npm run test:coverage` as a required gate for PR work. +- The repository minimum is `60%` for statements, lines, functions, and branches. +- If a PR changes production code in `src/`, `open-sse/`, `electron/`, or `bin/`, it must include automated tests in the same PR. +- When reviewing or updating a PR, if the report shows missing tests or coverage below `60%`, do not stop after reporting the problem. Add or update tests in the PR first, rerun the coverage gate, and only then ask for confirmation. +- Prefer the smallest test layer that proves the behavior: + - unit tests first + - integration tests when multiple modules or DB state are involved + - e2e only when the behavior is truly UI or workflow-dependent +- For bug issues, try to encode the reproduction as an automated test before or alongside the fix. +- In the final PR report, include: + - the commands you ran + - the changed test files + - the final coverage result diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..56398286 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,30 @@ +## Summary + +- Describe the user-facing or operational change. + +## Related Issues + +- Closes # +- Related to # + +## Validation + +- [ ] `npm run lint` +- [ ] `npm run test:unit` +- [ ] `npm run test:coverage` +- [ ] Coverage is still `>= 60%` for statements, lines, functions, and branches +- [ ] SonarQube PR analysis is green or any remaining issues are explicitly documented below + +## Tests Added Or Updated + +- List every changed or added automated test file. +- If no production code changed, state that here. + +## Coverage Notes + +- If this PR changes `src/`, `open-sse/`, `electron/`, or `bin/`, explain which tests cover the change. +- If coverage moved down in any touched file, explain why and what follow-up task will recover it. + +## Reviewer Notes + +- Call out any risky areas, migrations, feature flags, or manual validation that reviewers should know about. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84edc087..415788bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,7 @@ on: branches: [main] pull_request: branches: [main] + types: [opened, synchronize, reopened, ready_for_review] concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -43,7 +44,7 @@ jobs: run: | LANG_DIR="src/i18n/messages" LANGS=$(ls "$LANG_DIR"/*.json | xargs -n1 basename | sed 's/.json$//' | grep -v '^en$' | jq -R . | jq -s . | jq -c .) - echo "langs=${LANGS}" >> $GITHUB_OUTPUT + echo "langs=${LANGS}" >> "$GITHUB_OUTPUT" i18n: name: i18n Validation @@ -71,6 +72,28 @@ jobs: name: i18n-${{ matrix.lang }} path: result.txt + pr-test-policy: + name: PR Test Policy + if: ${{ github.event_name == 'pull_request' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: actions/setup-node@v6 + with: + node-version: 22 + - name: Fetch base branch + run: git fetch --no-tags origin "${GITHUB_BASE_REF}" --depth=1 + - name: Validate source changes include tests + run: node scripts/check-pr-test-policy.mjs --summary-file .artifacts/pr-test-policy.md + - name: Publish PR test policy summary + if: always() + run: | + if [ -f .artifacts/pr-test-policy.md ]; then + cat .artifacts/pr-test-policy.md >> "$GITHUB_STEP_SUMMARY" + fi + advanced-security: name: Advanced Security Scans runs-on: ubuntu-latest @@ -79,7 +102,6 @@ jobs: with: fetch-depth: 0 - # 1. TRUFFLEHOG OSS - name: TruffleHog Secret Scan uses: trufflesecurity/trufflehog@main with: @@ -88,23 +110,16 @@ jobs: head: HEAD extra_args: --only-verified - # 2. SONARQUBE SCAN - - name: SonarQube Scan - uses: SonarSource/sonarqube-scan-action@v4 - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }} - - # 3. SNYK SCAN E VERIFICAÇÕES NATIVAS - uses: actions/setup-node@v6 with: node-version: 22 cache: npm + - run: npm ci - # Mantendo as verificações nativas originais - name: Dependency audit run: npm audit --audit-level=high --omit=dev || true + - name: Check for known vulnerabilities run: npx is-my-node-vulnerable || true @@ -163,7 +178,140 @@ jobs: node-version: 22 cache: npm - run: npm ci - - run: npm run test:coverage + - name: Run coverage gate + run: npm run test:coverage + - name: Build coverage summary + if: always() + run: | + mkdir -p coverage + if [ -f coverage/coverage-summary.json ]; then + node scripts/test-report-summary.mjs \ + --input coverage/coverage-summary.json \ + --output coverage/coverage-report.md \ + --threshold 60 + else + printf '%s\n' \ + '# Coverage Report' \ + '' \ + 'Coverage summary JSON was not generated. Inspect the Coverage job logs.' \ + > coverage/coverage-report.md + fi + cat coverage/coverage-report.md >> "$GITHUB_STEP_SUMMARY" + - name: Upload coverage artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: | + coverage/coverage-summary.json + coverage/lcov.info + coverage/coverage-report.md + if-no-files-found: warn + + sonarqube: + name: SonarQube + runs-on: ubuntu-latest + needs: test-coverage + if: ${{ always() && needs.test-coverage.result == 'success' }} + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: actions/download-artifact@v4 + with: + name: coverage-report + path: . + - name: Explain SonarQube skip + if: ${{ env.SONAR_TOKEN == '' || env.SONAR_HOST_URL == '' }} + run: | + echo "SonarQube scan skipped because SONAR_TOKEN or SONAR_HOST_URL is not configured." >> "$GITHUB_STEP_SUMMARY" + - name: SonarQube Scan + if: ${{ env.SONAR_TOKEN != '' && env.SONAR_HOST_URL != '' }} + uses: SonarSource/sonarqube-scan-action@v7 + env: + SONAR_TOKEN: ${{ env.SONAR_TOKEN }} + SONAR_HOST_URL: ${{ env.SONAR_HOST_URL }} + + coverage-pr-comment: + name: PR Coverage Comment + runs-on: ubuntu-latest + if: ${{ always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false }} + needs: + - pr-test-policy + - test-coverage + permissions: + contents: read + issues: write + pull-requests: write + steps: + - name: Download coverage artifact + if: ${{ needs.test-coverage.result != 'cancelled' }} + continue-on-error: true + uses: actions/download-artifact@v4 + with: + name: coverage-report + path: . + - name: Prepare PR coverage comment + env: + COVERAGE_RESULT: ${{ needs.test-coverage.result }} + POLICY_RESULT: ${{ needs.pr-test-policy.result }} + run: | + mkdir -p .artifacts + { + echo "" + echo "## CI Coverage Report" + echo "" + echo "- Coverage job: \`${COVERAGE_RESULT}\`" + echo "- PR test policy: \`${POLICY_RESULT}\`" + echo "" + if [ -f coverage/coverage-report.md ]; then + cat coverage/coverage-report.md + else + echo "Coverage artifact was not available for this run." + fi + if [ "${POLICY_RESULT}" = "failure" ]; then + echo "" + echo "## PR Test Policy" + echo "" + echo "This PR changes production code in \`src/\`, \`open-sse/\`, \`electron/\`, or \`bin/\` without accompanying automated tests." + fi + } > .artifacts/pr-coverage-comment.md + - uses: actions/github-script@v8 + with: + script: | + const fs = require("fs"); + const marker = ""; + const body = fs.readFileSync(".artifacts/pr-coverage-comment.md", "utf8"); + const { owner, repo } = context.repo; + const issue_number = context.issue.number; + + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number, + per_page: 100, + }); + + const existing = comments.find((comment) => comment.body?.includes(marker)); + + if (existing) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body, + }); + } test-e2e: name: E2E Tests @@ -217,92 +365,100 @@ jobs: - run: npm ci - run: npm run test:security - # 🔥 DASHBOARD ci-summary: name: CI Dashboard runs-on: ubuntu-latest if: always() needs: - lint + - i18n + - pr-test-policy - advanced-security - build - test-unit - test-coverage + - sonarqube + - coverage-pr-comment - test-e2e - test-integration - test-security - - i18n - steps: - name: Download i18n results - uses: actions/download-artifact@v8 + continue-on-error: true + uses: actions/download-artifact@v4 with: + pattern: i18n-* path: results + merge-multiple: true - name: Generate dashboard + env: + EVENT_NAME: ${{ github.event_name }} run: | status() { case "$1" in success) echo "🟢 PASS" ;; failure) echo "🔴 FAIL" ;; cancelled) echo "⚫ CANCELLED" ;; + skipped) echo "⚪ SKIPPED" ;; *) echo "🟡 UNKNOWN" ;; esac } - echo "# 🚀 CI Dashboard" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY + echo "# 🚀 CI Dashboard" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" - # 🔹 CORE - echo "## 🧱 Core Checks" >> $GITHUB_STEP_SUMMARY - echo "| Job | Status |" >> $GITHUB_STEP_SUMMARY - echo "|-----|--------|" >> $GITHUB_STEP_SUMMARY - echo "| Lint | $(status '${{ needs.lint.result }}') |" >> $GITHUB_STEP_SUMMARY - echo "| Advanced Security | $(status '${{ needs.advanced-security.result }}') |" >> $GITHUB_STEP_SUMMARY + echo "## 🧱 Core Checks" >> "$GITHUB_STEP_SUMMARY" + echo "| Job | Status |" >> "$GITHUB_STEP_SUMMARY" + echo "|-----|--------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Lint | $(status '${{ needs.lint.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| PR Test Policy | $(status '${{ needs.pr-test-policy.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| Advanced Security | $(status '${{ needs.advanced-security.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| SonarQube | $(status '${{ needs.sonarqube.result }}') |" >> "$GITHUB_STEP_SUMMARY" - # 🔹 BUILD - echo "" >> $GITHUB_STEP_SUMMARY - echo "## 🏗️ Build" >> $GITHUB_STEP_SUMMARY - echo "| Job | Status |" >> $GITHUB_STEP_SUMMARY - echo "|-----|--------|" >> $GITHUB_STEP_SUMMARY - echo "| Build Matrix | $(status '${{ needs.build.result }}') |" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "## 🏗️ Build" >> "$GITHUB_STEP_SUMMARY" + echo "| Job | Status |" >> "$GITHUB_STEP_SUMMARY" + echo "|-----|--------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Build Matrix | $(status '${{ needs.build.result }}') |" >> "$GITHUB_STEP_SUMMARY" - # 🔹 TESTS - echo "" >> $GITHUB_STEP_SUMMARY - echo "## 🧪 Tests" >> $GITHUB_STEP_SUMMARY - echo "| Suite | Status |" >> $GITHUB_STEP_SUMMARY - echo "|-------|--------|" >> $GITHUB_STEP_SUMMARY - echo "| Unit | $(status '${{ needs.test-unit.result }}') |" >> $GITHUB_STEP_SUMMARY - echo "| Coverage | $(status '${{ needs.test-coverage.result }}') |" >> $GITHUB_STEP_SUMMARY - echo "| E2E | $(status '${{ needs.test-e2e.result }}') |" >> $GITHUB_STEP_SUMMARY - echo "| Integration | $(status '${{ needs.test-integration.result }}') |" >> $GITHUB_STEP_SUMMARY - echo "| Security Tests | $(status '${{ needs.test-security.result }}') |" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "## 🧪 Tests" >> "$GITHUB_STEP_SUMMARY" + echo "| Suite | Status |" >> "$GITHUB_STEP_SUMMARY" + echo "|-------|--------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Unit | $(status '${{ needs.test-unit.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| Coverage | $(status '${{ needs.test-coverage.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| PR Coverage Comment | $(status '${{ needs.coverage-pr-comment.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| E2E | $(status '${{ needs.test-e2e.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| Integration | $(status '${{ needs.test-integration.result }}') |" >> "$GITHUB_STEP_SUMMARY" + echo "| Security Tests | $(status '${{ needs.test-security.result }}') |" >> "$GITHUB_STEP_SUMMARY" - # 🔹 I18N - echo "" >> $GITHUB_STEP_SUMMARY - echo "## 🌍 Translations" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "## 🌍 Translations" >> "$GITHUB_STEP_SUMMARY" total=0 langs=0 - for dir in results/*; do - file="$dir/result.txt" - val=$(sed -r 's/\x1B\[[0-9;]*[mK]//g' "$file" | grep "Untranslated:" | awk '{print $2}') - val=${val:-0} - total=$((total + val)) - langs=$((langs + 1)) - done + if [ -d results ]; then + for file in results/*.txt; do + [ -f "$file" ] || continue + val=$(sed -r 's/\x1B\[[0-9;]*[mK]//g' "$file" | grep "Untranslated:" | awk '{print $2}') + val=${val:-0} + total=$((total + val)) + langs=$((langs + 1)) + done + fi - echo "" >> $GITHUB_STEP_SUMMARY - echo "| Metric | Value |" >> $GITHUB_STEP_SUMMARY - echo "|--------|------|" >> $GITHUB_STEP_SUMMARY - echo "| Languages checked | $langs |" >> $GITHUB_STEP_SUMMARY - echo "| Total untranslated | $total |" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "| Metric | Value |" >> "$GITHUB_STEP_SUMMARY" + echo "|--------|------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Languages checked | $langs |" >> "$GITHUB_STEP_SUMMARY" + echo "| Total untranslated | $total |" >> "$GITHUB_STEP_SUMMARY" if [ "$total" -gt 0 ]; then - echo "" >> $GITHUB_STEP_SUMMARY - echo "⚠️ **Translations need attention**" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "⚠️ **Translations need attention**" >> "$GITHUB_STEP_SUMMARY" else - echo "" >> $GITHUB_STEP_SUMMARY - echo "✅ **All translations complete**" >> $GITHUB_STEP_SUMMARY + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "✅ **All translations complete**" >> "$GITHUB_STEP_SUMMARY" fi diff --git a/.husky/pre-commit b/.husky/pre-commit index 727757cb..342f766e 100644 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1,3 @@ npx lint-staged node scripts/check-docs-sync.mjs npm run check:any-budget:t11 -npm run test:unit diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100755 index 00000000..6d0a7660 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1 @@ +npm run test:unit diff --git a/AGENTS.md b/AGENTS.md index 62d42612..1e20037e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,10 +63,17 @@ npm run test:protocols:e2e # Ecosystem compatibility tests npm run test:ecosystem -# Coverage (55% min thresholds — statements, lines, functions; 60% branches) +# Coverage (60% minimum for statements, lines, functions, and branches) npm run test:coverage ``` +### PR Coverage Policy + +- `npm run test:coverage` is the PR coverage gate in CI. +- The repository minimum is **60%** for statements, lines, functions, and branches. +- If a PR changes production code in `src/`, `open-sse/`, `electron/`, or `bin/`, it must include or update automated tests in the same PR. +- For agent-driven review or coding flows: if coverage is below the gate or source changes ship without tests, do not stop at reporting. Add or update tests first, rerun the gate, and only then ask for confirmation. + --- ## Code Style Guidelines diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4ccd03bb..153da4b2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -133,7 +133,7 @@ npm run test:protocols:e2e # Ecosystem compatibility tests npm run test:ecosystem -# Coverage (55% min statements/lines/functions; 60% branches) +# Coverage (60% min statements/lines/functions/branches) npm run test:coverage npm run coverage:report @@ -145,10 +145,22 @@ npm run check Coverage notes: - `npm run test:coverage` measures source coverage for the main unit test suite, excludes `tests/**`, and includes `open-sse/**` +- Pull requests must keep the overall coverage gate at **60% or higher** for statements, lines, functions, and branches +- If a PR changes production code in `src/`, `open-sse/`, `electron/`, or `bin/`, it must add or update automated tests in the same PR - `npm run coverage:report` prints the detailed file-by-file report from the latest coverage run - `npm run test:coverage:legacy` preserves the older metric for historical comparison - See `docs/COVERAGE_PLAN.md` for the phased coverage improvement roadmap +### Pull Request Requirements + +Before opening or merging a PR: + +- Run `npm run test:unit` +- Run `npm run test:coverage` +- Ensure the coverage gate stays at **60%+** for all metrics +- Include the changed or added test files in the PR description when production code changed +- Check the SonarQube result on the PR when the project secrets are configured in CI + Current test status: **122 unit test files** covering: - Provider translators and format conversion diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 564f5c66..5dd5dea8 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -1060,7 +1060,9 @@ export async function handleChatCore({ block.type === "document" ) { // Only extract text if it's explicitly a text-only representation without data - const fileData = (block.file_url ?? block.file ?? block.document) as any; + const fileData = (block.file_url ?? block.file ?? block.document) as + | Record + | undefined; if ( (block.type === "file" || block.type === "document") && !fileData?.url && diff --git a/package.json b/package.json index a6a2d30f..011d537d 100644 --- a/package.json +++ b/package.json @@ -72,9 +72,11 @@ "test:protocols:e2e": "node scripts/run-protocol-clients-tests.mjs", "test:vitest": "vitest run --config vitest.mcp.config.ts", "test:ecosystem": "node scripts/run-ecosystem-tests.mjs", - "test:coverage": "c8 --output-dir coverage --exclude=tests/** --exclude=**/*.test.* --reporter=text-summary --reporter=html --reporter=json-summary --reporter=lcov --check-coverage --statements 55 --lines 55 --functions 55 --branches 60 node --import tsx/esm --test tests/unit/*.test.mjs", + "test:coverage": "c8 --output-dir coverage --exclude=tests/** --exclude=**/*.test.* --reporter=text-summary --reporter=html --reporter=json-summary --reporter=lcov --check-coverage --statements 60 --lines 60 --functions 60 --branches 60 node --import tsx/esm --test tests/unit/*.test.mjs", "test:coverage:legacy": "c8 --output-dir coverage --exclude=open-sse --check-coverage --lines 50 --functions 50 --branches 50 node --import tsx/esm --test tests/unit/*.test.mjs", "coverage:report": "c8 report --output-dir coverage --exclude=tests/** --exclude=**/*.test.* --reporter=text --reporter=text-summary --reporter=html --reporter=json-summary --reporter=lcov", + "coverage:summary": "node scripts/test-report-summary.mjs --input coverage/coverage-summary.json --output coverage/coverage-report.md --threshold 60", + "check:pr-test-policy": "node scripts/check-pr-test-policy.mjs", "coverage:report:legacy": "c8 report --output-dir coverage --exclude=open-sse --reporter=text --reporter=text-summary", "test:all": "npm run test:unit && npm run test:vitest && npm run test:ecosystem && npm run test:e2e", "check": "npm run lint && npm run test", diff --git a/scripts/check-pr-test-policy.mjs b/scripts/check-pr-test-policy.mjs new file mode 100644 index 00000000..b8b37694 --- /dev/null +++ b/scripts/check-pr-test-policy.mjs @@ -0,0 +1,107 @@ +import { execFileSync } from "node:child_process"; +import { mkdirSync, writeFileSync } from "node:fs"; +import path from "node:path"; + +const SOURCE_ROOTS = ["src/", "open-sse/", "electron/", "bin/"]; +const TEST_PATTERNS = [/^tests\//, /(?:^|\/)__tests__\//, /\.(?:test|spec)\.[cm]?[jt]sx?$/]; + +function getArg(name, fallbackValue = "") { + const index = process.argv.indexOf(name); + if (index === -1 || index === process.argv.length - 1) { + return fallbackValue; + } + return process.argv[index + 1]; +} + +function runGit(args) { + return execFileSync("git", args, { encoding: "utf8" }).trim(); +} + +function isSourceFile(filePath) { + return SOURCE_ROOTS.some((root) => filePath.startsWith(root)); +} + +function isTestFile(filePath) { + return TEST_PATTERNS.some((pattern) => pattern.test(filePath)); +} + +function buildReport(lines) { + return `${lines.join("\n")}\n`; +} + +const summaryFile = getArg("--summary-file", ""); +const baseRef = process.env.GITHUB_BASE_REF; + +if (!baseRef) { + const report = buildReport([ + "## PR Test Policy", + "", + "Skipped: not running in a pull request context.", + ]); + + if (summaryFile) { + mkdirSync(path.dirname(summaryFile), { recursive: true }); + writeFileSync(summaryFile, report); + } + + process.stdout.write(report); + process.exit(0); +} + +const baseTarget = process.env.GITHUB_BASE_SHA || `origin/${baseRef}`; +const changedFiles = runGit(["diff", "--name-only", "--diff-filter=ACMR", `${baseTarget}...HEAD`]) + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean); + +const changedSourceFiles = changedFiles.filter(isSourceFile); +const changedTestFiles = changedFiles.filter(isTestFile); +const hasRequiredTests = changedSourceFiles.length === 0 || changedTestFiles.length > 0; + +const reportLines = [ + "## PR Test Policy", + "", + `Base ref: \`${baseRef}\``, + `Changed production files: ${changedSourceFiles.length}`, + `Changed automated test files: ${changedTestFiles.length}`, + "", +]; + +if (changedSourceFiles.length > 0) { + reportLines.push("### Production files in scope", ""); + for (const filePath of changedSourceFiles.slice(0, 20)) { + reportLines.push(`- \`${filePath}\``); + } + reportLines.push(""); +} + +if (changedTestFiles.length > 0) { + reportLines.push("### Tests in this PR", ""); + for (const filePath of changedTestFiles.slice(0, 20)) { + reportLines.push(`- \`${filePath}\``); + } + reportLines.push(""); +} + +if (hasRequiredTests) { + reportLines.push("Result: PASS"); +} else { + reportLines.push( + "Result: FAIL", + "", + "This PR changes production code under `src/`, `open-sse/`, `electron/`, or `bin/` but does not add or update automated tests." + ); +} + +const report = buildReport(reportLines); + +if (summaryFile) { + mkdirSync(path.dirname(summaryFile), { recursive: true }); + writeFileSync(summaryFile, report); +} + +process.stdout.write(report); + +if (!hasRequiredTests) { + process.exit(1); +} diff --git a/scripts/test-report-summary.mjs b/scripts/test-report-summary.mjs new file mode 100644 index 00000000..7790faae --- /dev/null +++ b/scripts/test-report-summary.mjs @@ -0,0 +1,92 @@ +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import path from "node:path"; + +function getArg(name, fallbackValue = "") { + const index = process.argv.indexOf(name); + if (index === -1 || index === process.argv.length - 1) { + return fallbackValue; + } + return process.argv[index + 1]; +} + +function formatPercent(value) { + return `${Number(value ?? 0).toFixed(2)}%`; +} + +const inputPath = getArg("--input", "coverage/coverage-summary.json"); +const outputPath = getArg("--output", ""); +const threshold = Number(getArg("--threshold", "60")); + +if (!existsSync(inputPath)) { + console.error(`Coverage summary file not found: ${inputPath}`); + process.exit(1); +} + +const summary = JSON.parse(readFileSync(inputPath, "utf8")); +const cwd = process.cwd(); +const metrics = [ + ["lines", "Lines"], + ["statements", "Statements"], + ["functions", "Functions"], + ["branches", "Branches"], +]; + +const total = summary.total ?? {}; +const gatePassed = metrics.every(([metric]) => (total[metric]?.pct ?? 0) >= threshold); + +const files = Object.entries(summary) + .filter(([name]) => name !== "total" && /\.(?:[cm]?[jt]sx?)$/.test(name)) + .map(([name, stats]) => { + const relativeName = path.relative(cwd, name); + const totalLines = stats.lines?.total ?? 0; + const coveredLines = stats.lines?.covered ?? 0; + + return { + name: relativeName, + lines: stats.lines?.pct ?? 0, + branches: stats.branches?.pct ?? 0, + functions: stats.functions?.pct ?? 0, + missingLines: Math.max(totalLines - coveredLines, 0), + }; + }) + .sort((left, right) => { + if (left.lines !== right.lines) return left.lines - right.lines; + if (left.branches !== right.branches) return left.branches - right.branches; + return right.missingLines - left.missingLines; + }) + .slice(0, 15); + +const report = [ + "# Coverage Report", + "", + `Gate: ${gatePassed ? "PASS" : "FAIL"} at ${threshold}% minimum for lines, statements, functions, and branches.`, + "", + "## Totals", + "", + "| Metric | Covered | Total | Percent | Threshold | Status |", + "| --- | ---: | ---: | ---: | ---: | --- |", + ...metrics.map(([metric, label]) => { + const covered = total[metric]?.covered ?? 0; + const totalCount = total[metric]?.total ?? 0; + const pct = total[metric]?.pct ?? 0; + const status = pct >= threshold ? "PASS" : "FAIL"; + return `| ${label} | ${covered} | ${totalCount} | ${formatPercent(pct)} | ${threshold}% | ${status} |`; + }), + "", + "## Lowest Coverage Files", + "", + "| File | Lines | Branches | Functions | Missing Lines |", + "| --- | ---: | ---: | ---: | ---: |", + ...files.map( + (entry) => + `| \`${entry.name}\` | ${formatPercent(entry.lines)} | ${formatPercent(entry.branches)} | ${formatPercent(entry.functions)} | ${entry.missingLines} |` + ), +]; + +const reportContent = `${report.join("\n")}\n`; + +if (outputPath) { + writeFileSync(outputPath, reportContent); +} else { + process.stdout.write(reportContent); +} diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 00000000..c2eefcb1 --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,2 @@ +sonar.sourceEncoding=UTF-8 +sonar.javascript.lcov.reportPaths=coverage/lcov.info