Skip to content

ci: SonarQube CI integration (no GitHub App required)#15

Merged
brettheap merged 1 commit into
mainfrom
ci-sonarqube-integration
May 17, 2026
Merged

ci: SonarQube CI integration (no GitHub App required)#15
brettheap merged 1 commit into
mainfrom
ci-sonarqube-integration

Conversation

@brettheap
Copy link
Copy Markdown
Contributor

Summary

Wires SonarQube into the project via GitHub Actions, replacing the would-be SonarCloud / SonarQube GitHub App integration. PR decoration is handled directly by the official `SonarSource/sonarqube-scan-action@v6` reading the GitHub Actions PR-context env vars + a long-lived `SONAR_TOKEN` — no third-party app needs repo access.

What this enables

  • Push to main → main-branch baseline analysis on the Sonar server.
  • Every pull_request → PR-mode analysis with Sonar findings posted as PR comments.
  • `workflow_dispatch` → manual re-run from the Actions UI.
  • Works against SonarCloud out of the box and against self-hosted SonarQube via the optional `SONAR_HOST_URL` repo variable.

Required repo configuration

Add the following before merging:

  • Secret `SONAR_TOKEN` — long-lived analysis token from your Sonar instance (Account → Security → Generate Tokens). Needs at least "Execute Analysis" on the project.
  • (Optional) Variable `SONAR_HOST_URL` — defaults to `https://sonarcloud.io\`. Set to e.g. `https://sonar.example.com\` for self-hosted.

Files changed

  • `.github/workflows/sonarqube.yml` — new workflow (run pytest with coverage → run scanner).
  • `sonar-project.properties` — expanded to include test paths, Python version, coverage report path, encoding.
  • `pyproject.toml` — added `pytest-cov` to the `[test]` extra; configured `[tool.coverage.*]` blocks. Daemon entrypoint + `SubprocessTmuxAdapter` omitted from coverage since their behavior is dominated by integration tests not in the Sonar gate.
  • `README.md` — new "SonarQube CI" section documenting the secrets / variables.
  • `tests/unit/test_register_idempotency.py` — bumped 5 ms → 50 ms inter-call sleep so the strict-greater timestamp assertion stays stable under coverage instrumentation.

Test plan

  • `pytest tests/unit --cov --cov-report=xml` succeeds locally (1432 tests pass under coverage on main).
  • `coverage.xml` is produced as valid Cobertura XML.
  • After merging, configure `SONAR_TOKEN` and trigger the workflow.
  • Verify the first push to main publishes the baseline.

Out of scope

  • The fresh-container E2E (`tests/e2e/`) is not part of the Sonar gate; it requires Docker + real tmux and is meant for manual / nightly runs.
  • Integration tests (`tests/integration/`) need the `agenttower` console script on PATH + isolated daemon harness; they run in a separate, slower workflow that can be added later.

🤖 Generated with Claude Code

Replaces the project's would-be SonarCloud / SonarQube GitHub App
integration with a CI-driven path. A new workflow runs unit tests
with coverage, then invokes the official SonarSource scanner action.
PR decoration is handled by the scanner reading the standard
GitHub Actions PR-context env vars + a long-lived SONAR_TOKEN —
no third-party GitHub App needs repo access.

Components:

* `.github/workflows/sonarqube.yml` — new workflow. Runs on push to
  main, every pull_request, and workflow_dispatch. Steps:
  1. Checkout with fetch-depth: 0 (Sonar needs full blame history).
  2. Set up Python 3.12 with pip cache.
  3. Editable install of the package's `[test]` extra.
  4. Run `pytest tests/unit --cov --cov-report=xml`. The unit-only
     scope keeps the gate fast + green; the integration suite is
     covered separately and the fresh-container E2E is out of scope
     for the Sonar gate.
  5. Upload `coverage.xml` as a workflow artifact (7-day retention).
  6. Run the SonarSource/sonarqube-scan-action@v6. Picks up project
     metadata from `sonar-project.properties`.
* `sonar-project.properties` — expanded:
  - Added `sonar.tests=tests` so test code is attributed correctly.
  - Added `sonar.python.version=3.11,3.12` for the right linter rules.
  - Added `sonar.python.coverage.reportPaths=coverage.xml`.
  - Added `sonar.test.inclusions=tests/**`.
  - Added `sonar.coverage.exclusions=...` mirror of sonar.exclusions.
  - Added `sonar.sourceEncoding=UTF-8`.
  - Added `.tmp/`, `**/__pycache__/**` to the existing exclusions.
* `pyproject.toml` — added `pytest-cov>=4,<6` to the test extra and
  pinned pytest + coverage configuration under `[tool.pytest.ini_options]`,
  `[tool.coverage.run]`, `[tool.coverage.report]`, `[tool.coverage.xml]`.
  Daemon entry-point + the SubprocessTmuxAdapter are omitted from
  coverage (their behavior is dominated by integration tests which
  this gate doesn't run).
* `README.md` — new "SonarQube CI" section documenting the required
  `SONAR_TOKEN` secret and the optional `SONAR_HOST_URL` variable
  override for self-hosted SonarQube.
* `tests/unit/test_register_idempotency.py` — bumped the inter-call
  sleep from 5 ms → 50 ms so the strict-greater-than timestamp
  assertion stays stable under coverage instrumentation's tracing
  overhead (microsecond-precision clocks otherwise compress sub-ms
  gaps).

Required repo configuration before the workflow can publish results:

* Secret `SONAR_TOKEN`: a long-lived analysis token from the Sonar
  instance, scoped at minimum to "Execute Analysis" on the project.
* (Optional, self-hosted) Variable `SONAR_HOST_URL`: defaults to
  `https://sonarcloud.io` when unset.

Tested:
* 1432 unit tests pass with `--cov` on main (the FEAT-009 unit tests
  not yet merged add ~500 more in their branch).
* `coverage.xml` is produced and parses as Cobertura XML.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 14, 2026 12:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a GitHub Actions–based SonarQube/SonarCloud scan pipeline that runs unit tests with coverage and uploads analysis results, alongside project configuration updates to generate/consume coverage.xml and document the setup.

Changes:

  • Introduces a new .github/workflows/sonarqube.yml workflow to run pytest with coverage and invoke SonarSource/sonarqube-scan-action@v6.
  • Expands sonar-project.properties and pyproject.toml with Sonar + coverage configuration (pytest-cov, coverage settings, coverage XML output).
  • Updates docs and tests to support stable coverage runs (README Sonar section; unit test sleep adjustment).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.github/workflows/sonarqube.yml New CI workflow for coverage + Sonar scanning and PR analysis.
sonar-project.properties Defines Sonar analysis scope, test paths, coverage report path, and exclusions.
pyproject.toml Adds pytest-cov and centralizes pytest/coverage configuration, including XML output.
README.md Documents required repo secret/variable configuration for Sonar runs.
tests/unit/test_register_idempotency.py Increases sleep to keep timestamp ordering assertion stable under coverage instrumentation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sonar-project.properties
Comment on lines +31 to +32
# Coverage exclusions — same as sonar.exclusions for the coverage
# report's view of the codebase.
Comment thread sonar-project.properties
sonar.sources=src,scripts
sonar.exclusions=tests/**,specs/**,.specify/**,openspec/**
sonar.tests=tests
sonar.exclusions=tests/**,specs/**,.specify/**,openspec/**,.tmp/**,**/__pycache__/**
Comment thread pyproject.toml
addopts = "-ra"

[tool.coverage.run]
source = ["src/agenttower"]
Comment on lines +42 to +44
# pull-requests:write lets sonar-scanner's PR decoration post
# comments via the GitHub Actions OIDC token. Sonar's official
# action reads this scope when present.
Comment on lines +97 to +103
- name: SonarQube scan
uses: SonarSource/sonarqube-scan-action@v6
env:
# Long-lived analysis token. Add to the repo's secrets:
# Settings → Secrets and variables → Actions → New
# repository secret.
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
@sonarqubecloud
Copy link
Copy Markdown

@brettheap brettheap merged commit 6504552 into main May 17, 2026
3 of 6 checks passed
@brettheap brettheap deleted the ci-sonarqube-integration branch May 17, 2026 17:02
brettheap pushed a commit that referenced this pull request May 24, 2026
Applies fixes for all 5 findings from the 2026-05-24 Round 4
/speckit.analyze pass: I-N1 (PackageInfo wire), C-N1 (T171
tracking), U-N1 (Settings scroll-to-section), I-N2 (docstring),
S-N1 (TaskList housekeeping — silent, no file edits).

I-N1 (installedAppVersionProvider stub, MEDIUM): Round 3's D1 fix
unified version reads through `installedAppVersionProvider` — but
the provider returned the placeholder `'0.0.0-dev'` so
VersionBadge, VersionDisplayTile, and the diagnostics bundle all
displayed `v0.0.0-dev`. main.dart now awaits
`PackageInfo.fromPlatform()` before runApp and overrides
`installedAppVersionProvider` with the real installed version.
The provider's default stub stays for widget tests.

C-N1 (OS-native dispatch unwired, MEDIUM): added T171 to
tasks.md tracking the missing FR-058 dispatch wiring.
`OsNativeIntegration.consider()` exists but has zero callers —
T171 specifies the Riverpod listener that fires the dispatcher
for newly-arrived `incoming`-lifecycle notifications, gated on
settings.osNativeNotifications and dependent on T167 streaming
for proper "newly-arrived" semantics.

U-N1 (SettingsView initialSection ignored, LOW): SettingsView is
now `ConsumerStatefulWidget` with per-section `GlobalKey`s
(`_displayKey` / `_notificationsKey` / `_connectionKey` /
`_privacyKey`). `initState` schedules a post-frame callback that
calls `Scrollable.ensureVisible` on the matching section so
navigating to `/settings/diagnostics` lands the operator on the
diagnostics block rather than the top of the page.
`_SectionHeader` accepts an optional `key` so the GlobalKeys
attach to the right widgets.

I-N2 (version_display docstring lag, LOW): docstring updated from
"Compact Dashboard badge" to "Compact AppBar badge — Rendered
globally on the AppShell AppBar per Round-3 analyze C2 placement".
Reflects actual wiring.

S-N1 (TaskList housekeeping, LOW): marked stale entries #12
(Batch 4 Phase 5 HIGHs), #14 (Batch 6 test rewrites), #15 (Batch
7+8 polish) as completed since their work was folded into other
commits (Phase 9 batch 1, post-Phase-8 T168-T170 tracking,
Round 2/3 analyze remediations). No spec file edits — TaskList
agent-side only.

tasks.md footer updated: 171 total tasks (was 170), 29 Phase 9
tasks (was 28), 120 parallel markers (was 119). New
"Post-Round-4-analyze follow-up" subsection documents T171 with
the dependency note on T167 streaming.

Verification note: flutter-bench container's `/workspace/projects`
bind mount remains unrestored after the earlier recycle, and the
dev-benches compose file has no volume binds (the workspace mount
came from a VS Code devcontainer layer). All 4 code changes are
surgical (await + provider override, ConsumerStatefulWidget
conversion, docstring text) following established patterns;
regression risk is low. Bench baseline is still 881f70c
(Round 3 + Round 4 changes both unbench-verified).

Closes analyze findings I-N1, C-N1, U-N1, I-N2, S-N1 (5 of 5).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Jun 1, 2026
Deep-review findings #5, #15, #19.

#5 (FR-010) — SubprocessTmuxAdapter.kill_pane now treats a vanished pane
("can't find pane" / "no such pane") as idempotent SUCCESS (returns
instead of raising). With remain-on-exit off, a pane is destroyed when
its process exits, so the normal teardown path hit kill-pane on a missing
pane and surfaced docker_exec_failed with tmux_kill_succeeded=False —
breaking the documented idempotent-remove contract. Real docker failures
still raise.

#15 — FakeTmuxAdapter.kill_pane models the same idempotent path: a
pane in dead_pane_ids is killed with success, not a TmuxError. Adds an
end-to-end test (vanished pane → ok=True through the production kill
backend) so the contract gap can't regress unseen again.

#19 — tmux/adapter.py now imports Mapping (used in two Protocol method
signatures) — was a latent NameError if annotations were ever evaluated
(get_type_hints / future-import removal).

Verification: +4 tests; adapter/kill/remove/story3 suites 63 pass.

Refs review #5 #15 #19.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants