refactor(complexity): decompose launcher doctor probes (S3776)

[farnalabs]

SonarCloud S3776: default_probes had cognitive complexity 91. Extracted _build_composed_config, _build_database_probes and _build_system_probes; orchestrator now wires probes at roughly 12. Behaviour and signatures preserved. ruff + mypy clean; CI backend suite validates.

[farnalabs]

Automated fix: migration 0240 duplicate-column on fresh DB

Root cause. CI failed in the BDD (full suite) Run migrations step with:

psycopg.errors.DuplicateColumn: column "updated_at" of relation "organisations" already exists

Migration 0240_reinstate_organisations_audit_columns unconditionally re-added the updated_at/updated_by/deleted_by columns and the fk_organisations_created_by FK. On a fresh DB those columns already exist: 0233 added them and 0239 is a no-op (it retains them), so alembic upgrade head hit a duplicate-column error and aborted the whole chain — blocking every PR that runs the migration suite.

Fix. Made 0240 idempotent by guarding each add_column/create_foreign_key with an inspect()-based existence check (same idiom as migration 0209). On a fresh DB the columns are present so the adds are skipped; on a prod DB where the old dropping 0239 actually removed them, they are added. No revision id changed, so no references need sweeping.

Commit: 274d0d141a7bbc8f3650688bbf95946adb37365a

Verified via pre-commit hooks (ruff, ruff-format, bandit, semgrep, gitleaks, import-linter) — all passed.

[farnalabs]

Feedback (not the formal decision): This diff is a mechanical extraction of default_probes into helpers (_build_composed_config, _build_database_probes, _build_system_probes) and reads as behavior-preserving — good overall. Minor cleanups suggested, none blocking: (1) _build_system_probes defines an unused local probe_degraded_reason that is then redefined verbatim in default_probes — delete the dead copy; (2) its docstring promises an 11-entry tuple ending with degraded_reason but returns only 10 entries; (3) the ' = env_snapshot # ensure unused-ref lint does not trigger' hack plus a second independent env_snapshot = dict(os.environ) capture in default_probes is confusing — either pass env_value as a returned probe or drop the unused capture; (4) the composed parameter of _build_system_probes is unused.

[modulo-reviewbot]

Approved by PR Reviewer post-decision node (PR #570, SHA 8a6d8a6).

Verdict: APPROVE (low-risk; policy-router: no high-risk-path match — doctor.py matches no high-risk glob, registry untouched, no test deletions/skip-xfail additions).

Review node findings: single-file behavior-preserving refactor extracting default_probes into _build_composed_config/_build_database_probes/_build_system_probes in backend/src/modulo/launcher/doctor.py. Probe logic unchanged; no security, correctness, or read-only-invariant regressions; no API/frontend surface touched.

Non-blocking minor cleanups for a follow-up:

1. _build_system_probes defines a dead local _probe_degraded_reason that is never returned and is redefined verbatim in default_probes (~line 1249) — delete it.
2. Its docstring lists 11 tuple entries ending with degraded_reason but returns only 10.
3. The composed parameter of _build_system_probes is unused.
4. The "_ = env_snapshot" lint-suppression plus a second independent dict(os.environ) capture in default_probes is confusing — capture once or return the env_value probe from the helper.

[farnalabs]

Automated merge conflict resolution

This branch had merge conflicts with origin/main (phase=merge-conflict). I merged origin/main into dist/sonar-cx-doctor and resolved the single conflict in backend/src/modulo/db/migrations/versions/0240_reinstate_organisations_audit_columns.py.

Conflict & resolution: Both sides made migration 0240 idempotent (to avoid DuplicateColumn on a fresh DB). The PR branch used sqlalchemy.inspect to enumerate existing columns/FKs and dropped them unconditionally in downgrade(); origin/main used the existing _column_exists/_fk_exists information_schema helpers and guarded both upgrade() and downgrade(). I kept origin/main's version because its downgrade() is also existence-guarded (the PR-branch downgrade would fail on a live DB where 0239 physically dropped the columns), and it reuses the helper functions already defined in the file. The down_revision (0239) is unchanged, so the migration chain stays intact.

Verification: alembic heads reports exactly one head (0240_reinstate_organisations_audit_columns); all pre-commit hooks passed.

Commit: 1e75efea16b368d340ce0fc038a6f8ba987979c3

[farnalabs]

Automated merge-conflict fix (Branch Fixer)

The merge queue reported a conflict squashing PR #570 into the queue branch (run reference: merge-conflict phase). Root cause: #570 carried a stray edit to migration 0240_reinstate_organisations_audit_columns.py — it added an unused _FK_NAME constant and dropped the docstring metadata block. That migration file is concurrently edited by the in-flight deploy-fix PRs (#509 / #534 / #559), so the 3-way squash-merge of #570 collided on it.

Fix

Commit 49ba0fe177f762d4d332119e91fbad2a799225d8 reverts 0240_reinstate_organisations_audit_columns.py to main's version.

• The actual change in refactor(complexity): decompose launcher doctor probes (S3776) #570 — the S3776 doctor-probe refactor in backend/src/modulo/launcher/doctor.py (300 lines) — is unique to this PR (no other queued PR touches that file), so it is preserved untouched.
• The _FK_NAME constant was dead code (never referenced anywhere in the branch), so dropping it loses no behaviour.

Verification

A git merge-tree simulation of squash-merging the fixed #570 onto a predecessor state that has already modified 0240 returns a clean tree with no conflict markers — #570 now only contributes doctor.py, which merges cleanly regardless of queue ordering. All pre-commit hooks (ruff, ruff-format, bandit, semgrep, gitleaks, check-migration-heads) passed on the commit.

Once this push re-triggers the queue, #570 should merge cleanly.

[farnalabs]

Feedback from complexity-decomposition review (refactor of default_probes for Sonar S3776). Behavior equivalence verified: ran tests/unit/launcher/test_doctor_defaults.py + test_doctor_helpers.py (52 passed), mypy and ruff clean on the file. Approving from the review stage; minor cleanups suggested, non-blocking: (1) _build_system_probes' docstring lists degraded_reason in the tuple order but the function neither returns it nor needs it — the docstring should drop it; (2) dead duplicated _probe_degraded_reason inside _build_system_probes (doctor.py:1127) shadows nothing and is unused — the wiring one is in default_probes; (3) composed and state params of _build_system_probes are unused; (4) _ = env_snapshot with the misleading comment is dead, and default_probes re-creates its own env_snapshot for __probe_env_value — either document the split clearly or snapshot inside default_probes only.

[modulo-reviewbot]

Approve: PR cleanly decomposes the oversized default_probes in backend/src/modulo/launcher/doctor.py into _build_composed_config, _build_database_probes and _build_system_probes, preserving behavior (probe_redis port/url handling equivalence checked). Verified tests/unit/launcher/test_doctor_defaults.py and test_doctor_helpers.py all pass (52 unit tests); mypy and ruff are clean on the changed file. CI checks pass except the SonarCloud coverage-import job still pending (proceeded per pending policy). Low-risk: no high-risk-path match in the registry; only doctor.py changed.

Non-blocking cleanups only (do not block merge): (1) _build_system_probes docstring lists degraded_reason in tuple order though it is not returned; (2) dead inner _probe_degraded_reason at doctor.py:1127 duplicated by the one wired in default_probes (~1249); (3) unused composed/state params of _build_system_probes; (4) _ = env_snapshot plus misleading comment is dead code, and env_snapshot is re-created in default_probes for __probe_env_value.

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud