From 8c73148dbed64a8c91a1f2d2c9b1c6e4b3ac90d4 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:25:11 +0000 Subject: [PATCH 01/14] [Spec Kit] Add specification for ruff BLE re-enablement (INBOX-19) Co-Authored-By: Claude Fable 5 --- .specify/feature.json | 2 +- .../checklists/requirements.md | 36 +++++ dev/specs/002-ruff-ble-reenable/spec.md | 128 ++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 dev/specs/002-ruff-ble-reenable/checklists/requirements.md create mode 100644 dev/specs/002-ruff-ble-reenable/spec.md diff --git a/.specify/feature.json b/.specify/feature.json index 1be33c1f1c0..e9417158612 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/001-entities-arch-migration" + "feature_directory": "specs/002-ruff-ble-reenable" } diff --git a/dev/specs/002-ruff-ble-reenable/checklists/requirements.md b/dev/specs/002-ruff-ble-reenable/checklists/requirements.md new file mode 100644 index 00000000000..aaf0a1665b9 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/checklists/requirements.md @@ -0,0 +1,36 @@ +# Specification Quality Checklist: Re-enable ruff BLE (blind-except) rule and fix all violations + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-07-22 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- **Domain-inherent tooling references**: This feature's subject *is* the lint toolchain (ruff rule BLE001, `pyproject.toml` ignore list, `# noqa` suppressions). References to those artifacts in requirements and success criteria are the domain vocabulary of the feature, not implementation leakage; they are the only precise way to express testable acceptance. No *incidental* technology choices (libraries, code structure, algorithms) appear. +- **No [NEEDS CLARIFICATION] markers**: three potentially ambiguous points (stale ~32 vs. measured 78 count; whether lint-only edits inside migration/auth files conflict with the "no migration/auth changes" hard constraints; local vs. CI test obligations) were resolved with documented reasoning in the Assumptions section, as the orchestrating workflow requires autonomous decisions. The migration/auth resolution is conservative: suppression-only, zero semantic change in those areas. +- All checklist items pass as of 2026-07-22; spec is ready for `/speckit-plan`. diff --git a/dev/specs/002-ruff-ble-reenable/spec.md b/dev/specs/002-ruff-ble-reenable/spec.md new file mode 100644 index 00000000000..1df5e37a575 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/spec.md @@ -0,0 +1,128 @@ +# Feature Specification: Re-enable ruff BLE (blind-except) rule and fix all violations + +**Feature Branch**: `pha/INBOX-19` + +**Created**: 2026-07-22 + +**Status**: Draft + +**Input**: User description: "Re-enable the ruff BLE (flake8-blind-except) rule and fix all of its violations in opsmill/infrahub (Engineering Inbox card INBOX-19). Remove BLE from the global ruff ignore list in pyproject.toml; at each violation site replace the blind except with the specific exception type(s) the guarded code can actually raise; where a broad catch is genuinely required keep `except Exception` with a targeted `# noqa: BLE001` and a brief justification comment." + +## Context + +The repository's lint configuration selects all ruff rules (`select = ["ALL"]`) and then globally ignores the BLE category (flake8-blind-except) under the "needs to be investigated" block in `pyproject.toml`. The team's suppression analysis (Patrick Ogenstad's 2026-02-18 Slack thread on lint ignores) ranked re-enabling BLE as priority #1: blind `except Exception:` handlers swallow real bugs, and handlers that catch `BaseException` (or bare `except:`) additionally swallow `KeyboardInterrupt`/`SystemExit`. + +Ground truth measured on this branch (2026-07-22): **78 BLE001 violations across 46 files** — the card's ~32 estimate is stale; the count grew with recently added graph migrations. Distribution: + +| Area | Sites | Notes | +|------|-------|-------| +| `backend/infrahub/core/migrations/` (graph migrations + shared) | 30 | Best-effort per-item loops in data backfills; behavior must not change (hard constraint) | +| Authentication paths (`api/auth.py`, `api/oauth2.py`, `api/oidc.py`, `auth/auth.py`) | 8 | Auth behavior must not change (hard constraint) | +| Other backend runtime (`artifacts`, `cli/upgrade`, `core/schema`, `core/validators`, `generators`, `git`, `message_bus`, `services`, `task_manager`, `telemetry`, `webhook`) | 16 | Mix of defensive task loops and narrowable handlers | +| Backend test helpers/suites (`backend/tests/`) | 13 | Includes one `except BaseException` (`tests/helpers/test_worker.py`) | +| Repo tooling (`tasks/release.py`, `tests/e2e/data/parity.py`, `utilities/infrahub_load_tester.py`) | 11 | Dev/CI tooling and load-test scripts | + +The enforcing gate is CI's `uv run ruff check . --exclude python_sdk` (full-repo run), so every one of the 78 sites must be resolved before the ignore entry can be removed. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Blind-except enforcement is active for all future code (Priority: P1) + +As an Infrahub developer, when I introduce a new `except Exception:` (or broader) handler without justification anywhere in the repository, the lint gate rejects my change locally and in CI, so bug-swallowing handlers can no longer enter the codebase unnoticed. + +**Why this priority**: The durable value of the card is regression prevention. Fixing today's 78 sites without turning the rule on would let the debt immediately re-accumulate. + +**Independent Test**: Remove `"BLE"` from the ignore list, add a temporary `except Exception: pass` to any linted file, run the lint gate — it must fail with BLE001; revert the temporary handler — it must pass. + +**Acceptance Scenarios**: + +1. **Given** the BLE entry is removed from the global ignore list in `pyproject.toml`, **When** `uv run ruff check . --exclude python_sdk` runs (the CI lint command), **Then** it reports zero violations. +2. **Given** the rule is active, **When** a developer adds an unjustified `except Exception:` handler and runs the lint gate, **Then** the gate fails with a BLE001 diagnostic pointing at the new handler. +3. **Given** the rule is active, **When** `uv run invoke backend.lint` runs, **Then** it exits successfully. + +--- + +### User Story 2 - Existing blind handlers are narrowed to real failure modes (Priority: P2) + +As an Infrahub developer or operator, code paths that previously swallowed *any* error now catch only the exception types the guarded code can actually raise, so genuinely unexpected failures (typos, contract violations, programming bugs) surface immediately instead of being silently absorbed or mislabeled. + +**Why this priority**: This is the direct bug-risk reduction the card was filed for. It ranks below P1 only because narrowing without enforcement decays, while enforcement without narrowing is impossible (CI would fail). + +**Independent Test**: For each narrowed site, the module's existing tests pass unchanged; `ruff check --select=BLE ` is clean for that file. + +**Acceptance Scenarios**: + +1. **Given** a handler whose guarded code has an identifiable set of raisable exception types, **When** the fix is applied, **Then** the handler names those specific types (or a project/library base type that covers them) and its body is unchanged. +2. **Given** a narrowed handler, **When** the exceptions it previously handled are raised by the guarded code, **Then** runtime behavior is identical to before the change (same logging, same fallback, same control flow). +3. **Given** the full set of narrowed sites, **When** the test suites covering the touched modules run, **Then** they pass without modification (except tests that themselves contained violations). + +--- + +### User Story 3 - Genuinely-broad catches are explicit and justified (Priority: P3) + +As a future maintainer reading defensive code (top-level task loops, best-effort cleanup, data-migration per-item guards), I can immediately see that a broad catch is intentional: it reads `except Exception` (never bare `except:`), carries a targeted `# noqa: BLE001` suppression, and a brief comment stating why swallowing arbitrary errors is required there. + +**Why this priority**: Documentation/readability value on top of P1/P2; it is what makes the suppression auditable rather than silent. + +**Independent Test**: Grep all `noqa: BLE001` occurrences; each must sit on an `except Exception` (or deliberately `except BaseException` where isolation demands it) with an adjacent justification, and none may be a bare `except:`. + +**Acceptance Scenarios**: + +1. **Given** a site where any failure must not break the surrounding loop/cleanup (e.g., per-node migration backfill, telemetry push, scheduled-task loop), **When** the fix is applied, **Then** the handler keeps `except Exception`, gains `# noqa: BLE001`, and a short justification comment, with zero behavioral change. +2. **Given** the completed change, **When** the repository is searched for bare `except:` clauses in linted Python code, **Then** none exist (E722 already enforces this; the change must not introduce any). +3. **Given** the completed change, **When** any handler still catches `BaseException`, **Then** it carries an explicit justification for also intercepting `KeyboardInterrupt`/`SystemExit` (only defensible in process-isolation/diagnostic harnesses). + +--- + +### Edge Cases + +- **Handlers inside hard-constraint areas (graph migrations, auth flows)**: narrowing would change runtime behavior for unexpected exception types in code where behavior changes are prohibited by the card (no migration changes, no auth changes). These sites MUST use the suppress-with-justification treatment (comment + `noqa`), never semantic narrowing. +- **`except BaseException` in `backend/tests/helpers/test_worker.py`**: broader than `Exception`; must be either narrowed or explicitly justified — never silently converted in a way that changes what the test harness intercepts. +- **Handlers that both log and re-raise or wrap**: ruff still flags them; the fix must preserve the wrap/re-raise semantics exactly. +- **Fixture/vendored Python files** (e.g., `backend/tests/fixtures/repos/...` with their own `pyproject.toml`): governed by their own ruff scope; out of remediation scope — the CI command's output is authoritative for what is in scope. +- **`python_sdk/` submodule**: explicitly excluded by the CI lint command and a separate repository; out of scope. +- **New violations landing on the base branch while this change is in flight**: the final verification must re-run the full-repo check at merge-readiness time, not rely on the initial inventory of 78. +- **Unused-suppression detection (RUF100)**: every added `# noqa: BLE001` must be *load-bearing* once BLE is active; a `noqa` added to a line ruff does not flag would itself fail the lint gate. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The global ruff ignore list in `pyproject.toml` MUST no longer contain the `"BLE"` entry, and no new blanket suppression of BLE (global ignore, per-file-ignore section, or directory-wide ignore) may be introduced in its place. +- **FR-002**: After the change, `uv run ruff check --select=BLE .` and the CI lint command `uv run ruff check . --exclude python_sdk` MUST both report zero violations from the repository root. +- **FR-003**: Every current violation site MUST be resolved by exactly one of two treatments: (a) narrowing the handler to the specific exception type(s) the guarded block can raise, or (b) keeping `except Exception` with a line-targeted `# noqa: BLE001` and a brief justification comment. Treatment (b) is mandatory — not optional — for sites inside graph migrations and authentication flows (hard-constraint areas). +- **FR-004**: No handler may be resolved by widening (e.g., converting to bare `except:` or to `except BaseException:`); the single existing `BaseException` handler MUST end up either narrowed or explicitly justified for intercepting interpreter-exit signals. +- **FR-005**: Narrowed handlers MUST preserve the existing handler body and control flow; the only intended behavioral difference is that exception types the guarded code cannot legitimately raise now propagate instead of being swallowed. +- **FR-006**: Sites in hard-constraint areas (anything under `backend/infrahub/core/migrations/`, and the authentication paths `backend/infrahub/api/auth.py`, `backend/infrahub/api/oauth2.py`, `backend/infrahub/api/oidc.py`, `backend/infrahub/auth/`) MUST have identical runtime semantics after the change — only comments and suppression markers may be added there. +- **FR-007**: The full lint gate MUST pass after the change: `uv run invoke backend.lint` (ruff + ty + mypy over `backend/`) and the repo-wide `uv run ruff check . --exclude python_sdk`, including format checks. +- **FR-008**: Existing tests covering touched modules MUST pass without behavioral test changes; test files that themselves contained violations may only change in their exception-handling annotations, not in what they assert. +- **FR-009**: The change MUST NOT touch: database schema or migration semantics, GraphQL/REST API contracts, authentication/authorization behavior, dependency sets, CI workflow definitions, or generated files. (Editing exception-handler *annotations* inside existing migration/auth files is permitted only under FR-006's identical-semantics rule.) +- **FR-010**: Every added `# noqa: BLE001` MUST be effective (suppress an actual diagnostic) so the codebase stays clean under unused-suppression checking, and MUST be accompanied by a justification comment on or adjacent to the handler. + +### Key Entities + +- **Violation site**: one flagged `except` clause — identified by file, line, and caught type (`Exception` ×77, `BaseException` ×1); classified into a remediation category (narrow vs. suppress-with-justification) and a risk zone (migration, auth, runtime, test, tooling). +- **Ignore-list entry**: the `"BLE"` string in the `[tool.ruff.lint]` `ignore` array of the root `pyproject.toml` — the single configuration change that activates enforcement. +- **Suppression marker**: a line-targeted `# noqa: BLE001` plus adjacent justification comment — the auditable unit for intentional broad catches. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: `uv run ruff check --select=BLE .` from the repository root reports 0 violations (baseline: 78). +- **SC-002**: The CI lint command `uv run ruff check . --exclude python_sdk` and `uv run invoke backend.lint` both exit 0 on the final state of the branch. +- **SC-003**: 100% of intentional broad catches are auditable: the count of `# noqa: BLE001` markers equals the count of justification comments attached to them, and a reviewer can enumerate them with a single search. +- **SC-004**: Zero bare `except:` clauses and zero unjustified `except BaseException:` clauses exist in linted Python code. +- **SC-005**: Test suites covering every touched module pass with unchanged assertions (locally runnable tiers: unit and component; heavier tiers deferred to CI). +- **SC-006**: Introducing a new unjustified blind except into any linted file makes the lint gate fail (verified once by mutation before finishing). +- **SC-007**: Runtime behavior in hard-constraint areas is unchanged: the diff for migration and auth files contains only comment/suppression additions (verifiable by diff inspection). + +## Assumptions + +- The card's "~32 sites" was accurate at analysis time (2026-02-18) but is superseded by the measured inventory of 78 sites / 46 files on this branch; the scope is *all current violations*, whatever the final count at verification time. +- "No DB schema or migration changes" (hard constraint) is interpreted as *no changes to what migrations do* — adding a comment and a `noqa` marker to an existing migration file does not constitute a migration change; renumbering, semantic edits, or new migrations would. The same interpretation applies to "no auth changes". +- The suppress-with-justification treatment is the *default* for defensive top-level loops (scheduled tasks, telemetry, webhook dispatch, artifact/generator task wrappers) because those handlers exist precisely to keep the worker loop alive against arbitrary failures; narrowing is reserved for handlers guarding a small, analyzable expression. +- "Tests related to touched modules pass" means the locally runnable test tiers (backend unit tests; component tests where practical) for the modules whose files changed; full integration/e2e tiers run in CI as usual and are not a local exit criterion. +- No changelog fragment is required: the change is developer-facing housekeeping with no user-visible behavior change (consistent with towncrier fragments being for user-facing changes). If reviewers disagree, a `housekeeping` fragment can be added at review time. +- `ruff rule BLE001` (the house method's "understand the rule first" step) confirms the rule flags `except Exception` and `except BaseException` handlers; ruff never flags narrower catches, so narrowing always satisfies the rule. +- The `python_testcontainers/` directory and test-fixture repos carry their own ruff configuration scopes; the authoritative in-scope file set is exactly what the CI command reports. From 53d513bba7c9998f1b8fead4096eb9557d606c5f Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:36:47 +0000 Subject: [PATCH 02/14] [Spec Kit] Add implementation plan for ruff BLE re-enablement (INBOX-19) 78-site inventory with per-site treatment matrix (8 narrow / 70 suppress), research decisions, and validation quickstart. Co-Authored-By: Claude Fable 5 --- dev/specs/002-ruff-ble-reenable/data-model.md | 159 ++++++++++++++++++ dev/specs/002-ruff-ble-reenable/plan.md | 120 +++++++++++++ dev/specs/002-ruff-ble-reenable/quickstart.md | 97 +++++++++++ dev/specs/002-ruff-ble-reenable/research.md | 79 +++++++++ 4 files changed, 455 insertions(+) create mode 100644 dev/specs/002-ruff-ble-reenable/data-model.md create mode 100644 dev/specs/002-ruff-ble-reenable/plan.md create mode 100644 dev/specs/002-ruff-ble-reenable/quickstart.md create mode 100644 dev/specs/002-ruff-ble-reenable/research.md diff --git a/dev/specs/002-ruff-ble-reenable/data-model.md b/dev/specs/002-ruff-ble-reenable/data-model.md new file mode 100644 index 00000000000..5804028cc66 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/data-model.md @@ -0,0 +1,159 @@ +# Data Model: BLE001 violation-site inventory and treatment matrix + +**Plan**: [plan.md](plan.md) | **Research**: [research.md](research.md) + +The "entities" of this feature are the 78 violation sites. Each row below is authoritative for the implementation phase. Analysis performed 2026-07-22 by four parallel read-only reviews of every site (surrounding code, raisable-exception surface, handler behavior); line numbers verified against `ruff check --select BLE001` output on this branch. + +## Treatment totals + +| Treatment | Count | Where | +|-----------|-------|-------| +| NARROW (specific exception types) | 8 | 6 in backend tests, 2 in `tasks/release.py` | +| SUPPRESS (`# noqa: BLE001` + justification) | 70 | 30 migrations, 8 auth, 16 backend runtime, 7 backend tests, 9 tooling | +| **Total** | **78** | 46 files | + +## Normalization rules (apply to every edit) + +1. **Suppression form**: bare rule-targeted `# noqa: BLE001` appended to the `except` line. The justification comment goes on its own line immediately **above** the `except` line (not as prose trailing the noqa — keeps ruff's noqa parsing unambiguous and lines under length limits). Where an accurate explanatory comment already exists adjacent to the handler (e.g. `git/sync.py:121-122`, `telemetry/tasks.py:125`, `test_merge_kill_recovery.py:86-88`, `parity.py` trailing comment), keep it — add the `noqa` and only add a new comment if the existing one doesn't state *why broad*. +2. **Narrowing form**: replace the caught type; never touch the handler body; add the exception import following the file's existing import placement convention (module-level, except `tasks/release.py` where `packaging.version` imports are deliberately *function-local* so invoke works without dev deps — extend those local imports in place). +3. **Behavior invariants**: SUPPRESS edits are annotation-only (comments + noqa; zero semantic tokens). NARROW edits change only the exception type expression. +4. **Stale-suppression cleanup**: where narrowing makes an existing `# noqa: S110` unused (typed excepts are exempt from S110 by default), remove it in the same edit — RUF100 (enabled via `select=ALL`) fails on unused noqa. +5. Migration and auth files (Batches A/B): SUPPRESS only — mandated by hard constraints, regardless of narrowability. + +## Batch A — Graph migrations (30 sites, all SUPPRESS, annotation-only) + +Handler pattern is uniform: convert any failure into `MigrationResult.errors` so the runner (`backend/infrahub/cli/db.py`) reports it and halts without an unhandled traceback (verified: `MigrationResult.success = not errors`; runner logs errors, marks FAILED, does not bump graph version). + +| File:Line | Handler behavior | Justification comment | +|-----------|------------------|----------------------| +| core/migrations/graph/m014_remove_index_attr_value.py:39 | Index-drop failure → result.errors, failed result | `# Migration contract: failures become MigrationResult errors; the runner reports them and halts` | +| core/migrations/graph/m029_duplicates_cleanup.py:656 | Whole cleanup wrapped → result.errors | same as m014 | +| core/migrations/graph/m036_drop_attr_value_index.py:39 | Index-drop failure → result.errors | same as m014 | +| core/migrations/graph/m043_create_hfid_display_label_in_db.py:116 | First failing sub-migration → record, return early | `# First failing sub-migration is recorded as a result error and aborts the remaining steps` | +| core/migrations/graph/m043_create_hfid_display_label_in_db.py:168 | Same, non-default branches | same as m043:116 | +| core/migrations/graph/m044_backfill_hfid_display_label_in_db.py:382 | Whole default-branch backfill → result.errors | same as m014 | +| core/migrations/graph/m044_backfill_hfid_display_label_in_db.py:514 | Whole per-branch backfill → result.errors | same as m014 | +| core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py:82 | Whole backfill → result.errors | same as m014 | +| core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py:163 | Per-branch backfill → result.errors | same as m014 | +| core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py:141 | Whole `_do_execute` → result.errors | same as m014 | +| core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py:196 | First failing sub-migration → record, return | same as m043:116 | +| core/migrations/graph/m047_backfill_or_null_display_label.py:416 | Default-branch pass → result.errors | same as m014 | +| core/migrations/graph/m047_backfill_or_null_display_label.py:465 | Per-branch pass → result.errors | same as m014 | +| core/migrations/graph/m059_fix_hfid_display_label_nulls.py:238 | Per-node recompute: log, record, skip node, continue | `# Best-effort per-node recompute: record the failure, skip this node, keep fixing the rest` | +| core/migrations/graph/m059_fix_hfid_display_label_nulls.py:247 | Per-node HFID recompute: same | same as m059:238 | +| core/migrations/graph/m059_fix_hfid_display_label_nulls.py:381 | Default+global pass → result.errors | same as m014 | +| core/migrations/graph/m059_fix_hfid_display_label_nulls.py:420 | Per-branch pass → result.errors | same as m014 | +| core/migrations/graph/m062_recompute_permission_display_labels.py:454 | Recompute (default) → result.errors | same as m014 | +| core/migrations/graph/m062_recompute_permission_display_labels.py:473 | Recompute (per branch) → result.errors | same as m014 | +| core/migrations/graph/m063_template_number_pool_cleanup.py:82 | Nullification loop → result.errors | same as m014 | +| core/migrations/graph/m064_template_ip_pool_relationship_cleanup.py:98 | Relationship cleanup → result.errors | same as m014 | +| core/migrations/graph/m066_consolidate_duplicate_number_pools.py:82 | Consolidation (inside open txn) → result.errors | `# Failures become MigrationResult errors so the runner reports them instead of crashing` | +| core/migrations/graph/m070_normalize_mac_address_values_to_colon.py:225 | Per-plan recompute loop → result.errors | same as m014 | +| core/migrations/graph/m071_recompute_hfid_for_ip_attributes.py:180 | Per-kind recompute loop → result.errors | same as m014 | +| core/migrations/graph/m072_index_hfid_values.py:169 | Normalize + index steps → result.errors | same as m014 | +| core/migrations/graph/m073_unify_ip_pool_resource_identifier.py:336 | Pool unification (inside open txn) → result.errors | same as m066 | +| core/migrations/graph/m074_normalize_indexed_hfid_values.py:156 | Normalization → result.errors | same as m014 | +| core/migrations/shared.py:157 | Per-query loop (SchemaMigration): record, abort remaining | `# Per-query failures become result errors so the runner reports them instead of crashing` | +| core/migrations/shared.py:245 | Per-query loop (GraphMigration): record, return early | same as shared:157 | +| core/migrations/shared.py:277 | Per-sub-migration loop: record, abort remaining | `# First failing sub-migration is recorded as a result error and aborts the remaining steps` | + +**Comment-truthfulness caveat** (from verification): m066:82, m073:336, shared.py:157, shared.py:245 catch *inside* an open transaction, so a caught failure commits partial work (rollback never fires). The chosen comments deliberately do **not** claim atomicity. See "Latent defects" below. + +## Batch B — Authentication paths (8 sites, all SUPPRESS, annotation-only) + +| File:Line | Handler behavior | Justification comment | +|-----------|------------------|----------------------| +| api/auth.py:63 | Login-event emission failure: warn, login still succeeds | `# Login event emission is best-effort telemetry; it must never fail a successful login` | +| api/auth.py:116 | Logout-event emission failure: warn, logout completes | `# Logout event emission is best-effort telemetry; it must never fail a successful logout` | +| api/oauth2.py:205 | OAuth2 login-event emission failure: warn, token returned | `# Login event emission is best-effort telemetry; it must never fail a successful OAuth2 login` | +| api/oidc.py:259 | OIDC login-event emission failure: warn, token returned | `# Login event emission is best-effort telemetry; it must never fail a successful OIDC login` | +| auth/auth.py:542 | Any token decode/claim failure → `AuthorizationError` (401) | `# Fail closed: any undecodable or malformed token must map to a 401 auth error, never a 500` | +| auth/auth.py:558 | Any refresh-token decode failure → `AuthorizationError` (401) | `# Fail closed: any undecodable or malformed refresh token must map to a 401, never a 500` | +| auth/auth.py:668 | Provider body not JSON → fall back to text / `GatewayError` (502) | `# Providers may return non-JSON bodies: fall back to text or fail closed with GatewayError (502)` | +| auth/auth.py:679 | Body unreadable → log + `GatewayError` (502) chained | `# If the body cannot be read at all, fail closed with GatewayError (502) rather than a 500` | + +Note: the four `auth/auth.py` handlers raise *new* exceptions (not the caught one), which BLE001 still flags — only re-raising the caught exception is exempt. Suppression preserves the fail-closed contract exactly. + +## Batch C — Backend runtime (16 sites, all SUPPRESS) + +All 16 are defensive boundaries; none has an enumerable raisable set. Every handler already logs or persists the failure (or returns it for rollback + re-raise), satisfying the house guideline. + +| File:Line | Boundary type | Justification comment | +|-----------|--------------|----------------------| +| artifacts/tasks.py:49 | Check boundary | `# noqa rationale: check boundary — any render failure must be recorded as a failed artifact check, not crash the flow` → comment: `# Check boundary: any render failure must be recorded as a failed artifact check, not crash the flow` | +| cli/upgrade.py:65 | CLI prerequisite | `# CLI prerequisite boundary: report any failure as an unreachable database and abort cleanly` | +| cli/upgrade.py:244 | Best-effort dry-run probe | `# Best-effort dry-run report: a failed schema probe is reported inline and the remaining checks still run` | +| core/schema/update_coordinator.py:350 | Capture-for-rollback | `# Any migration failure must be captured so the caller can roll back before re-raising it` | +| core/schema/update_coordinator.py:365 | Capture-for-rollback | same as :350 | +| core/validators/tasks.py:85 | Degrade-to-violation | `# Degrade any checker failure into a reported violation so schema validation fails visibly instead of crashing the task` | +| generators/tasks.py:253 | Flow boundary | `# Flow boundary: any generator failure must surface as a Failed state carrying the error, not a crashed flow run` | +| git/integrator.py:383 | Stamp-status-then-reraise | `# Any import failure must stamp the repository sync status as errored before being re-raised` | +| git/sync.py:120 | Per-branch isolation | keep existing comment (lines 121-122); add noqa only | +| message_bus/operations/__init__.py:34 | Consumer boundary | `# Message-bus boundary: any handler failure must be routed to the reply/retry/dead-letter protocol, never crash the consumer` | +| services/scheduler.py:89 | Keep-alive loop | `# Keep-alive: a failing recurring task must not kill the scheduler loop` | +| task_manager/flow_run/retention.py:63 | Best-effort per-item purge | `# Best-effort retention: skip flow runs that fail to purge and keep processing the batch` | +| telemetry/tasks.py:129 | Best-effort telemetry | existing comment at line ~125 documents the bail-out; add noqa; extend comment only if it doesn't say why broad | +| telemetry/tasks.py:152 | Best-effort telemetry | `# Best-effort telemetry: any send failure is recorded as FAILED on the snapshot, never propagated` | +| telemetry/tasks.py:159 | Best-effort telemetry | `# Best-effort telemetry: failing to persist the send status only warrants a warning` | +| webhook/tasks/process.py:90 | Best-effort capture | `# Best-effort capture: an artifact write failure must never alter or mask the delivery outcome` | + +(For artifacts/tasks.py:49 use the single comment line shown after the arrow.) + +## Batch D — Backend tests (13 sites: 6 NARROW, 7 SUPPRESS) + +| File:Line | Treatment | Detail | +|-----------|-----------|--------| +| tests/component/core/schema/schema_branch/test_process_idempotency.py:158 | NARROW | `except SchemaNotFoundError:` — add `from infrahub.exceptions import SchemaNotFoundError`; helper formats "only in after" diff lines; `get(name=...)` raises exactly this when absent | +| tests/component/core/schema/schema_branch/test_process_idempotency.py:164 | NARROW | same (mirror "only in before" case) | +| tests/component/core/schema/schema_branch/test_uniqueness_propagation.py:42 | NARROW | verbatim copy of the same helper — identical narrowing + import | +| tests/component/core/schema/schema_branch/test_uniqueness_propagation.py:48 | NARROW | same | +| tests/helpers/diagnostics.py:103 | SUPPRESS | `# Best-effort post-mortem dump: must never raise while reporting the original error` | +| tests/helpers/diagnostics.py:179 | SUPPRESS | `# Instrumentation must never break the real pool disconnect; log and continue` | +| tests/helpers/events.py:51 | SUPPRESS | `# Polling probe: query_event signals absence with a bare Exception; any failure means "not available yet"` (cannot narrow: `query_event` raises bare `Exception` by design — rewriting it is a behavior change) | +| tests/helpers/test_worker.py:107 | SUPPRESS — **stays `except BaseException`** | `# Any failure (incl. CancelledError) must resolve the "ready" future, else the fixture awaiting it hangs forever` — converting to `Exception` would let `CancelledError`/`SystemExit` escape and deadlock the fixture at `await ready` | +| tests/integration/git/conftest.py:31 | NARROW | `except httpx.HTTPError:` (httpx already imported); retry-poll loop; **remove the now-stale `# noqa: S110`** (typed excepts exempt from S110 → RUF100 would fail) | +| tests/integration/git/conftest.py:53 | NARROW | `except httpx.HTTPError:` — same poll pattern + same S110 cleanup. Residual: malformed-201-body errors now propagate loudly instead of retrying to deadline (acceptable: clearer fixture failure; medium confidence — fallback is SUPPRESS) | +| tests/integration_docker/test_merge_kill_recovery.py:85 | SUPPRESS | keep existing explanatory comment (lines 86-88); add noqa: `# Teardown: the deliberately-killed mutation may raise any SDK/transport error; retrieve and log it without masking the test result` | +| tests/scale/common/protocols.py:28 | SUPPRESS | `# Locust instrumentation: record every failure as a request event instead of crashing the user greenlet` | +| tests/scale/common/protocols.py:53 | SUPPRESS | same comment | + +## Batch E — Tooling (11 sites: 2 NARROW, 9 SUPPRESS) + +| File:Line | Treatment | Detail | +|-----------|-----------|--------| +| tasks/release.py:155 | NARROW | `except InvalidVersion:` — extend the *function-local* import at ~line 115 to `from packaging.version import InvalidVersion, Version` (do not hoist; locals are deliberate). Only `Version(...)` construction raises; in-file precedent at lines 273/297 | +| tasks/release.py:242 | NARROW | `except InvalidVersion:` — extend function-local import at ~line 213. Regex-permitted suffixes like `1.2.3-foo` are exactly `InvalidVersion` | +| tests/e2e/data/parity.py:81 | SUPPRESS | `_safe` wrapper returns error string per entry; keep/extend existing trailing comment: `# Diagnostic dump: record any per-entry failure as a string, never kill the whole dump` | +| utilities/infrahub_load_tester.py:47 | SUPPRESS | `# Load test: absorb any request failure and continue` | +| utilities/infrahub_load_tester.py:69 | SUPPRESS | same comment. **Do not fix** the pre-existing missing-`return` (unbound `all_branches`) — behavior preservation; see Latent defects | +| utilities/infrahub_load_tester.py:84 | SUPPRESS | `# Load test: absorb any request failure and continue with remaining branches` | +| utilities/infrahub_load_tester.py:108 | SUPPRESS | `# Load test: absorb any request failure and continue with remaining users` | +| utilities/infrahub_load_tester.py:113 | SUPPRESS | same as :108 | +| utilities/infrahub_load_tester.py:138 | SUPPRESS | same as :84 | +| utilities/infrahub_load_tester.py:148 | SUPPRESS | `# Load test: abort branch cleanup gracefully on any request failure` | +| utilities/infrahub_load_tester.py:165 | SUPPRESS | same as :84 | + +SDK exception types are deliberately not used for narrowing here: `infrahub_sdk` is imported only under `TYPE_CHECKING` in these files and the submodule isn't guaranteed present at analysis time; httpx errors can also leak through. + +## Batch F — Configuration flip + +| File | Change | +|------|--------| +| pyproject.toml (~line 511) | Delete the `"BLE", # flake8-blind-except (BLE)` entry from `[tool.ruff.lint] ignore` | +| changelog/+ruff-ble-blind-except.housekeeping.md | New towncrier fragment (housekeeping type, orphan `+` prefix) | + +## Latent defects observed (explicitly OUT OF SCOPE — follow-up candidates) + +Recorded so the review phase doesn't mistake them for regressions, and so they can become follow-up tickets: + +1. `auth/auth.py:678-679` — the empty-body `GatewayError` raised inside the inner `try` is swallowed by the outer handler and re-raised with a generic message; the specific message never reaches callers. +2. `m066_...py:82` and `m073_...py:336` — catch inside an open `db.start_transaction()` block returns normally → partial work **commits**; rollback never fires. +3. `core/migrations/shared.py:157` and `:245` — same partial-commit-on-failure pattern for every SchemaMigration/GraphMigration using the default `execute`. +4. `utilities/infrahub_load_tester.py:69` — missing `return` after the failure log leaves `all_branches` unbound; the per-branch loop's handler then eats the `NameError`. +5. `backend/tests/helpers/events.py:44,66` — `query_event` raises bare `Exception(...)`; a dedicated exception type would allow narrowing site D-7. + +None of these may be fixed in this pass (behavior preservation / hard constraints). All five would change runtime behavior. + +## Unflagged look-alikes (no action) + +`git/integrator.py` contains additional `except Exception` handlers (e.g. line 308) that BLE001 does **not** flag because their bodies re-raise the caught exception — ruff's built-in exemption. Ruff output is the sole authority for scope; do not "fix" unflagged handlers. diff --git a/dev/specs/002-ruff-ble-reenable/plan.md b/dev/specs/002-ruff-ble-reenable/plan.md new file mode 100644 index 00000000000..2b44e280b09 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/plan.md @@ -0,0 +1,120 @@ +# Implementation Plan: Re-enable ruff BLE (blind-except) rule and fix all violations + +**Branch**: `pha/INBOX-19` | **Date**: 2026-07-22 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `specs/002-ruff-ble-reenable/spec.md` + +## Summary + +Remove `"BLE"` from the global ruff ignore list in the root `pyproject.toml` and resolve all 78 current BLE001 violations (46 files) so the repo-wide CI lint gate (`uv run ruff check . --exclude python_sdk`) passes with the rule active. Each site receives exactly one of two treatments derived from a per-site analysis (see [data-model.md](data-model.md)): **narrow** the handler to the exception types the guarded code can actually raise, or **suppress** with a line-targeted `# noqa: BLE001` plus a truthful justification comment. Hard-constraint areas (graph migrations, authentication flows) are suppression-only — their runtime semantics must not change. No autofix exists for BLE001; every edit is manual and minimal, per the house `/fix-ruff-rule` method. + +## Technical Context + +**Language/Version**: Python 3.14 (backend), per root `pyproject.toml`; repo tooling (`tasks/`, `utilities/`, `tests/e2e/`) runs under the same toolchain + +**Primary Dependencies**: ruff 0.15 (lint), invoke 2.2 (task runner), uv (env). No new dependencies permitted or needed + +**Storage**: N/A — no data or schema changes; graph-migration files are edited annotation-only (comments + suppressions) + +**Testing**: pytest 9.0 — backend unit tests (`uv run invoke backend.test-unit`) and targeted component tests for touched modules; heavier tiers (integration, e2e, scale) validate in CI as usual + +**Target Platform**: Developer workstations + GitHub Actions CI (lint job runs `uv run ruff check . --exclude python_sdk`) + +**Project Type**: Codebase-quality change to an existing monorepo (lint config + point edits across backend, backend tests, tasks, utilities, e2e helpers) + +**Performance Goals**: N/A (no runtime-path changes; narrowed handlers have identical or marginally cheaper dispatch) + +**Constraints**: +- No DB schema or migration semantic changes (migration files: annotation-only edits) +- No GraphQL/REST API contract changes; no auth behavior changes (auth files: annotation-only edits) +- No new dependencies, no CI workflow edits, no manual edits to generated files +- Preserve runtime behavior for all exception types the guarded code can actually raise; only genuinely-unexpected exception types may newly propagate, and only at narrowed non-constraint sites + +**Scale/Scope**: 78 violation sites / 46 files, one config-line removal, zero new modules. Site inventory and per-site treatment matrix in [data-model.md](data-model.md) + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| # | Principle | Gate | Status | +|---|-----------|------|--------| +| I | Schema-Driven Integrity | No schema-layer or generated-file edits. | ✅ PASS — config + handler annotations only; generated dirs contain no violations | +| II | Branch-Safe by Default | No query or branch-logic changes. | ✅ PASS — no behavioral edits to branch-aware code paths | +| III | Type Safety & Explicit Contracts | Exception narrowing strengthens explicit contracts; mypy/ty must stay green. | ✅ PASS — narrowing names real exception types; `invoke backend.lint` (ruff+ty+mypy) is an exit gate | +| IV | Test Discipline | Existing tests must keep passing; no new feature surface needing new tests. | ✅ PASS — verification relies on existing suites + lint-gate mutation check (SC-006); no assertions change | +| V | Query Performance & Efficiency | No queries added or modified. | ✅ PASS | +| VI | Security & Input Boundaries | Auth error-handling semantics unchanged (suppression-only in auth files). | ✅ PASS — narrowing in auth paths is explicitly forbidden by plan policy | +| VII | Simplicity & Maintainability | Minimal diffs; no new abstractions or helpers. | ✅ PASS — per-site edits only; no shared "safe_catch" utility invented | + +**Quality gates** (constitution §Development Workflow): format (`uv run invoke format` must produce no diff on touched files), lint (`uv run invoke backend.lint` + repo-wide ruff check), tests (touched-module suites), changelog (not required — internal housekeeping, no user-facing change; recorded in spec Assumptions). + +**Post-design re-check (after Phase 1)**: ✅ PASS — design introduces no schema, query, auth, dependency, or generated-file changes; Complexity Tracking is empty. + +## Project Structure + +### Documentation (this feature) + +```text +specs/002-ruff-ble-reenable/ +├── spec.md # Feature specification +├── plan.md # This file +├── research.md # Phase 0: rule semantics, house method, policy decisions +├── data-model.md # Phase 1: full 78-site inventory with per-site treatment +├── quickstart.md # Phase 1: validation guide (commands + expected outcomes) +├── checklists/ +│ └── requirements.md # Spec quality checklist (complete) +└── tasks.md # Phase 2 output (/speckit-tasks — not created by /speckit-plan) +``` + +*(`specs/` is a symlink to `dev/specs/` — canonical git path is `dev/specs/002-ruff-ble-reenable/`.)* + +### Source Code (repository root) + +```text +pyproject.toml # [tool.ruff.lint] ignore: remove "BLE" (~line 511) +backend/infrahub/ +├── api/{auth,oauth2,oidc}.py # 4 sites — suppress (auth constraint) +├── auth/auth.py # 4 sites — suppress (auth constraint) +├── artifacts/tasks.py # 1 site +├── cli/upgrade.py # 2 sites +├── core/migrations/graph/m0*.py # 27 sites — suppress (migration constraint) +├── core/migrations/shared.py # 3 sites — suppress (migration constraint) +├── core/schema/update_coordinator.py # 2 sites +├── core/validators/tasks.py # 1 site +├── generators/tasks.py # 1 site +├── git/{integrator,sync}.py # 2 sites +├── message_bus/operations/__init__.py # 1 site +├── services/scheduler.py # 1 site +├── task_manager/flow_run/retention.py # 1 site +├── telemetry/tasks.py # 3 sites +└── webhook/tasks/process.py # 1 site +backend/tests/ +├── component/core/schema/schema_branch/… # 4 sites (2 files) +├── helpers/{diagnostics,events,test_worker}.py # 4 sites (incl. the BaseException handler) +├── integration/git/conftest.py # 2 sites +├── integration_docker/test_merge_kill_recovery.py # 1 site +└── scale/common/protocols.py # 2 sites +tasks/release.py # 2 sites +tests/e2e/data/parity.py # 1 site +utilities/infrahub_load_tester.py # 8 sites +``` + +**Structure Decision**: No structural changes. The feature is a config-line removal plus point edits at the 78 inventoried handler sites listed above; the authoritative per-site treatment matrix lives in [data-model.md](data-model.md). + +## Implementation Approach + +1. **Fix order** (fail-fast, house method's ~10-file batches): + 1. Batch A — hard-constraint suppressions, migrations (30 sites, annotation-only). + 2. Batch B — hard-constraint suppressions, auth (8 sites, annotation-only). + 3. Batch C — backend runtime sites (16, per-site treatment from data-model.md). + 4. Batch D — backend test sites (13, incl. the `BaseException` handler decision). + 5. Batch E — tooling sites (11: `tasks/release.py`, `tests/e2e/data/parity.py`, `utilities/infrahub_load_tester.py`). + 6. Config flip — remove `"BLE"` from `pyproject.toml` ignore list (only after all sites are clean under `--select=BLE`). +2. **Per-batch verification**: `uv run ruff check --select=BLE ` clean; `uv run ruff format --check` clean on touched files; batch-scoped tests where they exist. +3. **Final verification** (quickstart.md): repo-root `uv run ruff check --select=BLE .` → 0; `uv run ruff check . --exclude python_sdk` → 0; `uv run invoke backend.lint` → exit 0; mutation check (SC-006); diff audit of constraint areas (SC-007); touched-module unit/component tests. + +Justification-comment style (from repo guideline `dev/guidelines/backend/python.md` §Exception Handling): comment states *why the broad catch is required at this boundary* (keep-alive loop, best-effort cleanup, per-item migration continuation, auth degradation), placed on or immediately above the `except` line; suppression is always `# noqa: BLE001` (line-targeted, rule-targeted). + +## Complexity Tracking + +> No constitution violations — table intentionally empty. diff --git a/dev/specs/002-ruff-ble-reenable/quickstart.md b/dev/specs/002-ruff-ble-reenable/quickstart.md new file mode 100644 index 00000000000..972dc75e2c7 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/quickstart.md @@ -0,0 +1,97 @@ +# Quickstart: Validating the BLE re-enablement + +**Plan**: [plan.md](plan.md) | **Site inventory**: [data-model.md](data-model.md) + +Run everything from the repository root. Prerequisite: `uv sync --all-groups` already done (standard dev setup). + +## 1. Card acceptance checks + +```bash +# SC-001 — zero BLE violations repo-wide (baseline before the change: 78) +uv run ruff check --select=BLE . +# expected: "All checks passed!" + +# FR-001 — BLE gone from the global ignore list +grep -n '"BLE"' pyproject.toml +# expected: no match (exit 1) + +# SC-002 — card acceptance lint gate +uv run invoke backend.lint +# expected: exit 0 (ruff, ty, mypy all green) +``` + +## 2. CI-equivalent gates (stricter than the card) + +```bash +# the exact CI lint commands (.github/workflows/ci.yml:325-328) +uv run ruff check . --exclude python_sdk +uv run ruff format --check --diff --exclude python_sdk . +# expected: both exit 0 +``` + +## 3. Enforcement mutation check (SC-006) + +```bash +# temporarily plant an unjustified blind except in a linted file +cat >> tasks/utils.py <<'EOF' + + +def _ble_canary() -> None: + try: + pass + except Exception: + pass +EOF +uv run ruff check --select=BLE tasks/utils.py +# expected: 1 × BLE001 reported (proves the rule is live) +git checkout -- tasks/utils.py # remove the canary +``` + +## 4. Suppression audit (SC-003 / SC-004) + +```bash +# every suppression is line-targeted, justified, and enumerable in one search +grep -rn "noqa: BLE001" --include="*.py" . --exclude-dir=python_sdk --exclude-dir=.venv +# expected: each hit sits on an `except Exception`/`except BaseException` line +# with a justification comment on or immediately above it + +# no bare excepts anywhere in linted code (E722 backstop) +uv run ruff check --select=E722 . +# expected: "All checks passed!" +``` + +## 5. Behavior-preservation audit for hard-constraint areas (SC-007) + +```bash +# migrations + auth diffs must contain ONLY comment/noqa additions +git diff ..HEAD -- backend/infrahub/core/migrations/ \ + backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py \ + backend/infrahub/api/oidc.py backend/infrahub/auth/ +# expected: every changed hunk adds comments/`# noqa: BLE001` only; +# no executable line added, removed, or reordered +``` + +## 6. Tests for touched modules (SC-005) + +```bash +# cheap tier — full backend unit suite +uv run invoke backend.test-unit +# expected: passes (same result as base branch) + +# the two component-test files that were themselves violation sites +uv run pytest backend/tests/component/core/schema/schema_branch/test_process_idempotency.py \ + backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py +# expected: pass (requires local testcontainers; if unavailable, defer to CI and record it) +``` + +## Expected end state + +| Check | Expected | +|-------|----------| +| `ruff check --select=BLE .` | 0 violations (was 78) | +| `"BLE"` in `pyproject.toml` | absent | +| `ruff check . --exclude python_sdk` | exit 0 | +| `invoke backend.lint` | exit 0 | +| `noqa: BLE001` count | equals data-model.md SUPPRESS count, each justified | +| Migration/auth diffs | annotation-only | +| Unit tests | green | diff --git a/dev/specs/002-ruff-ble-reenable/research.md b/dev/specs/002-ruff-ble-reenable/research.md new file mode 100644 index 00000000000..3845eec8a9f --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/research.md @@ -0,0 +1,79 @@ +# Research: Re-enable ruff BLE (blind-except) rule + +**Date**: 2026-07-22 | **Plan**: [plan.md](plan.md) + +All Technical Context unknowns resolved. Every decision below was verified against the working tree on branch `pha/INBOX-19` (commands run 2026-07-22). + +## R1. Rule semantics (`ruff rule BLE001`) + +**Decision**: Treat BLE001 as flagging `except Exception:` and `except BaseException:` handlers, with two built-in exemptions that require no code change: handlers whose body re-raises (`raise`), and handlers that call stdlib `logging.exception(...)` / log with `exc_info=True` (recognized only for `lint.logger-objects`, which this repo does not configure). + +**Rationale**: Verified via `uv run ruff rule BLE001`. The repo logs through structlog (`infrahub.log.get_logger()`), which ruff does not recognize as a logger object here — so `log.exception(...)` at a flagged site does *not* exempt it. All 78 flagged sites are therefore genuine work items; none can be resolved by "it already logs". + +**Alternatives considered**: Configuring `lint.logger-objects` to teach ruff about structlog loggers, auto-exempting `log.exception` handlers. Rejected: it silently weakens the rule repo-wide (any future `except Exception: log.exception(...)` would pass without justification), diverges from the card's per-site auditability requirement, and would be a semantic lint-config change beyond the card's scope. + +## R2. Enforcement gates and verification commands + +**Decision**: Verify against four gates, in this order: (1) `uv run ruff check --select=BLE .` (card acceptance, repo root), (2) `uv run ruff check . --exclude python_sdk` (the exact CI lint command, `.github/workflows/ci.yml:326`), (3) `uv run invoke backend.lint` (ruff `--diff` + format check + ty + mypy over `backend/`, card acceptance), (4) `uv run ruff format --check --diff --exclude python_sdk .` (CI format step; touched files must stay format-clean). + +**Rationale**: `invoke backend.lint`'s ruff step runs `ruff check --diff backend`, which only surfaces *fixable* diagnostics — BLE001 has no autofix, so gate (3) alone would not prove BLE cleanliness; gate (2) is the gate that actually fails CI on any missed site anywhere in the repo (including `tasks/`, `utilities/`, `tests/e2e/`). Verified that the CLI `--select=BLE` overrides the config ignore list, so gate (1) works both before the config flip (inventory: 78) and after (must be 0). + +**Alternatives considered**: Relying only on the card's two acceptance commands. Rejected: they under-test — CI's full-repo `ruff check` is stricter than both. + +## R3. Site inventory (ground truth) + +**Decision**: Scope = the 78 BLE001 sites across 46 files measured on this branch (77 × `except Exception`, 1 × `except BaseException` in `backend/tests/helpers/test_worker.py:107`), re-measured at final verification. Full per-site inventory with treatments: [data-model.md](data-model.md). + +**Rationale**: The card's "~32 sites" dates from the 2026-02-18 analysis; graph migrations added since (m043–m074) contribute ~20 new sites. Counted via `ruff check --select=BLE --output-format=concise .` → 78 matches, 46 unique files. + +**Alternatives considered**: Fixing only the original ~32. Rejected: the config flip is all-or-nothing — CI runs repo-wide, so every current site must be resolved. + +## R4. Treatment policy (narrow vs. suppress) + +**Decision**: Two treatments, assigned per site in data-model.md: + +- **NARROW** — replace `Exception` with the specific type(s) the guarded block realistically raises (project base `infrahub.exceptions.Error` counts as a legitimate narrowing when the try block only raises Infrahub errors). Only where the raisable set is identifiable with high confidence AND letting unexpected types propagate is acceptable at that boundary. +- **SUPPRESS** — keep `except Exception` byte-identical, append `# noqa: BLE001` on the except line, add a one-line justification comment. Mandatory for hard-constraint areas (all of `backend/infrahub/core/migrations/`, and `backend/infrahub/api/{auth,oauth2,oidc}.py` + `backend/infrahub/auth/`); default for keep-alive boundaries (worker/task loops, telemetry, webhook dispatch, best-effort cleanup, per-item migration continuation) and for load-test statistics loops. + +Never widen; never introduce bare `except:` (E722 guards that anyway); the single `BaseException` site is resolved per its harness-isolation purpose (analysis in data-model.md) — either justified as-is or reduced to `Exception` only if that provably cannot change what the test harness intercepts. + +**Rationale**: Matches the card's suggested solution verbatim and the pre-existing house guideline `dev/guidelines/backend/python.md` §Exception Handling ("broad `except Exception` is justified only at a top-level boundary… log the exception… never discard it"). The hard-constraint mandate (suppression-only in migrations/auth) is the conservative reading of the card's "no DB schema or migration changes / no auth changes": narrowing changes which exception types propagate — a runtime behavior change — while comment + noqa additions are semantically inert. + +**Alternatives considered**: (a) Narrowing migration handlers to `(Error, Neo4jError, …)` — rejected: any mis-enumeration alters migration failure behavior on real customer data; prohibited by constraint. (b) Adding `BLE001` to `per-file-ignores` for `backend/tests/**` or `backend/infrahub/core/migrations/**` — rejected: blanket suppression removes per-site auditability, re-creates the debt invisibly, and violates spec FR-001. (c) Wrapping broad catches in a shared helper (`with suppress_and_log(...)`) — rejected: behavior-affecting refactor, violates minimal-change method and constitution VII (premature abstraction). + +## R5. Suppression style + +**Decision**: Line-targeted `# noqa: BLE001` on the `except` line, with a brief justification comment immediately above the `except` line (or inline where the line stays ≤ line-length 120). Comment states *why arbitrary failures must be absorbed at that boundary* — not what the code does. Example: + +```python +# Keep-alive boundary: one failing scheduled task must not kill the scheduler loop. +except Exception as exc: # noqa: BLE001 +``` + +**Rationale**: Ruff `noqa` must be on the diagnostic's line to take effect; rule-targeted form keeps every other rule active on that line. RUF100 (`unused-noqa`) is enabled via `select = ["ALL"]`, so any `noqa: BLE001` that stops matching a real diagnostic fails the lint gate — this keeps suppressions load-bearing (spec FR-010) with no extra tooling. + +**Alternatives considered**: Bare `# noqa` (rejected: suppresses all rules on the line, RUF100-unfriendly); block-level `# ruff: noqa: BLE001` file pragmas (rejected: file-wide suppression, not auditable per site). + +## R6. Batching, tests, and fix order + +**Decision**: Execute in six batches — (A) migrations 30, (B) auth 8 (both suppression-only), (C) backend runtime 16, (D) backend tests 13, (E) tooling 11, then (F) flip the config (remove `"BLE"` from `ignore`) and run full verification. After each batch: `ruff check --select=BLE` on touched paths + `ruff format --check` on touched files. Local test obligation: `uv run invoke backend.test-unit` once after all code batches, plus module-scoped component tests for `backend/tests/component/core/schema/schema_branch/*` (the two component test files touched are themselves the tests to run — they are runnable locally with testcontainers when available; if the local environment cannot run them, record that and defer to CI). + +**Rationale**: House `/fix-ruff-rule` method mandates ~10-file batches and validation between steps. Config flip goes last so the tree is never in a state where the active rule fails mid-work. Unit tests are the cheap tier proving no import-time or behavior regressions; the only *behavior-relevant* narrowings land in batches C–E, whose modules are covered by the unit suite where coverage exists. + +**Alternatives considered**: Flipping the config first and fixing until green — rejected: leaves the working tree failing lint at every intermediate commit, breaking the checkpoint-commit convention. + +## R7. Changelog fragment + +**Decision**: Add a towncrier fragment `changelog/+ruff-ble-blind-except.housekeeping.md` (orphan `+` prefix — no GitHub issue; `housekeeping` type exists in `[tool.towncrier.type]` with existing precedent, e.g. `+downsize-docker-image.housekeeping.md`). + +**Rationale**: Cheap, follows observed repo practice for internal improvements, and pre-empts review churn. Refines the spec assumption ("no fragment required") in the permitted direction — the spec explicitly allows adding one. + +**Alternatives considered**: No fragment (spec default) — kept as fallback if towncrier lint rejects the orphan fragment for any reason. + +## R8. Out-of-scope boundaries (verified) + +- `python_sdk/` — git submodule, separate repo, excluded by the CI command. Untouched. +- `python_testcontainers/` — carries its own `[tool.ruff]` config (own select/ignore); repo-root ruff runs lint it under *its* config; it reports no BLE001 today. Untouched. +- `backend/tests/fixtures/repos/**` — fixture repos with their own `pyproject.toml`; not governed by the root config. Untouched. +- Generated dirs (`backend/infrahub/core/schema/generated/`, `protocols.py`, `*/graphql_queries/*.py`) — zero BLE001 sites there today; will not be edited. +- CI workflows — read for gate discovery only (`ci.yml:325-328`); no edits. From 6018a80eb9fbe0acc4945552881e595ace9e87ad Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:40:00 +0000 Subject: [PATCH 03/14] [Spec Kit] Add critique report and apply its recommendations (INBOX-19) Verdict PROCEED, no must-address findings. Applied: PR-narrative note (plan), rollback + release.py sanity checks (quickstart), changelog assumption harmonized (spec). Co-Authored-By: Claude Fable 5 --- .../critiques/critique-20260722-073834.md | 92 +++++++++++++++++++ dev/specs/002-ruff-ble-reenable/plan.md | 2 + dev/specs/002-ruff-ble-reenable/quickstart.md | 11 +++ dev/specs/002-ruff-ble-reenable/spec.md | 2 +- 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 dev/specs/002-ruff-ble-reenable/critiques/critique-20260722-073834.md diff --git a/dev/specs/002-ruff-ble-reenable/critiques/critique-20260722-073834.md b/dev/specs/002-ruff-ble-reenable/critiques/critique-20260722-073834.md new file mode 100644 index 00000000000..909e71f1d63 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/critiques/critique-20260722-073834.md @@ -0,0 +1,92 @@ +# Critique Report: Re-enable ruff BLE (blind-except) rule + +**Date**: 2026-07-22 07:38 UTC | **Spec**: [../spec.md](../spec.md) | **Plan**: [../plan.md](../plan.md) +**Reviewed artifacts**: spec.md, plan.md, research.md, data-model.md, quickstart.md, constitution v1.0.0. No tasks.md yet (correct at this stage); no prior critiques. + +## Executive Summary + +The spec and plan are unusually well-grounded for a lint-debt card: the 78-site inventory was measured (not estimated), every site has a verified per-site treatment with justification text, and the risk posture is deliberately conservative — **zero of the 8 narrowings touch production runtime code** (6 are in test helpers/fixtures, 2 in release tooling), while all 70 suppressions are annotation-only. Constitution gates all pass; the hard constraints (no migration/auth/API/dependency/CI changes) are structurally respected by the treatment policy rather than by hope. + +The main critique themes are about **honest value framing** (the outcome is enforcement + auditability, not mass narrowing — the PR must say so) and **small artifact inconsistencies** (spec assumption vs. plan decision on the changelog fragment; a missing cheap verification for the only two behavior-relevant non-test narrowings). No must-address findings. + +**Verdict: ✅ PROCEED** — with three low-risk artifact touch-ups applied as remediation (see Findings). + +## Product Lens Findings + +### 3a. Problem Validation — strong + +Evidence chain is unusually good: the team's own 2026-02-18 suppression analysis ranked BLE #1; the violation count grew 32 → 78 in five months (m043–m074 migrations), which *is* the cost-of-inaction data point. Scope (all current violations + the config flip) is the minimal scope that can ship at all, since CI lints the whole repo. + +### 3b. User Value Assessment — solid, one framing gap + +- P1 (enforcement) / P2 (narrowing) / P3 (auditable suppressions) are independently testable and correctly prioritized: enforcement is the durable value. +- **Finding P1 (💡)**: The measured outcome is 70/78 sites suppressed. That is the *correct* outcome per the card's own instructions and the house guideline (most sites are genuine defensive boundaries), but a reviewer skimming the card ("replace the blind except with the specific exception type(s)") may read 90% `noqa` as evasion. The value story must be framed as: (a) future blind-excepts blocked at lint time, (b) all 70 intentional broad catches now carry explicit, greppable justifications, (c) zero production behavior risk. → Add a "PR narrative" note to plan.md's Implementation Approach so the implement/report phases carry it into the PR description. + +### 3c. Alternative Approaches — adequately explored + +research.md R4/R5 reject the three plausible shortcuts (logger-objects config, per-file-ignores, shared helper) for the right reasons. Not building = debt keeps compounding at the observed ~9 sites/month. + +### 3d. Edge Cases & User Experience — covered + +BaseException handler, S110/RUF100 interplay, base-branch drift, fixture-repo scopes all addressed. Developer-facing friction (in-flight branches hitting BLE001 after this merges) is standard lint-rule lifecycle, communicated via the changelog fragment — resolved, no action. + +### 3e. Success Measurement — measurable + +SC-001..SC-007 are all commands or countable greps. **Finding P2 (💡)**: rollback story is trivial (revert / re-add one ignore line) but unstated; one sentence in quickstart makes the operational posture explicit. → Applied with remediation. + +## Engineering Lens Findings + +### 4a. Architecture Soundness — sound + +No new abstractions (constitution VII respected); follows the documented `/fix-ruff-rule` house method including its ~10-file batching. + +### 4b. Failure Mode Analysis — strong posture, one residual + +- The classic failure mode of blind-except cleanups — a narrowed handler letting a previously-absorbed exception escape in production — is structurally avoided: **no production runtime site is narrowed** (verified in data-model.md; all 16 runtime sites SUPPRESS, migrations/auth mandated SUPPRESS). +- **Finding E1 (💡)**: The only narrowings whose behavior matters outside CI-tested paths are `tasks/release.py:155/242` (`InvalidVersion`) and the two integration-fixture poll loops (`httpx.HTTPError`). Release tasks have no test coverage; integration fixtures don't run locally. Both narrowings are analytically solid (in-file precedent at release.py:273/297; httpx hierarchy verified), but a 30-second local sanity check exists and should be in the validation guide: `uv run python -c "from packaging.version import Version, InvalidVersion; Version('1.2.3-foo')"` (expect `InvalidVersion` raised) + `uv run invoke --list` (proves release.py imports). → Applied to quickstart §6. +- **Finding E2 (💡)**: spec Assumptions state "No changelog fragment is required" while research.md R7 decides to add one. Both are defensible; the artifacts should agree. → Spec assumption reworded to record the R7 decision. + +### 4c. Security & Privacy Review — pass + +Auth sites are annotation-only; fail-closed contracts (401/502) preserved verbatim; justification comments were fact-checked against the actual handler behavior (e.g. `AuthorizationError` → 401, `GatewayError` → 502). No new attack surface; no secrets; no trust-boundary change. + +### 4d. Performance & Scalability — N/A + +No runtime-path changes. + +### 4e. Testing Strategy — appropriate to risk, one accepted gap + +Unit suite + the two touched component-test files cover the NARROW sites that run locally. **Accepted risk (recorded)**: the `tests/integration/git/conftest.py` narrowings are only exercised by CI's integration tier; the fallback (SUPPRESS) is documented in data-model.md if CI disagrees. The tasks phase must carry this as an explicit verification note rather than silent hope. + +### 4f. Operational Readiness — pass + +Rollback = one-line ignore re-add or revert (see P2). No migrations, no deploys, no flags. + +### 4g. Dependencies & Integration Risks — pass + +`packaging`, `httpx`, `infrahub.exceptions.SchemaNotFoundError` all verified present/importable; no new dependencies; `python_sdk`/`python_testcontainers`/fixture-repo scopes verified out of scope. + +## Cross-Lens Insights + +- **X1 (✅ convergent strength)**: The conservative treatment policy simultaneously satisfies the product constraint (zero behavior change where it's prohibited) and the engineering risk posture (zero production regressions) while still delivering the durable product value (the gate + auditability). The 70/78 ratio is the *evidence* the policy was followed, not a shortcut. +- **X2 (💡 = P1+E1)**: The PR narrative should lead with: "enforcement on; 78 sites resolved; 8 narrowings, all in tests/tooling; zero production behavior change; 5 latent defects documented for follow-up." That single paragraph pre-empts both the product ("did we get value?") and engineering ("did we break something?") reviewer questions. + +## Findings Summary Table + +| ID | Lens | Severity | Category | Finding | Suggestion | Resolution | +|----|------|----------|----------|---------|------------|------------| +| P1 | Product | 💡 | User Value | 70/78 suppression ratio needs honest framing or reads as evasion | Add PR-narrative note to plan.md Implementation Approach | **Applied** (plan.md) | +| P2 | Product | 💡 | Success Measurement | Rollback story unstated | One line in quickstart | **Applied** (quickstart.md) | +| E1 | Engineering | 💡 | Testing | No local check for release.py narrowing (untested module) | Add `InvalidVersion` sanity + `invoke --list` import check to quickstart §6 | **Applied** (quickstart.md) | +| E2 | Engineering | 💡 | Consistency | Spec assumption contradicts research R7 on changelog fragment | Harmonize spec Assumptions with R7 decision | **Applied** (spec.md) | +| E3 | Engineering | ✅ note | Failure Modes | Zero production-runtime narrowings — structural risk avoidance | None (record) | Recorded | +| E4 | Engineering | 💡 | Testing | Integration-fixture narrowings only verified in CI | Tasks phase must carry an explicit "CI-verified" note + documented SUPPRESS fallback | **Carried to tasks phase** | +| X1 | Both | ✅ note | Scope × Risk | Treatment policy converges product constraint & eng risk | None (record) | Recorded | +| X2 | Both | 💡 | Communication | PR description content | Single narrative paragraph (see above) | **Carried to implement/report phase** | +| Q1 | Both | 🤔 | Follow-ups | 5 latent defects found during analysis (partial-commit-in-txn ×4, auth message masking, load-tester unbound var, bare-Exception signal) — file tickets? | Out of this card's scope; surface in final implementation report for the human to triage into tickets | **Resolved autonomously**: report-only (filing tickets is an outward action beyond the card) | + +🎯 Must-Address: **none**. 💡 Recommendations: 6 (5 applied/carried, 1 record-only). 🤔 Questions: 1 (resolved autonomously, rationale above). + +## Verdict + +✅ **PROCEED** — no must-address findings; recommendations applied or explicitly carried into the tasks/implement phases. Next step: `/speckit-tasks`. diff --git a/dev/specs/002-ruff-ble-reenable/plan.md b/dev/specs/002-ruff-ble-reenable/plan.md index 2b44e280b09..1048e40965e 100644 --- a/dev/specs/002-ruff-ble-reenable/plan.md +++ b/dev/specs/002-ruff-ble-reenable/plan.md @@ -115,6 +115,8 @@ utilities/infrahub_load_tester.py # 8 sites Justification-comment style (from repo guideline `dev/guidelines/backend/python.md` §Exception Handling): comment states *why the broad catch is required at this boundary* (keep-alive loop, best-effort cleanup, per-item migration continuation, auth degradation), placed on or immediately above the `except` line; suppression is always `# noqa: BLE001` (line-targeted, rule-targeted). +**PR narrative** (critique finding P1/X2 — carry into the implementation report and PR description): lead with "BLE001 enforcement is now on; 78 sites resolved — 8 narrowed (all in tests/tooling, none in production runtime), 70 kept intentionally broad with per-site justification + `noqa`; zero production behavior change; 5 latent defects discovered during analysis are documented for follow-up, deliberately not fixed here." The 70/78 suppression ratio is the evidence the conservative treatment policy was followed — most sites are genuine defensive boundaries where the house guideline itself prescribes a documented broad catch. + ## Complexity Tracking > No constitution violations — table intentionally empty. diff --git a/dev/specs/002-ruff-ble-reenable/quickstart.md b/dev/specs/002-ruff-ble-reenable/quickstart.md index 972dc75e2c7..3c2ef539517 100644 --- a/dev/specs/002-ruff-ble-reenable/quickstart.md +++ b/dev/specs/002-ruff-ble-reenable/quickstart.md @@ -74,6 +74,13 @@ git diff ..HEAD -- backend/infrahub/core/migrations/ \ ## 6. Tests for touched modules (SC-005) ```bash +# the two behavior-relevant tooling narrowings (tasks/release.py → InvalidVersion) +uv run python -c "from packaging.version import Version; Version('1.2.3-foo')" +# expected: raises packaging.version.InvalidVersion (proves the narrowed type +# is exactly what non-standard versions raise) +uv run invoke --list > /dev/null && echo "invoke imports OK" +# expected: "invoke imports OK" (proves tasks/release.py still imports) + # cheap tier — full backend unit suite uv run invoke backend.test-unit # expected: passes (same result as base branch) @@ -95,3 +102,7 @@ uv run pytest backend/tests/component/core/schema/schema_branch/test_process_ide | `noqa: BLE001` count | equals data-model.md SUPPRESS count, each justified | | Migration/auth diffs | annotation-only | | Unit tests | green | + +**Rollback**: single-commit revert, or re-add `"BLE"` to the `[tool.ruff.lint]` ignore list — the code fixes remain valid either way (narrowed handlers and justified suppressions are correct with or without the rule active). + +**CI-only verification (accepted)**: the two `tests/integration/git/conftest.py` narrowings (`httpx.HTTPError`) are exercised only by CI's integration tier; if CI shows the poll loops now failing on something httpx-shaped that isn't `HTTPError`, fall back to the documented SUPPRESS treatment for those two sites (data-model.md Batch D). diff --git a/dev/specs/002-ruff-ble-reenable/spec.md b/dev/specs/002-ruff-ble-reenable/spec.md index 1df5e37a575..50bec5c3d79 100644 --- a/dev/specs/002-ruff-ble-reenable/spec.md +++ b/dev/specs/002-ruff-ble-reenable/spec.md @@ -123,6 +123,6 @@ As a future maintainer reading defensive code (top-level task loops, best-effort - "No DB schema or migration changes" (hard constraint) is interpreted as *no changes to what migrations do* — adding a comment and a `noqa` marker to an existing migration file does not constitute a migration change; renumbering, semantic edits, or new migrations would. The same interpretation applies to "no auth changes". - The suppress-with-justification treatment is the *default* for defensive top-level loops (scheduled tasks, telemetry, webhook dispatch, artifact/generator task wrappers) because those handlers exist precisely to keep the worker loop alive against arbitrary failures; narrowing is reserved for handlers guarding a small, analyzable expression. - "Tests related to touched modules pass" means the locally runnable test tiers (backend unit tests; component tests where practical) for the modules whose files changed; full integration/e2e tiers run in CI as usual and are not a local exit criterion. -- No changelog fragment is required: the change is developer-facing housekeeping with no user-visible behavior change (consistent with towncrier fragments being for user-facing changes). If reviewers disagree, a `housekeeping` fragment can be added at review time. +- A towncrier `housekeeping` changelog fragment is included (plan decision R7): the change is developer-facing housekeeping with no user-visible behavior change, and the repo has precedent for recording such changes under the registered `housekeeping` type. - `ruff rule BLE001` (the house method's "understand the rule first" step) confirms the rule flags `except Exception` and `except BaseException` handlers; ruff never flags narrower catches, so narrowing always satisfies the rule. - The `python_testcontainers/` directory and test-fixture repos carry their own ruff configuration scopes; the authoritative in-scope file set is exactly what the CI command reports. From 9c42b3d1234c5c2bf13ef97516b907b02eb77fce Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:42:33 +0000 Subject: [PATCH 04/14] [Spec Kit] Add task breakdown for ruff BLE re-enablement (INBOX-19) 22 tasks across 3 user stories: 8 parallel suppress batches (US3), 3 parallel narrow batches (US2), config flip + verification (US1), plus setup drift-check and polish audits. Co-Authored-By: Claude Fable 5 --- dev/specs/002-ruff-ble-reenable/tasks.md | 130 +++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 dev/specs/002-ruff-ble-reenable/tasks.md diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md new file mode 100644 index 00000000000..6b48566a531 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -0,0 +1,130 @@ +# Tasks: Re-enable ruff BLE (blind-except) rule and fix all violations + +**Input**: Design documents from `specs/002-ruff-ble-reenable/` + +**Prerequisites**: plan.md, spec.md, research.md, **data-model.md (authoritative per-site treatment matrix — every fix task below references its batch table)**, quickstart.md + +**Tests**: No new tests are written (spec FR-008: existing tests must pass unchanged). Verification is lint-gate + existing-suite based. + +**Organization**: Grouped by user story. **Execution order is inverted relative to story priority**: US1 (P1, enforcement flip) *depends on* US2+US3 (all 78 sites fixed) because CI lints the whole repo the moment the ignore entry is removed. US3 and US2 are mutually independent and internally parallel. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: US1 = enforcement active; US2 = handlers narrowed; US3 = broad catches justified + +**Editing rules for every fix task** (data-model.md "Normalization rules"): SUPPRESS = justification comment on its own line immediately above the `except` line + bare `# noqa: BLE001` appended to the `except` line; keep existing adjacent comments; zero semantic tokens changed. NARROW = change only the caught-type expression + add the import per file convention. Never touch handler bodies. After each task: `uv run ruff check --select=BLE ` reports 0 for those files AND `uv run ruff format --check ` is clean. + +--- + +## Phase 1: Setup + +**Purpose**: Confirm the working inventory still matches the plan before editing. + +- [ ] T001 Re-measure the violation inventory from repo root with `uv run ruff check --select=BLE --output-format=concise .` and reconcile against the 78 sites in specs/002-ruff-ble-reenable/data-model.md; if any site moved (line drift) locate it by handler shape in the same file; if any *new* site appeared, classify it with the same policy (constraint area → SUPPRESS; defensive boundary → SUPPRESS; enumerable surface → NARROW) and append it to the matching batch table in specs/002-ruff-ble-reenable/data-model.md before proceeding + +--- + +## Phase 2: Foundational + +**No foundational tasks** — the feature has no shared scaffolding; Phase 1's inventory check is the only prerequisite. Proceed directly to the story phases. + +--- + +## Phase 3: User Story 3 — Genuinely-broad catches are explicit and justified (Priority: P3) — executes first + +**Goal**: All 70 SUPPRESS sites carry a truthful justification comment + line-targeted `# noqa: BLE001`, with zero behavioral change (annotation-only diffs). + +**Independent Test**: `uv run ruff check --select=BLE ` reports 0; `git diff` for migration/auth files shows only comment/noqa additions. + +### Implementation for User Story 3 + +- [ ] T002 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m014–m047 (13 sites): backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py:39, m029_duplicates_cleanup.py:656, m036_drop_attr_value_index.py:39, m043_create_hfid_display_label_in_db.py:116+168, m044_backfill_hfid_display_label_in_db.py:382+514, m045_backfill_hfid_display_label_in_db_profile_template.py:82+163, m046_fill_agnostic_hfid_display_labels.py:141+196, m047_backfill_or_null_display_label.py:416+465 — use each row's exact justification comment +- [ ] T003 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m059–m074 (14 sites): backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py:238+247+381+420, m062_recompute_permission_display_labels.py:454+473, m063_template_number_pool_cleanup.py:82, m064_template_ip_pool_relationship_cleanup.py:98, m066_consolidate_duplicate_number_pools.py:82, m070_normalize_mac_address_values_to_colon.py:225, m071_recompute_hfid_for_ip_attributes.py:180, m072_index_hfid_values.py:169, m073_unify_ip_pool_resource_identifier.py:336, m074_normalize_indexed_hfid_values.py:156 — m066/m073 use the transaction-safe wording (no atomicity claims) +- [ ] T004 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to backend/infrahub/core/migrations/shared.py:157+245+277 (3 sites; :157/:245 use the per-query wording without atomicity claims) +- [ ] T005 [P] [US3] Apply SUPPRESS per data-model.md Batch B rows to the 8 auth sites: backend/infrahub/api/auth.py:63+116, backend/infrahub/api/oauth2.py:205, backend/infrahub/api/oidc.py:259, backend/infrahub/auth/auth.py:542+558+668+679 — annotation-only; fail-closed comments exactly as tabled +- [ ] T006 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 1, 9 sites / 8 files): backend/infrahub/artifacts/tasks.py:49, backend/infrahub/cli/upgrade.py:65+244, backend/infrahub/core/schema/update_coordinator.py:350+365, backend/infrahub/core/validators/tasks.py:85, backend/infrahub/generators/tasks.py:253, backend/infrahub/git/integrator.py:383, backend/infrahub/git/sync.py:120 (sync.py: keep the existing lines-121-122 comment, add noqa only) +- [ ] T007 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 2, 7 sites / 5 files): backend/infrahub/message_bus/operations/__init__.py:34, backend/infrahub/services/scheduler.py:89, backend/infrahub/task_manager/flow_run/retention.py:63, backend/infrahub/telemetry/tasks.py:129+152+159 (:129 has an existing intent comment ~line 125 — add noqa, extend comment only if it doesn't say why broad), backend/infrahub/webhook/tasks/process.py:90 +- [ ] T008 [P] [US3] Apply SUPPRESS per data-model.md Batch D rows to the 7 backend-test suppress sites: backend/tests/helpers/diagnostics.py:103+179, backend/tests/helpers/events.py:51, backend/tests/helpers/test_worker.py:107 (**stays `except BaseException`** — use the ready-future justification comment verbatim), backend/tests/integration_docker/test_merge_kill_recovery.py:85 (keep existing lines-86-88 comment, add noqa + tabled comment), backend/tests/scale/common/protocols.py:28+53 +- [ ] T009 [P] [US3] Apply SUPPRESS per data-model.md Batch E rows to the 9 tooling suppress sites: tests/e2e/data/parity.py:81 (keep/extend the existing trailing comment) and utilities/infrahub_load_tester.py:47+69+84+108+113+138+148+165 (do **not** fix the pre-existing missing-`return` at :69 — behavior preservation, see data-model.md Latent defects) +- [ ] T010 [US3] Story checkpoint: run `uv run ruff check --select=BLE backend/infrahub backend/tests tests/e2e/data/parity.py utilities/infrahub_load_tester.py` — every remaining violation must be one of the 8 NARROW sites only; run `uv run ruff format --check` on all files touched by T002–T009 (clean); run `git diff -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` and verify every hunk is comment/noqa-only (spec SC-007) + +**Checkpoint**: All intentional broad catches are now auditable; only the 8 NARROW sites still flag. + +--- + +## Phase 4: User Story 2 — Existing blind handlers are narrowed to real failure modes (Priority: P2) + +**Goal**: The 8 analyzable handlers catch the specific exception types the guarded code raises; handler bodies untouched. + +**Independent Test**: `uv run ruff check --select=BLE backend/tests/component/core/schema/schema_branch backend/tests/integration/git/conftest.py tasks/release.py` reports 0; existing tests pass unchanged. + +### Implementation for User Story 2 + +- [ ] T011 [P] [US2] Narrow the duplicated `_describe_hash_diff` helper per data-model.md Batch D: replace `except Exception` with `except SchemaNotFoundError` at backend/tests/component/core/schema/schema_branch/test_process_idempotency.py:158+164 and backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py:42+48, adding `from infrahub.exceptions import SchemaNotFoundError` to each file's imports +- [ ] T012 [P] [US2] Narrow the two poll loops in backend/tests/integration/git/conftest.py:31+53 per data-model.md Batch D: `except Exception` → `except httpx.HTTPError` (httpx already imported) and **remove the now-stale `# noqa: S110` on those lines** (typed excepts are S110-exempt; RUF100 fails on unused noqa) +- [ ] T013 [P] [US2] Narrow the two version-probe handlers in tasks/release.py:155+242 per data-model.md Batch E: `except Exception` → `except InvalidVersion`, extending the **function-local** imports (~line 115 and ~line 213) to `from packaging.version import InvalidVersion, Version` — do not hoist to module level (locals are deliberate so invoke runs without dev deps) +- [ ] T014 [US2] Story checkpoint: `uv run ruff check --select=BLE .` from repo root reports **0** (all 78 resolved); `uv run ruff format --check` clean on the 6 narrowed files; sanity checks `uv run python -c "from packaging.version import Version; Version('1.2.3-foo')"` (expect InvalidVersion raised) and `uv run invoke --list > /dev/null` (imports OK); run `uv run pytest backend/tests/component/core/schema/schema_branch/test_process_idempotency.py backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py` if the local environment supports testcontainers — otherwise record "deferred to CI" with the reason (critique E4: conftest narrowings are CI-verified by design; documented fallback = SUPPRESS per data-model.md) + +**Checkpoint**: Zero BLE001 violations repo-wide; rule can now be activated. + +--- + +## Phase 5: User Story 1 — Blind-except enforcement is active for all future code (Priority: P1) 🎯 the durable value + +**Goal**: BLE is enforced by the normal lint gates; a new unjustified blind except fails lint locally and in CI. + +**Independent Test**: spec US1 acceptance scenarios — config flipped, full gates green, canary mutation fails lint. + +### Implementation for User Story 1 + +- [ ] T015 [US1] Remove the `"BLE", # flake8-blind-except (BLE)` line from the `[tool.ruff.lint]` `ignore` list in pyproject.toml (~line 511) — depends on T010 + T014 (all sites resolved) +- [ ] T016 [P] [US1] Add towncrier fragment changelog/+ruff-ble-blind-except.housekeeping.md: one sentence stating the BLE (flake8-blind-except) ruff rule is now enforced — blind `except Exception` handlers are either narrowed or carry an explicit justified `# noqa: BLE001` +- [ ] T017 [US1] Full-gate verification (quickstart.md §1–2): `uv run ruff check --select=BLE .` → 0; `uv run ruff check . --exclude python_sdk` → exit 0; `uv run ruff format --check --diff --exclude python_sdk .` → exit 0; `uv run invoke backend.lint` → exit 0 (ruff + ty + mypy) +- [ ] T018 [US1] Enforcement mutation check (quickstart.md §3, spec SC-006): append the canary `except Exception: pass` function to tasks/utils.py, verify `uv run ruff check --select=BLE tasks/utils.py` reports exactly 1 × BLE001, then `git checkout -- tasks/utils.py` and verify the tree is clean + +**Checkpoint**: Enforcement live; all card acceptance criteria met except final audits. + +--- + +## Phase 6: Polish & Cross-Cutting Verification + +**Purpose**: Auditability proofs and existing-suite regression evidence (spec SC-003/004/005). + +- [ ] T019 [P] Suppression audit (quickstart.md §4): `grep -rn "noqa: BLE001" --include="*.py" . --exclude-dir=python_sdk --exclude-dir=.venv` — count must equal the SUPPRESS total from data-model.md (70, plus any T001 additions); each hit sits on an `except Exception`/`except BaseException` line with a justification comment on or immediately above it; `uv run ruff check --select=E722 .` → 0 bare excepts +- [ ] T020 [P] Run `uv run invoke backend.test-unit` — must pass with unchanged results (spec SC-005); if any failure, it must be traceable to something other than this change (compare against base) before proceeding +- [ ] T021 Re-verify hard-constraint diffs end-state (spec SC-007): `git diff ..HEAD -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` contains only comment/`noqa` additions; record the diff summary in the implementation report +- [ ] T022 Run the complete quickstart.md top-to-bottom as a final pass and record each command's outcome (this is the evidence table for the implementation report; include the T014 component-test outcome or its CI-deferral note) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: none — start immediately. +- **Foundational (Phase 2)**: empty — skip. +- **US3 (Phase 3) and US2 (Phase 4)**: both depend only on T001. T002–T009 and T011–T013 are all [P] — 11 independent fix tasks touching disjoint file sets; T010 and T014 are their story checkpoints. +- **US1 (Phase 5)**: T015 depends on T010 + T014 (every site resolved). T016 is independent [P]. T017–T018 depend on T015. +- **Polish (Phase 6)**: T019–T021 depend on T017; T022 last. + +### Story Dependency Note (deviation from template independence) + +US1 (P1) is *implemented last* despite being the highest-value story: activating the rule before the 78 sites are resolved would fail every lint gate. This inversion is inherent to lint-enablement work and was accepted in the spec ("enforcement without narrowing is impossible — CI would fail"). US2 and US3 are fully independent of each other (disjoint files) and each independently testable via per-file `ruff check --select=BLE`. + +### Parallel Opportunities + +```text +After T001: + T002 | T003 | T004 | T005 | T006 | T007 | T008 | T009 (US3 — 8 parallel suppress batches) + T011 | T012 | T013 (US2 — 3 parallel narrow batches) +Then: T010 (US3 gate) and T014 (US2 gate) +Then: T015 → (T016 parallel) → T017 → T018 +Then: T019 | T020 → T021 → T022 +``` + +## Implementation Strategy + +Single-branch, incremental, committed per logical group (checkpoint-commit convention). MVP = all of US3+US2+US1 — this feature only ships whole (the config flip is all-or-nothing). Stop-and-validate points are T010, T014, T017. If T017 fails on a rule other than BLE (e.g. RUF100 on a mistyped noqa), fix within the offending task's scope and re-run. If a NARROW site proves wrong at T014/T020 (an expected exception type escapes in tests), fall back to that site's documented SUPPRESS treatment rather than widening the narrow set speculatively. + +**PR narrative reminder** (plan.md / critique X2): enforcement on; 78 sites — 8 narrowed (tests/tooling only), 70 justified suppressions; zero production behavior change; 5 latent defects documented for follow-up, deliberately untouched. From a590463f1dac41b6342434e20a061fc38483532f Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:43:50 +0000 Subject: [PATCH 05/14] =?UTF-8?q?[Spec=20Kit]=20Add=20spec/ask=20alignment?= =?UTF-8?q?=20check=20=E2=80=94=20verdict=20ALIGNED=20(INBOX-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source PRD resolved: inline card text + Slack suppression-analysis thread (fetched). No missing/changed/dropped requirements; 0 of 2 remediation passes used. Co-Authored-By: Claude Fable 5 --- .../002-ruff-ble-reenable/alignment-check.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 dev/specs/002-ruff-ble-reenable/alignment-check.md diff --git a/dev/specs/002-ruff-ble-reenable/alignment-check.md b/dev/specs/002-ruff-ble-reenable/alignment-check.md new file mode 100644 index 00000000000..a2a1d0b742b --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/alignment-check.md @@ -0,0 +1,33 @@ +# Spec/Ask Alignment Check + +**Date**: 2026-07-22 | **Spec**: [spec.md](spec.md) | **Checked after**: tasks.md generation (commit 9c42b3d12) + +## 1. Source + +- **Inline ask** (primary requirements): Engineering Inbox card INBOX-19 text passed to `/speckit.opsmill.auto` — context, suggested solution, acceptance criteria, scorecard, hard constraints. +- **Referenced URL** (context/evidence): Patrick Ogenstad's 2026-02-18 Slack thread, channel `C051C8WQ4C9`, ts `1771428456.822439` — **fetched successfully via the Slack connector**. The thread contains the full suppression-impact analysis; it ranks "Re-enable BLE (32 violations)" as priority #1 (Tier 1 — Direct Bug Risk, effort Low) and endorses the fix-in-small-chunks / agent-in-background workflow. It adds *evidence*, not requirements, beyond the inline ask. + +## 2. Verdict + +**✅ ALIGNED** + +## 3. Findings + +| Severity | Category | PRD reference | Spec reference | Description | +|----------|----------|---------------|----------------|-------------| +| none | — | "Remove BLE from the global ruff ignore list (~line 511)" | FR-001, tasks T015 | Present, exact. | +| none | — | "replace the blind except with the specific exception type(s) the guarded code can actually raise" | FR-003(a), US2 | Present; per-site analysis (data-model.md) determines where this is achievable. | +| none | — | "where a broad catch is genuinely required… keep `except Exception` with a targeted `# noqa: BLE001` and a brief justification comment" | FR-003(b), FR-010, US3 | Present, verbatim policy. | +| none | — | "never a bare `except:` that also swallows KeyboardInterrupt/SystemExit" | FR-004, SC-004 | Present; E722 backstop added. | +| none | — | House method `/fix-ruff-rule` (understand rule, minimal changes, preserve functionality) | research R1/R6, FR-005 | Followed, including ~10-file batching. | +| none | — | Acceptance: `ruff check --select=BLE .` clean; BLE out of ignore; `invoke backend.lint` passes; touched-module tests pass | SC-001, FR-001, SC-002, SC-005 | All present; spec **adds** the stricter CI-equivalent gate (`ruff check . --exclude python_sdk`) — necessary elaboration, not scope creep (CI must pass for the card to be done). | +| none | — | Hard constraints (no DB schema/migration changes, no API contract changes, no auth changes, no new deps, no CI workflow changes, no generated-file edits; STOP if required) | FR-006, FR-009, plan Constitution Check | Respected structurally: constraint areas are suppression-only/annotation-only; no stop-condition triggered. | +| info | interpretation (documented) | "no DB schema or migration changes… no auth changes" | spec Assumptions ¶2, FR-006 | 30 violations live *inside* migration files and 8 inside auth files; fixing "all violations" while never touching those files is unsatisfiable. Spec resolves this as "no *semantic* changes" — comment/`noqa` additions only, byte-equivalent runtime behavior, verified by diff audit (SC-007, T021). This is the only reading under which the card is internally consistent, and it is the conservative one. | +| info | superseded detail | "~32 sites" (card + Slack thread, measured 2026-02-18) | spec Context, Assumptions ¶1 | Measured ground truth on this branch is 78 sites / 46 files (migrations m043–m074 added since February). Scope follows the card's operative clause ("fix **all** of its violations"), not the stale count. | +| minor | added (convention) | — | spec Assumptions (changelog), research R7, T016 | Spec/plan add a towncrier `housekeeping` changelog fragment. Not requested by the PRD, but repo-convention compliance with existing precedent; zero scope risk. | + +No PRD requirement is missing, no requirement semantics changed, no acceptance criterion dropped or softened (two were strengthened), and no spec addition contradicts a PRD constraint. + +## 4. Action + +**Proceed.** No remediation passes needed (0 of 2 budget used). The two `info` rows are documented interpretations already carried in spec Assumptions; the `minor` addition is deliberate and reversible. From e24482113e02b30c9784ece3fb32f20f0b4a97c7 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 07:52:23 +0000 Subject: [PATCH 06/14] =?UTF-8?q?[Spec=20Kit]=20Complete=20T001=20inventor?= =?UTF-8?q?y=20re-measurement=20=E2=80=94=2078/78=20sites=20reconciled,=20?= =?UTF-8?q?no=20drift=20(INBOX-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-ran `uv run ruff check --select=BLE --output-format=concise .` from the repo root: 78 BLE001 violations (77 `Exception`, 1 `BaseException` at backend/tests/helpers/test_worker.py:107), byte-identical to the data-model.md batch tables — zero line drift, zero new sites, zero removed sites, no python_sdk findings. data-model.md unchanged; only the T001 checkbox is ticked. Co-Authored-By: Claude Fable 5 --- dev/specs/002-ruff-ble-reenable/tasks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 6b48566a531..bb2bf14b744 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -21,7 +21,7 @@ **Purpose**: Confirm the working inventory still matches the plan before editing. -- [ ] T001 Re-measure the violation inventory from repo root with `uv run ruff check --select=BLE --output-format=concise .` and reconcile against the 78 sites in specs/002-ruff-ble-reenable/data-model.md; if any site moved (line drift) locate it by handler shape in the same file; if any *new* site appeared, classify it with the same policy (constraint area → SUPPRESS; defensive boundary → SUPPRESS; enumerable surface → NARROW) and append it to the matching batch table in specs/002-ruff-ble-reenable/data-model.md before proceeding +- [X] T001 Re-measure the violation inventory from repo root with `uv run ruff check --select=BLE --output-format=concise .` and reconcile against the 78 sites in specs/002-ruff-ble-reenable/data-model.md; if any site moved (line drift) locate it by handler shape in the same file; if any *new* site appeared, classify it with the same policy (constraint area → SUPPRESS; defensive boundary → SUPPRESS; enumerable surface → NARROW) and append it to the matching batch table in specs/002-ruff-ble-reenable/data-model.md before proceeding --- From 0c1a9bdaa83caf966ab329a938daebbff4101def Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:01:25 +0000 Subject: [PATCH 07/14] =?UTF-8?q?[Spec=20Kit]=20Suppress=20BLE001=20in=20g?= =?UTF-8?q?raph=20migrations=20=E2=80=94=20Batch=20A=20(INBOX-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks T002–T004: annotation-only SUPPRESS treatment for all 30 Batch A sites per data-model.md — a justification comment above each except plus a line-targeted `# noqa: BLE001`. Zero semantic tokens changed (SC-007); m066/m073 and shared.py:157/:245 use the transaction-safe wordings. The noqas read as RUF100 "non-enabled" until T015 removes BLE from the ignore list — expected interim state. Co-Authored-By: Claude Fable 5 --- .../migrations/graph/m014_remove_index_attr_value.py | 3 ++- .../core/migrations/graph/m029_duplicates_cleanup.py | 3 ++- .../migrations/graph/m036_drop_attr_value_index.py | 3 ++- .../graph/m043_create_hfid_display_label_in_db.py | 6 ++++-- .../graph/m044_backfill_hfid_display_label_in_db.py | 6 ++++-- ...fill_hfid_display_label_in_db_profile_template.py | 6 ++++-- .../graph/m046_fill_agnostic_hfid_display_labels.py | 6 ++++-- .../graph/m047_backfill_or_null_display_label.py | 6 ++++-- .../graph/m059_fix_hfid_display_label_nulls.py | 12 ++++++++---- .../m062_recompute_permission_display_labels.py | 6 ++++-- .../graph/m063_template_number_pool_cleanup.py | 3 ++- .../m064_template_ip_pool_relationship_cleanup.py | 3 ++- .../graph/m066_consolidate_duplicate_number_pools.py | 3 ++- .../m070_normalize_mac_address_values_to_colon.py | 3 ++- .../graph/m071_recompute_hfid_for_ip_attributes.py | 3 ++- .../core/migrations/graph/m072_index_hfid_values.py | 3 ++- .../graph/m073_unify_ip_pool_resource_identifier.py | 3 ++- .../graph/m074_normalize_indexed_hfid_values.py | 3 ++- backend/infrahub/core/migrations/shared.py | 9 ++++++--- dev/specs/002-ruff-ble-reenable/tasks.md | 6 +++--- 20 files changed, 63 insertions(+), 33 deletions(-) diff --git a/backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py b/backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py index 6fa6b449b71..232675a5ecf 100644 --- a/backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py +++ b/backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py @@ -36,7 +36,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: index_manager = IndexManagerNeo4j(db=db) index_manager.init(nodes=[INDEX_TO_DELETE], rels=[]) await index_manager.drop() - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result diff --git a/backend/infrahub/core/migrations/graph/m029_duplicates_cleanup.py b/backend/infrahub/core/migrations/graph/m029_duplicates_cleanup.py index dad337ff77e..fc9a4fc05a3 100644 --- a/backend/infrahub/core/migrations/graph/m029_duplicates_cleanup.py +++ b/backend/infrahub/core/migrations/graph/m029_duplicates_cleanup.py @@ -653,7 +653,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: duplicate_relationships_cleanup_query = await DeleteDuplicateRelationships.init(db=db) await duplicate_relationships_cleanup_query.execute(db=db) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 migration_result.errors.append(str(exc)) return migration_result diff --git a/backend/infrahub/core/migrations/graph/m036_drop_attr_value_index.py b/backend/infrahub/core/migrations/graph/m036_drop_attr_value_index.py index beb9261916b..b7c0e495260 100644 --- a/backend/infrahub/core/migrations/graph/m036_drop_attr_value_index.py +++ b/backend/infrahub/core/migrations/graph/m036_drop_attr_value_index.py @@ -36,7 +36,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: index_manager = IndexManagerNeo4j(db=db) index_manager.init(nodes=[INDEX_TO_DELETE], rels=[]) await index_manager.drop() - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result diff --git a/backend/infrahub/core/migrations/graph/m043_create_hfid_display_label_in_db.py b/backend/infrahub/core/migrations/graph/m043_create_hfid_display_label_in_db.py index 8b11666f414..d09dc5a6407 100644 --- a/backend/infrahub/core/migrations/graph/m043_create_hfid_display_label_in_db.py +++ b/backend/infrahub/core/migrations/graph/m043_create_hfid_display_label_in_db.py @@ -113,7 +113,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: execution_result = await migration.execute(migration_input=migration_input, branch=default_branch) result.errors.extend(execution_result.errors) progress.update(update_task, advance=1) - except Exception as exc: + # First failing sub-migration is recorded as a result error and aborts the remaining steps + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result @@ -165,7 +166,8 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: execution_result = await migration.execute(migration_input=migration_input, branch=branch) result.errors.extend(execution_result.errors) progress.update(update_task, advance=1) - except Exception as exc: + # First failing sub-migration is recorded as a result error and aborts the remaining steps + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result diff --git a/backend/infrahub/core/migrations/graph/m044_backfill_hfid_display_label_in_db.py b/backend/infrahub/core/migrations/graph/m044_backfill_hfid_display_label_in_db.py index 930b4d6b5ce..63961fc52b2 100644 --- a/backend/infrahub/core/migrations/graph/m044_backfill_hfid_display_label_in_db.py +++ b/backend/infrahub/core/migrations/graph/m044_backfill_hfid_display_label_in_db.py @@ -379,7 +379,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: update_task=update_task, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() @@ -511,6 +512,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: at=at, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py b/backend/infrahub/core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py index 2d98fe0e554..d9732d0156e 100644 --- a/backend/infrahub/core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py +++ b/backend/infrahub/core/migrations/graph/m045_backfill_hfid_display_label_in_db_profile_template.py @@ -79,7 +79,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: update_task=update_task, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() @@ -160,6 +161,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: at=at, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py b/backend/infrahub/core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py index 3bfffbb3297..bee656a57f6 100644 --- a/backend/infrahub/core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py +++ b/backend/infrahub/core/migrations/graph/m046_fill_agnostic_hfid_display_labels.py @@ -138,7 +138,8 @@ async def _do_one_schema_all( async def execute(self, migration_input: MigrationInput) -> MigrationResult: try: return await self._do_execute(migration_input=migration_input) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) async def _do_execute(self, migration_input: MigrationInput) -> MigrationResult: @@ -193,7 +194,8 @@ async def _do_execute(self, migration_input: MigrationInput) -> MigrationResult: execution_result = await migration.execute(migration_input=migration_input, branch=global_branch) result.errors.extend(execution_result.errors) progress.update(update_task, advance=1) - except Exception as exc: + # First failing sub-migration is recorded as a result error and aborts the remaining steps + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result diff --git a/backend/infrahub/core/migrations/graph/m047_backfill_or_null_display_label.py b/backend/infrahub/core/migrations/graph/m047_backfill_or_null_display_label.py index 689e4802417..290b9efb39d 100644 --- a/backend/infrahub/core/migrations/graph/m047_backfill_or_null_display_label.py +++ b/backend/infrahub/core/migrations/graph/m047_backfill_or_null_display_label.py @@ -413,7 +413,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: update_task=backfill_task, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() @@ -462,6 +463,7 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: at=at, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py b/backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py index db9f98ef66e..7a3322dc631 100644 --- a/backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py +++ b/backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py @@ -235,7 +235,8 @@ async def _compute_values_for_batch( value = await self._compute_display_label(db=db, schema=schema, node=node, console=console) if value is not None: dl_values[node_uuid] = value - except Exception as exc: + # Best-effort per-node recompute: record the failure, skip this node, keep fixing the rest + except Exception as exc: # noqa: BLE001 console.print(f" Skipping display_label for {node_uuid} ({kind}): {exc}") errors.append(f"display_label compute failed for {node_uuid} ({kind}): {exc}") @@ -244,7 +245,8 @@ async def _compute_values_for_batch( value = await self._compute_hfid(db=db, schema=schema, node=node, console=console) if value is not None: hfid_values[node_uuid] = value - except Exception as exc: + # Best-effort per-node recompute: record the failure, skip this node, keep fixing the rest + except Exception as exc: # noqa: BLE001 console.print(f" Skipping human_friendly_id for {node_uuid} ({kind}): {exc}") errors.append(f"human_friendly_id compute failed for {node_uuid} ({kind}): {exc}") @@ -378,7 +380,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: else: console.print("No nodes with bad values found on global branch") - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result @@ -417,5 +420,6 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: progress=progress, progress_task=task, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) diff --git a/backend/infrahub/core/migrations/graph/m062_recompute_permission_display_labels.py b/backend/infrahub/core/migrations/graph/m062_recompute_permission_display_labels.py index 680fd93c3dc..a26923af40b 100644 --- a/backend/infrahub/core/migrations/graph/m062_recompute_permission_display_labels.py +++ b/backend/infrahub/core/migrations/graph/m062_recompute_permission_display_labels.py @@ -451,7 +451,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: update_task=update_task, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}" return MigrationResult(errors=[error_msg]) @@ -470,7 +471,8 @@ async def execute_against_branch(self, migration_input: MigrationInput, branch: await self._compute_object_permission_display_labels( db=db, branch=branch, attribute_schema=display_label_attribute_schema, console=console ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}" return MigrationResult(errors=[error_msg]) diff --git a/backend/infrahub/core/migrations/graph/m063_template_number_pool_cleanup.py b/backend/infrahub/core/migrations/graph/m063_template_number_pool_cleanup.py index da6f416ecde..cff37508721 100644 --- a/backend/infrahub/core/migrations/graph/m063_template_number_pool_cleanup.py +++ b/backend/infrahub/core/migrations/graph/m063_template_number_pool_cleanup.py @@ -79,7 +79,8 @@ async def _process_templates( ) await query.execute(db=db) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}" return MigrationResult(errors=[error_msg]) diff --git a/backend/infrahub/core/migrations/graph/m064_template_ip_pool_relationship_cleanup.py b/backend/infrahub/core/migrations/graph/m064_template_ip_pool_relationship_cleanup.py index 4dd7e5b96f5..fba488d66f5 100644 --- a/backend/infrahub/core/migrations/graph/m064_template_ip_pool_relationship_cleanup.py +++ b/backend/infrahub/core/migrations/graph/m064_template_ip_pool_relationship_cleanup.py @@ -95,7 +95,8 @@ async def _process_templates( migration_input=migration_input, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}" return MigrationResult(errors=[error_msg]) diff --git a/backend/infrahub/core/migrations/graph/m066_consolidate_duplicate_number_pools.py b/backend/infrahub/core/migrations/graph/m066_consolidate_duplicate_number_pools.py index e706239d928..b6ea4902762 100644 --- a/backend/infrahub/core/migrations/graph/m066_consolidate_duplicate_number_pools.py +++ b/backend/infrahub/core/migrations/graph/m066_consolidate_duplicate_number_pools.py @@ -79,7 +79,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: await self._update_schema_parameters(db=dbt, pool_id_map=pool_id_map, console=console) - except Exception as exc: + # Failures become MigrationResult errors so the runner reports them instead of crashing + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {repr(exc)}" return MigrationResult(errors=[error_msg]) diff --git a/backend/infrahub/core/migrations/graph/m070_normalize_mac_address_values_to_colon.py b/backend/infrahub/core/migrations/graph/m070_normalize_mac_address_values_to_colon.py index f6b16cc80ac..c1fa388deec 100644 --- a/backend/infrahub/core/migrations/graph/m070_normalize_mac_address_values_to_colon.py +++ b/backend/infrahub/core/migrations/graph/m070_normalize_mac_address_values_to_colon.py @@ -222,7 +222,8 @@ async def _run( display_label_attribute_schema=display_label_attribute_schema, at=at, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc) or f"{type(exc).__name__}: {repr(exc)}"]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m071_recompute_hfid_for_ip_attributes.py b/backend/infrahub/core/migrations/graph/m071_recompute_hfid_for_ip_attributes.py index 1ca5ed8f374..413f27e8da3 100644 --- a/backend/infrahub/core/migrations/graph/m071_recompute_hfid_for_ip_attributes.py +++ b/backend/infrahub/core/migrations/graph/m071_recompute_hfid_for_ip_attributes.py @@ -177,7 +177,8 @@ async def _run( display_label_attribute_schema=display_label_attribute_schema, at=at, ) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc) or f"{type(exc).__name__}: {repr(exc)}"]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m072_index_hfid_values.py b/backend/infrahub/core/migrations/graph/m072_index_hfid_values.py index 56d8628aa75..fe700408b83 100644 --- a/backend/infrahub/core/migrations/graph/m072_index_hfid_values.py +++ b/backend/infrahub/core/migrations/graph/m072_index_hfid_values.py @@ -166,7 +166,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: try: await self._normalize_hfid_values(db=db) await self._index_hfid_values(db=db) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/graph/m073_unify_ip_pool_resource_identifier.py b/backend/infrahub/core/migrations/graph/m073_unify_ip_pool_resource_identifier.py index a075c9c8aa0..8d7487b534c 100644 --- a/backend/infrahub/core/migrations/graph/m073_unify_ip_pool_resource_identifier.py +++ b/backend/infrahub/core/migrations/graph/m073_unify_ip_pool_resource_identifier.py @@ -333,7 +333,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: await self._bootstrap_core_ip_pool_generic(db=dbt, at=schema_root_at, user_id=user_id) await self._append_inherit_from_for_all_pools(db=dbt) await self._rewrite_resource_relationship_attributes(db=dbt) - except Exception as exc: + # Failures become MigrationResult errors so the runner reports them instead of crashing + except Exception as exc: # noqa: BLE001 error_msg = str(exc) or f"{type(exc).__name__}: {exc!r}" return MigrationResult(errors=[error_msg]) diff --git a/backend/infrahub/core/migrations/graph/m074_normalize_indexed_hfid_values.py b/backend/infrahub/core/migrations/graph/m074_normalize_indexed_hfid_values.py index 04244413166..77288a981a7 100644 --- a/backend/infrahub/core/migrations/graph/m074_normalize_indexed_hfid_values.py +++ b/backend/infrahub/core/migrations/graph/m074_normalize_indexed_hfid_values.py @@ -153,7 +153,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: db = migration_input.db try: await self._normalize_hfid_values(db=db) - except Exception as exc: + # Migration contract: failures become MigrationResult errors; the runner reports them and halts + except Exception as exc: # noqa: BLE001 return MigrationResult(errors=[str(exc)]) return MigrationResult() diff --git a/backend/infrahub/core/migrations/shared.py b/backend/infrahub/core/migrations/shared.py index c2fd3eeaad3..3d186106fc0 100644 --- a/backend/infrahub/core/migrations/shared.py +++ b/backend/infrahub/core/migrations/shared.py @@ -154,7 +154,8 @@ async def execute_queries( ) await query.execute(db=migration_input.db) result.nbr_migrations_executed += query.get_nbr_migrations_executed() - except Exception as exc: + # Per-query failures become result errors so the runner reports them instead of crashing + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result @@ -242,7 +243,8 @@ async def do_execute(self, migration_input: MigrationInput) -> MigrationResult: try: query = await migration_query.init(db=migration_input.db, at=migration_input.at) await query.execute(db=migration_input.db) - except Exception as exc: + # Per-query failures become result errors so the runner reports them instead of crashing + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result @@ -274,7 +276,8 @@ async def execute(self, migration_input: MigrationInput) -> MigrationResult: try: execution_result = await migration.execute(migration_input=migration_input, branch=default_branch) result.errors.extend(execution_result.errors) - except Exception as exc: + # First failing sub-migration is recorded as a result error and aborts the remaining steps + except Exception as exc: # noqa: BLE001 result.errors.append(str(exc)) return result diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index bb2bf14b744..6e71b6f9766 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -39,9 +39,9 @@ ### Implementation for User Story 3 -- [ ] T002 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m014–m047 (13 sites): backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py:39, m029_duplicates_cleanup.py:656, m036_drop_attr_value_index.py:39, m043_create_hfid_display_label_in_db.py:116+168, m044_backfill_hfid_display_label_in_db.py:382+514, m045_backfill_hfid_display_label_in_db_profile_template.py:82+163, m046_fill_agnostic_hfid_display_labels.py:141+196, m047_backfill_or_null_display_label.py:416+465 — use each row's exact justification comment -- [ ] T003 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m059–m074 (14 sites): backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py:238+247+381+420, m062_recompute_permission_display_labels.py:454+473, m063_template_number_pool_cleanup.py:82, m064_template_ip_pool_relationship_cleanup.py:98, m066_consolidate_duplicate_number_pools.py:82, m070_normalize_mac_address_values_to_colon.py:225, m071_recompute_hfid_for_ip_attributes.py:180, m072_index_hfid_values.py:169, m073_unify_ip_pool_resource_identifier.py:336, m074_normalize_indexed_hfid_values.py:156 — m066/m073 use the transaction-safe wording (no atomicity claims) -- [ ] T004 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to backend/infrahub/core/migrations/shared.py:157+245+277 (3 sites; :157/:245 use the per-query wording without atomicity claims) +- [X] T002 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m014–m047 (13 sites): backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py:39, m029_duplicates_cleanup.py:656, m036_drop_attr_value_index.py:39, m043_create_hfid_display_label_in_db.py:116+168, m044_backfill_hfid_display_label_in_db.py:382+514, m045_backfill_hfid_display_label_in_db_profile_template.py:82+163, m046_fill_agnostic_hfid_display_labels.py:141+196, m047_backfill_or_null_display_label.py:416+465 — use each row's exact justification comment +- [X] T003 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m059–m074 (14 sites): backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py:238+247+381+420, m062_recompute_permission_display_labels.py:454+473, m063_template_number_pool_cleanup.py:82, m064_template_ip_pool_relationship_cleanup.py:98, m066_consolidate_duplicate_number_pools.py:82, m070_normalize_mac_address_values_to_colon.py:225, m071_recompute_hfid_for_ip_attributes.py:180, m072_index_hfid_values.py:169, m073_unify_ip_pool_resource_identifier.py:336, m074_normalize_indexed_hfid_values.py:156 — m066/m073 use the transaction-safe wording (no atomicity claims) +- [X] T004 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to backend/infrahub/core/migrations/shared.py:157+245+277 (3 sites; :157/:245 use the per-query wording without atomicity claims) - [ ] T005 [P] [US3] Apply SUPPRESS per data-model.md Batch B rows to the 8 auth sites: backend/infrahub/api/auth.py:63+116, backend/infrahub/api/oauth2.py:205, backend/infrahub/api/oidc.py:259, backend/infrahub/auth/auth.py:542+558+668+679 — annotation-only; fail-closed comments exactly as tabled - [ ] T006 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 1, 9 sites / 8 files): backend/infrahub/artifacts/tasks.py:49, backend/infrahub/cli/upgrade.py:65+244, backend/infrahub/core/schema/update_coordinator.py:350+365, backend/infrahub/core/validators/tasks.py:85, backend/infrahub/generators/tasks.py:253, backend/infrahub/git/integrator.py:383, backend/infrahub/git/sync.py:120 (sync.py: keep the existing lines-121-122 comment, add noqa only) - [ ] T007 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 2, 7 sites / 5 files): backend/infrahub/message_bus/operations/__init__.py:34, backend/infrahub/services/scheduler.py:89, backend/infrahub/task_manager/flow_run/retention.py:63, backend/infrahub/telemetry/tasks.py:129+152+159 (:129 has an existing intent comment ~line 125 — add noqa, extend comment only if it doesn't say why broad), backend/infrahub/webhook/tasks/process.py:90 From 5b438fe6e8e832c238eb526c9ff00948a3fb5451 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:09:14 +0000 Subject: [PATCH 08/14] =?UTF-8?q?[Spec=20Kit]=20Suppress=20BLE001=20in=20a?= =?UTF-8?q?uth=20+=20backend=20runtime=20=E2=80=94=20Batches=20B/C=20(INBO?= =?UTF-8?q?X-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks T005–T007: annotation-only SUPPRESS treatment for the 8 Batch B auth sites and 16 Batch C backend-runtime sites per data-model.md — a justification comment above each except plus a line-targeted `# noqa: BLE001`. Zero semantic tokens changed; auth diffs are comment/noqa-only (SC-007). Two flagged sites got noqa only, keeping their existing justification comments: git/sync.py:120 (lines 121-122) and telemetry/tasks.py:129 (line 125 already states why broad). The noqas read as RUF100 "non-enabled" until T015 removes BLE from the ignore list — expected interim state. Co-Authored-By: Claude Fable 5 --- backend/infrahub/api/auth.py | 6 ++++-- backend/infrahub/api/oauth2.py | 3 ++- backend/infrahub/api/oidc.py | 3 ++- backend/infrahub/artifacts/tasks.py | 3 ++- backend/infrahub/auth/auth.py | 12 ++++++++---- backend/infrahub/cli/upgrade.py | 6 ++++-- backend/infrahub/core/schema/update_coordinator.py | 6 ++++-- backend/infrahub/core/validators/tasks.py | 3 ++- backend/infrahub/generators/tasks.py | 3 ++- backend/infrahub/git/integrator.py | 3 ++- backend/infrahub/git/sync.py | 2 +- backend/infrahub/message_bus/operations/__init__.py | 3 ++- backend/infrahub/services/scheduler.py | 3 ++- backend/infrahub/task_manager/flow_run/retention.py | 3 ++- backend/infrahub/telemetry/tasks.py | 8 +++++--- backend/infrahub/webhook/tasks/process.py | 3 ++- dev/specs/002-ruff-ble-reenable/tasks.md | 6 +++--- 17 files changed, 49 insertions(+), 27 deletions(-) diff --git a/backend/infrahub/api/auth.py b/backend/infrahub/api/auth.py index 34d73055e73..c3716374a3c 100644 --- a/backend/infrahub/api/auth.py +++ b/backend/infrahub/api/auth.py @@ -60,7 +60,8 @@ async def login_user( auth_method=AuthMethod.PASSWORD, ) await service.event.send(event=event) - except Exception as ex: + # Login event emission is best-effort telemetry; it must never fail a successful login + except Exception as ex: # noqa: BLE001 log.warning(f"Failed to emit login event for account_id={auth_result.account_id}: {str(ex)}") return auth_result.token @@ -113,7 +114,8 @@ async def logout( session_id=session_id, ) await service.event.send(event=event) - except Exception as ex: + # Logout event emission is best-effort telemetry; it must never fail a successful logout + except Exception as ex: # noqa: BLE001 log.warning(f"Failed to emit logout event for account_id={user_session.account_id}: {str(ex)}") delete_response_cookies(response=response) diff --git a/backend/infrahub/api/oauth2.py b/backend/infrahub/api/oauth2.py index 8d01c4814f7..328cf5b4a08 100644 --- a/backend/infrahub/api/oauth2.py +++ b/backend/infrahub/api/oauth2.py @@ -202,7 +202,8 @@ async def token( identity_source=provider_name, ) await service.event.send(event=event) - except Exception as ex: + # Login event emission is best-effort telemetry; it must never fail a successful OAuth2 login + except Exception as ex: # noqa: BLE001 log.warning(f"Failed to emit OAuth2 login event for account_id={auth_result.account_id}: {str(ex)}") return models.UserTokenWithUrl( diff --git a/backend/infrahub/api/oidc.py b/backend/infrahub/api/oidc.py index 5f8e11b845d..ea2a1c3cbe7 100644 --- a/backend/infrahub/api/oidc.py +++ b/backend/infrahub/api/oidc.py @@ -256,7 +256,8 @@ async def token( identity_source=provider_name, ) await service.event.send(event=event) - except Exception as ex: + # Login event emission is best-effort telemetry; it must never fail a successful OIDC login + except Exception as ex: # noqa: BLE001 log.warning(f"Failed to emit OIDC login event for account_id={auth_result.account_id}: {str(ex)}") return models.UserTokenWithUrl( diff --git a/backend/infrahub/artifacts/tasks.py b/backend/infrahub/artifacts/tasks.py index 5ee43ea1221..a413cdc75e2 100644 --- a/backend/infrahub/artifacts/tasks.py +++ b/backend/infrahub/artifacts/tasks.py @@ -46,7 +46,8 @@ async def create(model: CheckArtifactCreate) -> ValidatorConclusion: check_message = "Artifact rendered successfully" conclusion = ValidatorConclusion.SUCCESS - except Exception as exc: + # Check boundary: any render failure must be recorded as a failed artifact check, not crash the flow + except Exception as exc: # noqa: BLE001 artifact.status.value = "Error" await artifact.save() severity = "critical" diff --git a/backend/infrahub/auth/auth.py b/backend/infrahub/auth/auth.py index 295b4c36a43..f1240a7f5b7 100644 --- a/backend/infrahub/auth/auth.py +++ b/backend/infrahub/auth/auth.py @@ -539,7 +539,8 @@ async def validate_jwt_access_token(token: str) -> AccountSession: session_id = payload["session_id"] except jwt.ExpiredSignatureError: raise AuthorizationError("Expired Signature") from None - except Exception: + # Fail closed: any undecodable or malformed token must map to a 401 auth error, never a 500 + except Exception: # noqa: BLE001 raise AuthorizationError("Invalid token") from None if payload["type"] == "access": @@ -555,7 +556,8 @@ async def validate_jwt_refresh_token(db: InfrahubDatabase, token: str) -> models session_id = payload["session_id"] except jwt.ExpiredSignatureError: raise AuthorizationError("Expired Signature") from None - except Exception: + # Fail closed: any undecodable or malformed refresh token must map to a 401, never a 500 + except Exception: # noqa: BLE001 raise AuthorizationError("Invalid token") from None await validate_active_account(db=db, account_id=str(account_id)) @@ -665,7 +667,8 @@ def safe_get_response_body(response: httpx.Response, raise_error_on_empty_body: # Try to parse as JSON first try: return response.json() - except Exception as json_error: + # Providers may return non-JSON bodies: fall back to text or fail closed with GatewayError (502) + except Exception as json_error: # noqa: BLE001 try: # Try to get as text text_body = response.text @@ -676,7 +679,8 @@ def safe_get_response_body(response: httpx.Response, raise_error_on_empty_body: status_code=response.status_code, ) raise GatewayError(message="Authentication provider returned an empty response") from json_error - except Exception: + # If the body cannot be read at all, fail closed with GatewayError (502) rather than a 500 + except Exception: # noqa: BLE001 log.error( "Unable to read response body from authentication provider", url=str(response.url), diff --git a/backend/infrahub/cli/upgrade.py b/backend/infrahub/cli/upgrade.py index 2f3b8734a94..4b35d6692cd 100644 --- a/backend/infrahub/cli/upgrade.py +++ b/backend/infrahub/cli/upgrade.py @@ -62,7 +62,8 @@ async def validate_prerequisites(db: InfrahubDatabase) -> bool: except DatabaseError as exc: console.log(f"{ERROR_BADGE} Database prerequisite check failed: {exc}") return False - except Exception as exc: + # CLI prerequisite boundary: report any failure as an unreachable database and abort cleanly + except Exception as exc: # noqa: BLE001 console.log(f"{ERROR_BADGE} Database is unreachable: {exc}") console.log( " Verify that the database is running and that the connection settings in your configuration file are correct." @@ -241,7 +242,8 @@ async def _upgrade_check(db: InfrahubDatabase, root_node_graph_version: int) -> console.log(" Schema has differences, update required") else: console.log(" Up to date, nothing to do") - except Exception as exc: + # Best-effort dry-run report: a failed schema probe is reported inline and the remaining checks still run + except Exception as exc: # noqa: BLE001 console.log(f" Unable to check: {exc}") console.log("\nBranches:") diff --git a/backend/infrahub/core/schema/update_coordinator.py b/backend/infrahub/core/schema/update_coordinator.py index 408e4364e5f..8e878ee5d03 100644 --- a/backend/infrahub/core/schema/update_coordinator.py +++ b/backend/infrahub/core/schema/update_coordinator.py @@ -347,7 +347,8 @@ async def _run_migrations_via_workflow( expected_return=list[str], parameters={"message": apply_migration_data}, ) - except Exception as exc: + # Any migration failure must be captured so the caller can roll back before re-raising it + except Exception as exc: # noqa: BLE001 exception = exc return error_msgs, exception @@ -362,7 +363,8 @@ async def _run_migrations_directly( try: error_msgs = await schema_apply_migrations(message=apply_migration_data) - except Exception as exc: + # Any migration failure must be captured so the caller can roll back before re-raising it + except Exception as exc: # noqa: BLE001 exception = exc return error_msgs, exception diff --git a/backend/infrahub/core/validators/tasks.py b/backend/infrahub/core/validators/tasks.py index d43ebab1b33..e16537f7e2b 100644 --- a/backend/infrahub/core/validators/tasks.py +++ b/backend/infrahub/core/validators/tasks.py @@ -82,7 +82,8 @@ async def schema_path_validate( ) try: violations = await aggregated_constraint_checker.run_constraints(constraint_request) - except Exception as exc: + # Degrade any checker failure into a reported violation so schema validation fails visibly instead of crashing the task + except Exception as exc: # noqa: BLE001 violation = SchemaViolation( node_id="unknown", node_kind=node_schema.kind, diff --git a/backend/infrahub/generators/tasks.py b/backend/infrahub/generators/tasks.py index c5a69090dbb..a919add0c73 100644 --- a/backend/infrahub/generators/tasks.py +++ b/backend/infrahub/generators/tasks.py @@ -250,5 +250,6 @@ async def request_generator_definition_run( try: await asyncio.gather(*tasks) return Completed(message=f"Successfully run {len(tasks)} generators") - except Exception as exc: + # Flow boundary: any generator failure must surface as a Failed state carrying the error, not a crashed flow run + except Exception as exc: # noqa: BLE001 return Failed(message="One or more generators failed", error=exc) diff --git a/backend/infrahub/git/integrator.py b/backend/infrahub/git/integrator.py index 659e5429bc6..ea6986772a5 100644 --- a/backend/infrahub/git/integrator.py +++ b/backend/infrahub/git/integrator.py @@ -380,7 +380,8 @@ async def apply_import_plan(self, plan: ObjectImportPlan) -> None: fingerprint_composer=fingerprint_composer, ) - except Exception as exc: + # Any import failure must stamp the repository sync status as errored before being re-raised + except Exception as exc: # noqa: BLE001 sync_status = RepositorySyncStatus.ERROR_IMPORT error = exc diff --git a/backend/infrahub/git/sync.py b/backend/infrahub/git/sync.py index 74dac462cc7..ec102eb4560 100644 --- a/backend/infrahub/git/sync.py +++ b/backend/infrahub/git/sync.py @@ -117,7 +117,7 @@ async def sync(self, repo: InfrahubRepository, staging_branch: str | None = None await self._importer.apply_branch_import(repo, plan) except (RepositoryConnectionError, RepositoryCredentialsError): raise - except Exception as exc: + except Exception as exc: # noqa: BLE001 # The import already records its own per-branch error status before re-raising, so # isolate the failure here to keep importing the remaining branches. failed_imports.append( diff --git a/backend/infrahub/message_bus/operations/__init__.py b/backend/infrahub/message_bus/operations/__init__.py index b9d16dd0c0b..d782d017ed1 100644 --- a/backend/infrahub/message_bus/operations/__init__.py +++ b/backend/infrahub/message_bus/operations/__init__.py @@ -31,7 +31,8 @@ async def execute_message( if skip_flow and isinstance(func, Flow): func = func.fn await func(message=message) - except Exception as exc: + # Message-bus boundary: any handler failure must be routed to the reply/retry/dead-letter protocol, never crash the consumer + except Exception as exc: # noqa: BLE001 if message.reply_requested: response = RPCErrorResponse(errors=[str(exc)], initial_message=message.model_dump()) await message_bus.reply_if_initiator_meta(message=response, initiator=message) diff --git a/backend/infrahub/services/scheduler.py b/backend/infrahub/services/scheduler.py index 0c84d2eca04..5a530aed32f 100644 --- a/backend/infrahub/services/scheduler.py +++ b/backend/infrahub/services/scheduler.py @@ -86,7 +86,8 @@ async def run_schedule(self, schedule: Schedule) -> None: while self.running: try: await schedule.function(self.service) - except Exception as exc: + # Keep-alive: a failing recurring task must not kill the scheduler loop + except Exception as exc: # noqa: BLE001 self.service.log.error(str(exc)) for _ in range(schedule.interval): if not self.running: diff --git a/backend/infrahub/task_manager/flow_run/retention.py b/backend/infrahub/task_manager/flow_run/retention.py index 6e96da45f83..59806b7cbd0 100644 --- a/backend/infrahub/task_manager/flow_run/retention.py +++ b/backend/infrahub/task_manager/flow_run/retention.py @@ -60,7 +60,8 @@ async def purge( ) purged_total += 1 batch_purged += 1 - except Exception as e: + # Best-effort retention: skip flow runs that fail to purge and keep processing the batch + except Exception as e: # noqa: BLE001 logger.warning(f"Failed to {action} flow run {flow_run.id}: {e}") failed_purges.append(flow_run.id) diff --git a/backend/infrahub/telemetry/tasks.py b/backend/infrahub/telemetry/tasks.py index 216af63f6c3..0afbfed3efa 100644 --- a/backend/infrahub/telemetry/tasks.py +++ b/backend/infrahub/telemetry/tasks.py @@ -126,7 +126,7 @@ async def send_telemetry_push() -> None: try: await repository.save(snapshot) log.info(f"Telemetry snapshot stored locally (uuid={snapshot.uuid}).") - except Exception as exc: + except Exception as exc: # noqa: BLE001 log.warning(f"Failed to store telemetry snapshot locally: {exc}") return @@ -149,12 +149,14 @@ async def send_telemetry_push() -> None: await post_telemetry_data(url=config.SETTINGS.main.telemetry_endpoint, payload=payload) snapshot.remote_send_status = RemoteSendStatus.SENT log.info("Telemetry data sent to remote endpoint successfully.") - except Exception as exc: + # Best-effort telemetry: any send failure is recorded as FAILED on the snapshot, never propagated + except Exception as exc: # noqa: BLE001 snapshot.remote_send_status = RemoteSendStatus.FAILED log.warning(f"Failed to send telemetry data to remote endpoint: {exc}") # Update remote send status in DB try: await repository.save(snapshot) - except Exception as exc: + # Best-effort telemetry: failing to persist the send status only warrants a warning + except Exception as exc: # noqa: BLE001 log.warning(f"Failed to update snapshot remote send status: {exc}") diff --git a/backend/infrahub/webhook/tasks/process.py b/backend/infrahub/webhook/tasks/process.py index 75c7c17b89f..1ab334258f7 100644 --- a/backend/infrahub/webhook/tasks/process.py +++ b/backend/infrahub/webhook/tasks/process.py @@ -87,7 +87,8 @@ async def _record_http_capture(capture: CapturedHttp) -> None: data=capture.to_artifact_data(), flow_run_id=UUID(flow_run.id), ) - except Exception as exc: + # Best-effort capture: an artifact write failure must never alter or mask the delivery outcome + except Exception as exc: # noqa: BLE001 get_run_logger().warning(f"Could not record the delivery capture: {exc}") diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 6e71b6f9766..2a999d3d43c 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -42,9 +42,9 @@ - [X] T002 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m014–m047 (13 sites): backend/infrahub/core/migrations/graph/m014_remove_index_attr_value.py:39, m029_duplicates_cleanup.py:656, m036_drop_attr_value_index.py:39, m043_create_hfid_display_label_in_db.py:116+168, m044_backfill_hfid_display_label_in_db.py:382+514, m045_backfill_hfid_display_label_in_db_profile_template.py:82+163, m046_fill_agnostic_hfid_display_labels.py:141+196, m047_backfill_or_null_display_label.py:416+465 — use each row's exact justification comment - [X] T003 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to migrations m059–m074 (14 sites): backend/infrahub/core/migrations/graph/m059_fix_hfid_display_label_nulls.py:238+247+381+420, m062_recompute_permission_display_labels.py:454+473, m063_template_number_pool_cleanup.py:82, m064_template_ip_pool_relationship_cleanup.py:98, m066_consolidate_duplicate_number_pools.py:82, m070_normalize_mac_address_values_to_colon.py:225, m071_recompute_hfid_for_ip_attributes.py:180, m072_index_hfid_values.py:169, m073_unify_ip_pool_resource_identifier.py:336, m074_normalize_indexed_hfid_values.py:156 — m066/m073 use the transaction-safe wording (no atomicity claims) - [X] T004 [P] [US3] Apply SUPPRESS per data-model.md Batch A rows to backend/infrahub/core/migrations/shared.py:157+245+277 (3 sites; :157/:245 use the per-query wording without atomicity claims) -- [ ] T005 [P] [US3] Apply SUPPRESS per data-model.md Batch B rows to the 8 auth sites: backend/infrahub/api/auth.py:63+116, backend/infrahub/api/oauth2.py:205, backend/infrahub/api/oidc.py:259, backend/infrahub/auth/auth.py:542+558+668+679 — annotation-only; fail-closed comments exactly as tabled -- [ ] T006 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 1, 9 sites / 8 files): backend/infrahub/artifacts/tasks.py:49, backend/infrahub/cli/upgrade.py:65+244, backend/infrahub/core/schema/update_coordinator.py:350+365, backend/infrahub/core/validators/tasks.py:85, backend/infrahub/generators/tasks.py:253, backend/infrahub/git/integrator.py:383, backend/infrahub/git/sync.py:120 (sync.py: keep the existing lines-121-122 comment, add noqa only) -- [ ] T007 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 2, 7 sites / 5 files): backend/infrahub/message_bus/operations/__init__.py:34, backend/infrahub/services/scheduler.py:89, backend/infrahub/task_manager/flow_run/retention.py:63, backend/infrahub/telemetry/tasks.py:129+152+159 (:129 has an existing intent comment ~line 125 — add noqa, extend comment only if it doesn't say why broad), backend/infrahub/webhook/tasks/process.py:90 +- [X] T005 [P] [US3] Apply SUPPRESS per data-model.md Batch B rows to the 8 auth sites: backend/infrahub/api/auth.py:63+116, backend/infrahub/api/oauth2.py:205, backend/infrahub/api/oidc.py:259, backend/infrahub/auth/auth.py:542+558+668+679 — annotation-only; fail-closed comments exactly as tabled +- [X] T006 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 1, 9 sites / 8 files): backend/infrahub/artifacts/tasks.py:49, backend/infrahub/cli/upgrade.py:65+244, backend/infrahub/core/schema/update_coordinator.py:350+365, backend/infrahub/core/validators/tasks.py:85, backend/infrahub/generators/tasks.py:253, backend/infrahub/git/integrator.py:383, backend/infrahub/git/sync.py:120 (sync.py: keep the existing lines-121-122 comment, add noqa only) +- [X] T007 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 2, 7 sites / 5 files): backend/infrahub/message_bus/operations/__init__.py:34, backend/infrahub/services/scheduler.py:89, backend/infrahub/task_manager/flow_run/retention.py:63, backend/infrahub/telemetry/tasks.py:129+152+159 (:129 has an existing intent comment ~line 125 — add noqa, extend comment only if it doesn't say why broad), backend/infrahub/webhook/tasks/process.py:90 - [ ] T008 [P] [US3] Apply SUPPRESS per data-model.md Batch D rows to the 7 backend-test suppress sites: backend/tests/helpers/diagnostics.py:103+179, backend/tests/helpers/events.py:51, backend/tests/helpers/test_worker.py:107 (**stays `except BaseException`** — use the ready-future justification comment verbatim), backend/tests/integration_docker/test_merge_kill_recovery.py:85 (keep existing lines-86-88 comment, add noqa + tabled comment), backend/tests/scale/common/protocols.py:28+53 - [ ] T009 [P] [US3] Apply SUPPRESS per data-model.md Batch E rows to the 9 tooling suppress sites: tests/e2e/data/parity.py:81 (keep/extend the existing trailing comment) and utilities/infrahub_load_tester.py:47+69+84+108+113+138+148+165 (do **not** fix the pre-existing missing-`return` at :69 — behavior preservation, see data-model.md Latent defects) - [ ] T010 [US3] Story checkpoint: run `uv run ruff check --select=BLE backend/infrahub backend/tests tests/e2e/data/parity.py utilities/infrahub_load_tester.py` — every remaining violation must be one of the 8 NARROW sites only; run `uv run ruff format --check` on all files touched by T002–T009 (clean); run `git diff -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` and verify every hunk is comment/noqa-only (spec SC-007) From 7ee067033acefe35b4eddb37763737b7bcf64eb2 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:18:24 +0000 Subject: [PATCH 09/14] [Spec Kit] Suppress BLE001 in test helpers + tooling; US3 checkpoint green (INBOX-19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch D (7 backend-test sites) and Batch E (9 tooling sites) receive the SUPPRESS treatment from data-model.md: a justification comment above each except plus a line-targeted noqa. Annotation-only — zero semantic tokens changed; test_worker.py keeps its BaseException catch and the load tester's latent missing-return stays untouched by design. US3 checkpoint (T010): remaining BLE001 violations are exactly the 8 NARROW sites; ruff format clean on all 42 touched files; migrations+auth diff vs base is mechanically verified comment/noqa-only (SC-007). Co-Authored-By: Claude Fable 5 --- backend/tests/helpers/diagnostics.py | 6 +++-- backend/tests/helpers/events.py | 3 ++- backend/tests/helpers/test_worker.py | 3 ++- .../test_merge_kill_recovery.py | 3 ++- backend/tests/scale/common/protocols.py | 6 +++-- dev/specs/002-ruff-ble-reenable/tasks.md | 6 ++--- tests/e2e/data/parity.py | 3 ++- utilities/infrahub_load_tester.py | 24 ++++++++++++------- 8 files changed, 35 insertions(+), 19 deletions(-) diff --git a/backend/tests/helpers/diagnostics.py b/backend/tests/helpers/diagnostics.py index 79f3e31867c..8bb21601412 100644 --- a/backend/tests/helpers/diagnostics.py +++ b/backend/tests/helpers/diagnostics.py @@ -100,7 +100,8 @@ def dump_event_loop_closed_diagnostic(nodeid: str, exc: BaseException) -> None: lines.extend(_format_stack_block(creation_stack, indent=" ")) else: lines.append(" redis pool: ") - except Exception as diag_exc: + # Best-effort post-mortem dump: must never raise while reporting the original error + except Exception as diag_exc: # noqa: BLE001 lines.append(f" (diagnostic dump failed: {diag_exc!r})") print("\n".join(lines), file=stderr, flush=True) @@ -176,7 +177,8 @@ async def _instrumented_connect(self: Connection) -> None: async def _instrumented_disconnect(self: ConnectionPool, inuse_connections: bool = True) -> None: try: _dump_pool_loop_divergence(self) - except Exception as diag_exc: + # Instrumentation must never break the real pool disconnect; log and continue + except Exception as diag_exc: # noqa: BLE001 print(f"(redis loop divergence dump failed: {diag_exc!r})", file=stderr, flush=True) await original_disconnect(self, inuse_connections=inuse_connections) diff --git a/backend/tests/helpers/events.py b/backend/tests/helpers/events.py index 64f04d54caf..34c6d66018a 100644 --- a/backend/tests/helpers/events.py +++ b/backend/tests/helpers/events.py @@ -48,7 +48,8 @@ async def has_event(client: PrefectClient, event_id: UUID) -> bool: try: await query_event(client=client, event_id=event_id) return True - except Exception: + # Polling probe: query_event signals absence with a bare Exception; any failure means "not available yet" + except Exception: # noqa: BLE001 return False diff --git a/backend/tests/helpers/test_worker.py b/backend/tests/helpers/test_worker.py index d519ed133f4..0e62a0da18f 100644 --- a/backend/tests/helpers/test_worker.py +++ b/backend/tests/helpers/test_worker.py @@ -104,7 +104,8 @@ async def lifecycle() -> None: try: await worker.setup(client=client, metric_port=0) await worker.sync_with_backend() - except BaseException as exc: + # Any failure (incl. CancelledError) must resolve the "ready" future, else the fixture awaiting it hangs forever + except BaseException as exc: # noqa: BLE001 ready.set_exception(exc) return ready.set_result(None) diff --git a/backend/tests/integration_docker/test_merge_kill_recovery.py b/backend/tests/integration_docker/test_merge_kill_recovery.py index 9513260ec50..8ad009b9954 100644 --- a/backend/tests/integration_docker/test_merge_kill_recovery.py +++ b/backend/tests/integration_docker/test_merge_kill_recovery.py @@ -82,7 +82,8 @@ async def _drive_branch_to_merge_failed( await merge_task except asyncio.CancelledError: pass - except Exception as exc: + # Teardown: the deliberately-killed mutation may raise any SDK/transport error; retrieve and log it without masking the test result + except Exception as exc: # noqa: BLE001 # The mutation legitimately errors once its worker is SIGKILLed mid-flight; retrieve the # exception so an unrelated early failure (validation, connectivity) surfaces in the # captured test output instead of being lost as an unretrieved-task warning. diff --git a/backend/tests/scale/common/protocols.py b/backend/tests/scale/common/protocols.py index b03a024a21c..99f2f4bd690 100644 --- a/backend/tests/scale/common/protocols.py +++ b/backend/tests/scale/common/protocols.py @@ -25,7 +25,8 @@ def execute_graphql(self, *args, **kwargs) -> Any: start_perf_counter = time.perf_counter() try: request_meta["response"] = super().execute_graphql(*args, **kwargs) - except Exception as e: + # Locust instrumentation: record every failure as a request event instead of crashing the user greenlet + except Exception as e: # noqa: BLE001 request_meta["exception"] = e request_meta["response_time"] = (time.perf_counter() - start_perf_counter) * 1000 self._request_event.fire(**request_meta) @@ -50,7 +51,8 @@ def _request(self, *args, **kwargs): start_perf_counter = time.perf_counter() try: request_meta["response"] = super()._request(*args, **kwargs) - except Exception as e: + # Locust instrumentation: record every failure as a request event instead of crashing the user greenlet + except Exception as e: # noqa: BLE001 request_meta["exception"] = e request_meta["response_time"] = (time.perf_counter() - start_perf_counter) * 1000 self._request_event.fire(**request_meta) diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 2a999d3d43c..9c3c82ae855 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -45,9 +45,9 @@ - [X] T005 [P] [US3] Apply SUPPRESS per data-model.md Batch B rows to the 8 auth sites: backend/infrahub/api/auth.py:63+116, backend/infrahub/api/oauth2.py:205, backend/infrahub/api/oidc.py:259, backend/infrahub/auth/auth.py:542+558+668+679 — annotation-only; fail-closed comments exactly as tabled - [X] T006 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 1, 9 sites / 8 files): backend/infrahub/artifacts/tasks.py:49, backend/infrahub/cli/upgrade.py:65+244, backend/infrahub/core/schema/update_coordinator.py:350+365, backend/infrahub/core/validators/tasks.py:85, backend/infrahub/generators/tasks.py:253, backend/infrahub/git/integrator.py:383, backend/infrahub/git/sync.py:120 (sync.py: keep the existing lines-121-122 comment, add noqa only) - [X] T007 [P] [US3] Apply SUPPRESS per data-model.md Batch C rows (part 2, 7 sites / 5 files): backend/infrahub/message_bus/operations/__init__.py:34, backend/infrahub/services/scheduler.py:89, backend/infrahub/task_manager/flow_run/retention.py:63, backend/infrahub/telemetry/tasks.py:129+152+159 (:129 has an existing intent comment ~line 125 — add noqa, extend comment only if it doesn't say why broad), backend/infrahub/webhook/tasks/process.py:90 -- [ ] T008 [P] [US3] Apply SUPPRESS per data-model.md Batch D rows to the 7 backend-test suppress sites: backend/tests/helpers/diagnostics.py:103+179, backend/tests/helpers/events.py:51, backend/tests/helpers/test_worker.py:107 (**stays `except BaseException`** — use the ready-future justification comment verbatim), backend/tests/integration_docker/test_merge_kill_recovery.py:85 (keep existing lines-86-88 comment, add noqa + tabled comment), backend/tests/scale/common/protocols.py:28+53 -- [ ] T009 [P] [US3] Apply SUPPRESS per data-model.md Batch E rows to the 9 tooling suppress sites: tests/e2e/data/parity.py:81 (keep/extend the existing trailing comment) and utilities/infrahub_load_tester.py:47+69+84+108+113+138+148+165 (do **not** fix the pre-existing missing-`return` at :69 — behavior preservation, see data-model.md Latent defects) -- [ ] T010 [US3] Story checkpoint: run `uv run ruff check --select=BLE backend/infrahub backend/tests tests/e2e/data/parity.py utilities/infrahub_load_tester.py` — every remaining violation must be one of the 8 NARROW sites only; run `uv run ruff format --check` on all files touched by T002–T009 (clean); run `git diff -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` and verify every hunk is comment/noqa-only (spec SC-007) +- [X] T008 [P] [US3] Apply SUPPRESS per data-model.md Batch D rows to the 7 backend-test suppress sites: backend/tests/helpers/diagnostics.py:103+179, backend/tests/helpers/events.py:51, backend/tests/helpers/test_worker.py:107 (**stays `except BaseException`** — use the ready-future justification comment verbatim), backend/tests/integration_docker/test_merge_kill_recovery.py:85 (keep existing lines-86-88 comment, add noqa + tabled comment), backend/tests/scale/common/protocols.py:28+53 +- [X] T009 [P] [US3] Apply SUPPRESS per data-model.md Batch E rows to the 9 tooling suppress sites: tests/e2e/data/parity.py:81 (keep/extend the existing trailing comment) and utilities/infrahub_load_tester.py:47+69+84+108+113+138+148+165 (do **not** fix the pre-existing missing-`return` at :69 — behavior preservation, see data-model.md Latent defects) +- [X] T010 [US3] Story checkpoint: run `uv run ruff check --select=BLE backend/infrahub backend/tests tests/e2e/data/parity.py utilities/infrahub_load_tester.py` — every remaining violation must be one of the 8 NARROW sites only; run `uv run ruff format --check` on all files touched by T002–T009 (clean); run `git diff -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` and verify every hunk is comment/noqa-only (spec SC-007) **Checkpoint**: All intentional broad catches are now auditable; only the 8 NARROW sites still flag. diff --git a/tests/e2e/data/parity.py b/tests/e2e/data/parity.py index 7899a457d78..038cc133e54 100644 --- a/tests/e2e/data/parity.py +++ b/tests/e2e/data/parity.py @@ -78,7 +78,8 @@ async def _safe[T](fn: Callable[[], Awaitable[T]]) -> T | str: try: return await fn() - except Exception as exc: # capture per-entry, never kill the whole dump + # Diagnostic dump: record any per-entry failure as a string, never kill the whole dump + except Exception as exc: # noqa: BLE001 return f"ERROR: {type(exc).__name__}: {exc}" diff --git a/utilities/infrahub_load_tester.py b/utilities/infrahub_load_tester.py index ad28e6ae17a..0bf3d3e510d 100644 --- a/utilities/infrahub_load_tester.py +++ b/utilities/infrahub_load_tester.py @@ -44,7 +44,8 @@ async def _create_one(idx: int, client: InfrahubClient, prefix: str, log: loggin ) await proposed_change.save() log.info(f"✅ Created proposed change for branch {branch_name}") - except Exception as e: + # Load test: absorb any request failure and continue + except Exception as e: # noqa: BLE001 log.error(f"❌ Error creating proposed change for branch {branch_name}: {e}") log.info(f"✅ User {uname} created with branch {branch_name}") @@ -66,7 +67,8 @@ async def _delete_branches(client: InfrahubClient, prefix: str, usernames: Itera """Delete branches `/ one by one.""" try: all_branches = await client.branch.all() - except Exception as e: + # Load test: absorb any request failure and continue + except Exception as e: # noqa: BLE001 log.error(f"Error retrieving branches: {e}") for uname in usernames: @@ -81,7 +83,8 @@ async def _delete_branches(client: InfrahubClient, prefix: str, usernames: Itera log.info(f"🗑️ Branch {br} deleted") else: log.warning(f"Branch {br} not found") - except Exception as exc: + # Load test: absorb any request failure and continue with remaining branches + except Exception as exc: # noqa: BLE001 log.error(f"Error deleting branch {br}: {exc}") @@ -105,12 +108,14 @@ async def _delete_users(client: InfrahubClient, usernames: Iterable[str], log: l await user.delete() log.info(f"🗑️ User {uname} Deleted") break - except Exception as e: + # Load test: absorb any request failure and continue with remaining users + except Exception as e: # noqa: BLE001 log.error(f"Error while deleting: {str(e)}") else: log.warning(f"User {uname} not found in the list") - except Exception as exc: + # Load test: absorb any request failure and continue with remaining users + except Exception as exc: # noqa: BLE001 log.error(f"❌ General: {exc}") @@ -135,7 +140,8 @@ async def create_admin_branches(client: InfrahubClient, log: logging.Logger, *, await tag.save() log.info(f"✅ Branch created: {branch_name} with test tag") - except Exception as exc: + # Load test: absorb any request failure and continue with remaining branches + except Exception as exc: # noqa: BLE001 log.error(f"❌ Error creating branch {branch_name}: {exc}") @@ -145,7 +151,8 @@ async def delete_admin_branches(client: InfrahubClient, log: logging.Logger, *, try: all_branches = await client.branch.all() - except Exception as e: + # Load test: abort branch cleanup gracefully on any request failure + except Exception as e: # noqa: BLE001 log.error(f"Error retrieving branches: {e}") return @@ -162,7 +169,8 @@ async def delete_admin_branches(client: InfrahubClient, log: logging.Logger, *, log.info(f"🗑️ Branch {branch_name} deleted") else: log.warning(f"Branch {branch_name} not found") - except Exception as exc: + # Load test: absorb any request failure and continue with remaining branches + except Exception as exc: # noqa: BLE001 log.error(f"Error deleting branch {branch_name}: {exc}") From 68ca27a21217085b2adc9131382cc597309965f1 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:30:18 +0000 Subject: [PATCH 10/14] [Spec Kit] Narrow 8 blind excepts in tests/tooling; US2 checkpoint green (INBOX-19) Narrow the 8 analyzable BLE001 handlers per the data-model treatment matrix (Batches D/E NARROW rows), handler bodies untouched: - schema_branch component tests: except Exception -> SchemaNotFoundError in the duplicated _describe_hash_diff helper (2 sites per file) - integration/git/conftest.py poll loops: except Exception -> httpx.HTTPError, dropping the now-stale noqa: S110 (typed excepts are S110-exempt) - tasks/release.py version probes: except Exception -> InvalidVersion, extending the deliberate function-local packaging imports US2 checkpoint: repo BLE001 = 0 (--exclude python_sdk, per the CI lint gate); ruff format clean; InvalidVersion probe and invoke --list green; the 12 touched component tests pass locally against the neo4j testcontainer (12 passed in 65.16s). Co-Authored-By: Claude Fable 5 --- .../core/schema/schema_branch/test_process_idempotency.py | 5 +++-- .../schema/schema_branch/test_uniqueness_propagation.py | 5 +++-- backend/tests/integration/git/conftest.py | 4 ++-- dev/specs/002-ruff-ble-reenable/tasks.md | 8 ++++---- tasks/release.py | 8 ++++---- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/backend/tests/component/core/schema/schema_branch/test_process_idempotency.py b/backend/tests/component/core/schema/schema_branch/test_process_idempotency.py index c28b067b5b3..886a04d53dd 100644 --- a/backend/tests/component/core/schema/schema_branch/test_process_idempotency.py +++ b/backend/tests/component/core/schema/schema_branch/test_process_idempotency.py @@ -7,6 +7,7 @@ from infrahub.core.schema.relationship_schema import RelationshipSchema from infrahub.core.schema.schema_branch import SchemaBranch from infrahub.database import InfrahubDatabase +from infrahub.exceptions import SchemaNotFoundError LOCATION_GENERIC = GenericSchema( name="Location", @@ -155,13 +156,13 @@ def _describe_hash_diff(before: SchemaBranch, after: SchemaBranch) -> str: try: obj_before = before.get(name=name, duplicate=False) dump_before = obj_before.model_dump() - except Exception: + except SchemaNotFoundError: lines.append(f"{name}: only in 'after'") continue try: obj_after = after.get(name=name, duplicate=False) dump_after = obj_after.model_dump() - except Exception: + except SchemaNotFoundError: lines.append(f"{name}: only in 'before'") continue if obj_before.get_hash() == obj_after.get_hash(): diff --git a/backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py b/backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py index f8b29d04b9f..4e3fe9ded9b 100644 --- a/backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py +++ b/backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py @@ -11,6 +11,7 @@ from infrahub.core.schema.generic_schema import GenericSchema from infrahub.core.schema.node_schema import NodeSchema from infrahub.core.schema.relationship_schema import RelationshipSchema +from infrahub.exceptions import SchemaNotFoundError if TYPE_CHECKING: from infrahub.core.schema.schema_branch import SchemaBranch @@ -39,13 +40,13 @@ def _describe_hash_diff(before: SchemaBranch, after: SchemaBranch) -> str: try: obj_before = before.get(name=name, duplicate=False) dump_before = obj_before.model_dump() - except Exception: + except SchemaNotFoundError: lines.append(f"{name}: only in 'after'") continue try: obj_after = after.get(name=name, duplicate=False) dump_after = obj_after.model_dump() - except Exception: + except SchemaNotFoundError: lines.append(f"{name}: only in 'before'") continue if obj_before.get_hash() == obj_after.get_hash(): diff --git a/backend/tests/integration/git/conftest.py b/backend/tests/integration/git/conftest.py index fc0dee7f9f6..e762b19a59a 100644 --- a/backend/tests/integration/git/conftest.py +++ b/backend/tests/integration/git/conftest.py @@ -28,7 +28,7 @@ def _wait_for_http(url: str, timeout: int = 30) -> None: resp = httpx.get(url, timeout=1.0, follow_redirects=True) if resp.status_code < 500: return - except Exception: # noqa: S110 + except httpx.HTTPError: pass time.sleep(0.5) pytest.fail(f"HTTP endpoint {url} did not become available within {timeout}s") @@ -50,7 +50,7 @@ def _create_api_token(base_url: str) -> str: last_location = resp.headers.get("location", "") if resp.status_code == 201: return resp.json()["sha1"] - except Exception: # noqa: S110 + except httpx.HTTPError: pass time.sleep(0.5) pytest.fail(f"Failed to create Gogs API token within 30s (last status={last_status}, location={last_location!r})") diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 9c3c82ae855..16c9b67ed8c 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -61,10 +61,10 @@ ### Implementation for User Story 2 -- [ ] T011 [P] [US2] Narrow the duplicated `_describe_hash_diff` helper per data-model.md Batch D: replace `except Exception` with `except SchemaNotFoundError` at backend/tests/component/core/schema/schema_branch/test_process_idempotency.py:158+164 and backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py:42+48, adding `from infrahub.exceptions import SchemaNotFoundError` to each file's imports -- [ ] T012 [P] [US2] Narrow the two poll loops in backend/tests/integration/git/conftest.py:31+53 per data-model.md Batch D: `except Exception` → `except httpx.HTTPError` (httpx already imported) and **remove the now-stale `# noqa: S110` on those lines** (typed excepts are S110-exempt; RUF100 fails on unused noqa) -- [ ] T013 [P] [US2] Narrow the two version-probe handlers in tasks/release.py:155+242 per data-model.md Batch E: `except Exception` → `except InvalidVersion`, extending the **function-local** imports (~line 115 and ~line 213) to `from packaging.version import InvalidVersion, Version` — do not hoist to module level (locals are deliberate so invoke runs without dev deps) -- [ ] T014 [US2] Story checkpoint: `uv run ruff check --select=BLE .` from repo root reports **0** (all 78 resolved); `uv run ruff format --check` clean on the 6 narrowed files; sanity checks `uv run python -c "from packaging.version import Version; Version('1.2.3-foo')"` (expect InvalidVersion raised) and `uv run invoke --list > /dev/null` (imports OK); run `uv run pytest backend/tests/component/core/schema/schema_branch/test_process_idempotency.py backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py` if the local environment supports testcontainers — otherwise record "deferred to CI" with the reason (critique E4: conftest narrowings are CI-verified by design; documented fallback = SUPPRESS per data-model.md) +- [X] T011 [P] [US2] Narrow the duplicated `_describe_hash_diff` helper per data-model.md Batch D: replace `except Exception` with `except SchemaNotFoundError` at backend/tests/component/core/schema/schema_branch/test_process_idempotency.py:158+164 and backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py:42+48, adding `from infrahub.exceptions import SchemaNotFoundError` to each file's imports +- [X] T012 [P] [US2] Narrow the two poll loops in backend/tests/integration/git/conftest.py:31+53 per data-model.md Batch D: `except Exception` → `except httpx.HTTPError` (httpx already imported) and **remove the now-stale `# noqa: S110` on those lines** (typed excepts are S110-exempt; RUF100 fails on unused noqa) +- [X] T013 [P] [US2] Narrow the two version-probe handlers in tasks/release.py:155+242 per data-model.md Batch E: `except Exception` → `except InvalidVersion`, extending the **function-local** imports (~line 115 and ~line 213) to `from packaging.version import InvalidVersion, Version` — do not hoist to module level (locals are deliberate so invoke runs without dev deps) +- [X] T014 [US2] Story checkpoint: `uv run ruff check --select=BLE .` from repo root reports **0** (all 78 resolved); `uv run ruff format --check` clean on the 6 narrowed files; sanity checks `uv run python -c "from packaging.version import Version; Version('1.2.3-foo')"` (expect InvalidVersion raised) and `uv run invoke --list > /dev/null` (imports OK); run `uv run pytest backend/tests/component/core/schema/schema_branch/test_process_idempotency.py backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py` if the local environment supports testcontainers — otherwise record "deferred to CI" with the reason (critique E4: conftest narrowings are CI-verified by design; documented fallback = SUPPRESS per data-model.md) **Checkpoint**: Zero BLE001 violations repo-wide; rule can now be activated. diff --git a/tasks/release.py b/tasks/release.py index 31214f812dd..61a3306bb89 100644 --- a/tasks/release.py +++ b/tasks/release.py @@ -112,7 +112,7 @@ def update_helm_chart(context: Context, chart_repo: str | None = "helm/", versio print(" - [release] Update Helm chart") # Import here to not require installing packaging when running invoke without installing dependencies. - from packaging.version import Version + from packaging.version import InvalidVersion, Version # Explicit version (target release) wins over the installed package metadata # (which is resolved from the git tag at build time) @@ -152,7 +152,7 @@ def update_helm_chart(context: Context, chart_repo: str | None = "helm/", versio new_helm_version = Version( f"{new_helm_version.major}.{new_helm_version.minor}.{new_helm_version.micro + 1}" ) - except Exception: + except InvalidVersion: # Fallback in case app_version has non-standard format for Helm comparison print(f"Warning: Unable to strictly compare versions, using default Helm chart version: {new_helm_version}") @@ -210,7 +210,7 @@ def update_docker_compose( print(" - [release] Update docker-compose.yml") # Import here to not require installing packaging when running invoke without installing dependencies. - from packaging.version import Version + from packaging.version import InvalidVersion, Version # Explicit version (target release) wins over the installed package metadata # (which is resolved from the git tag at build time) @@ -239,7 +239,7 @@ def update_docker_compose( # rewrites the pinned image tags downward. try: should_update = not new_version.is_prerelease and new_version > Version(old_version) - except Exception: + except InvalidVersion: should_update = False if should_update: # Replace old version with the new version in the image field From f648739a71255a7bff5e89be4d758c1f2e20576a Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:37:49 +0000 Subject: [PATCH 11/14] =?UTF-8?q?[Spec=20Kit]=20Enable=20BLE=20rule=20?= =?UTF-8?q?=E2=80=94=20remove=20ignore,=20add=20changelog;=20all=20gates?= =?UTF-8?q?=20green=20(INBOX-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 (US1), tasks T015-T018: - T015: remove "BLE" from the [tool.ruff.lint] ignore list in pyproject.toml - T016: towncrier housekeeping fragment for the BLE enforcement - T017: full gates green — ruff --select=BLE (0, python_sdk excluded: initialized submodule is out of scope), full ruff check, ruff format --check, invoke backend.lint (ruff + ty + mypy) all exit 0 - T018: canary mutation check — 1 x BLE001 reported on a planted blind except in tasks/utils.py, canary reverted, tree clean Co-Authored-By: Claude Fable 5 --- changelog/+ruff-ble-blind-except.housekeeping.md | 1 + dev/specs/002-ruff-ble-reenable/tasks.md | 8 ++++---- pyproject.toml | 1 - 3 files changed, 5 insertions(+), 5 deletions(-) create mode 100644 changelog/+ruff-ble-blind-except.housekeeping.md diff --git a/changelog/+ruff-ble-blind-except.housekeeping.md b/changelog/+ruff-ble-blind-except.housekeeping.md new file mode 100644 index 00000000000..2bb031edf51 --- /dev/null +++ b/changelog/+ruff-ble-blind-except.housekeeping.md @@ -0,0 +1 @@ +The BLE (flake8-blind-except) ruff rule is now enforced — blind `except Exception` handlers are either narrowed to the specific exception types they guard against or carry an explicit justified `# noqa: BLE001`. diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 16c9b67ed8c..57d0169008d 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -78,10 +78,10 @@ ### Implementation for User Story 1 -- [ ] T015 [US1] Remove the `"BLE", # flake8-blind-except (BLE)` line from the `[tool.ruff.lint]` `ignore` list in pyproject.toml (~line 511) — depends on T010 + T014 (all sites resolved) -- [ ] T016 [P] [US1] Add towncrier fragment changelog/+ruff-ble-blind-except.housekeeping.md: one sentence stating the BLE (flake8-blind-except) ruff rule is now enforced — blind `except Exception` handlers are either narrowed or carry an explicit justified `# noqa: BLE001` -- [ ] T017 [US1] Full-gate verification (quickstart.md §1–2): `uv run ruff check --select=BLE .` → 0; `uv run ruff check . --exclude python_sdk` → exit 0; `uv run ruff format --check --diff --exclude python_sdk .` → exit 0; `uv run invoke backend.lint` → exit 0 (ruff + ty + mypy) -- [ ] T018 [US1] Enforcement mutation check (quickstart.md §3, spec SC-006): append the canary `except Exception: pass` function to tasks/utils.py, verify `uv run ruff check --select=BLE tasks/utils.py` reports exactly 1 × BLE001, then `git checkout -- tasks/utils.py` and verify the tree is clean +- [X] T015 [US1] Remove the `"BLE", # flake8-blind-except (BLE)` line from the `[tool.ruff.lint]` `ignore` list in pyproject.toml (~line 511) — depends on T010 + T014 (all sites resolved) +- [X] T016 [P] [US1] Add towncrier fragment changelog/+ruff-ble-blind-except.housekeeping.md: one sentence stating the BLE (flake8-blind-except) ruff rule is now enforced — blind `except Exception` handlers are either narrowed or carry an explicit justified `# noqa: BLE001` +- [X] T017 [US1] Full-gate verification (quickstart.md §1–2): `uv run ruff check --select=BLE .` → 0; `uv run ruff check . --exclude python_sdk` → exit 0; `uv run ruff format --check --diff --exclude python_sdk .` → exit 0; `uv run invoke backend.lint` → exit 0 (ruff + ty + mypy) — note: the `--select=BLE` gate was run as `uv run ruff check --select=BLE . --exclude python_sdk` because the python_sdk submodule is checked out locally (separate repo, excluded from this repo's lint gates; contains 4 out-of-scope BLE001 findings) +- [X] T018 [US1] Enforcement mutation check (quickstart.md §3, spec SC-006): append the canary `except Exception: pass` function to tasks/utils.py, verify `uv run ruff check --select=BLE tasks/utils.py` reports exactly 1 × BLE001, then `git checkout -- tasks/utils.py` and verify the tree is clean **Checkpoint**: Enforcement live; all card acceptance criteria met except final audits. diff --git a/pyproject.toml b/pyproject.toml index 44616aa3b07..904477c84ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -508,7 +508,6 @@ ignore = [ "FBT", # flake8-boolean-trap "G", # flake8-logging-format "RSE", # flake8-raise - "BLE", # flake8-blind-except (BLE) ################################################################################################## # Documentation rules - resolve in smaller batches after deciding which of these we want to have # From 06fd34d83ee89873db82009040c921da3d07bcd8 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 08:57:04 +0000 Subject: [PATCH 12/14] [Spec Kit] Polish: suppression audit, unit suite, SC-007 diff audit, quickstart replay (INBOX-19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T019: 70/70 noqa: BLE001 suppressions counted (= data-model.md SUPPRESS total); 68 carry the justification on/above the except line, 2 use the tabled alternate positions (git/sync.py:120 below-line, telemetry/tasks.py:129 try-block header); E722 backstop clean. T020: backend unit suite green — 1846 passed, 17 warnings in 33.63s. T021: constraint-area diff from feature fork point 9247c6763 is annotation-only — 23 files, +76/-38; every addition is a comment or a byte-matched noqa'd except line (0 violations). Note: the stable merge-base 01a1ab5de predates the fork and includes unrelated upstream develop work; audit uses the fork parent. T022: full quickstart replay green (BLE=0, CI gates, backend.lint, canary mutation check, suppression audit, InvalidVersion probe); component tests cited from the prior local run (12 passed). Co-Authored-By: Claude Fable 5 --- dev/specs/002-ruff-ble-reenable/tasks.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/specs/002-ruff-ble-reenable/tasks.md b/dev/specs/002-ruff-ble-reenable/tasks.md index 57d0169008d..b92b2c24d78 100644 --- a/dev/specs/002-ruff-ble-reenable/tasks.md +++ b/dev/specs/002-ruff-ble-reenable/tasks.md @@ -91,10 +91,10 @@ **Purpose**: Auditability proofs and existing-suite regression evidence (spec SC-003/004/005). -- [ ] T019 [P] Suppression audit (quickstart.md §4): `grep -rn "noqa: BLE001" --include="*.py" . --exclude-dir=python_sdk --exclude-dir=.venv` — count must equal the SUPPRESS total from data-model.md (70, plus any T001 additions); each hit sits on an `except Exception`/`except BaseException` line with a justification comment on or immediately above it; `uv run ruff check --select=E722 .` → 0 bare excepts -- [ ] T020 [P] Run `uv run invoke backend.test-unit` — must pass with unchanged results (spec SC-005); if any failure, it must be traceable to something other than this change (compare against base) before proceeding -- [ ] T021 Re-verify hard-constraint diffs end-state (spec SC-007): `git diff ..HEAD -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` contains only comment/`noqa` additions; record the diff summary in the implementation report -- [ ] T022 Run the complete quickstart.md top-to-bottom as a final pass and record each command's outcome (this is the evidence table for the implementation report; include the T014 component-test outcome or its CI-deferral note) +- [X] T019 [P] Suppression audit (quickstart.md §4): `grep -rn "noqa: BLE001" --include="*.py" . --exclude-dir=python_sdk --exclude-dir=.venv` — count must equal the SUPPRESS total from data-model.md (70, plus any T001 additions); each hit sits on an `except Exception`/`except BaseException` line with a justification comment on or immediately above it; `uv run ruff check --select=E722 .` → 0 bare excepts +- [X] T020 [P] Run `uv run invoke backend.test-unit` — must pass with unchanged results (spec SC-005); if any failure, it must be traceable to something other than this change (compare against base) before proceeding +- [X] T021 Re-verify hard-constraint diffs end-state (spec SC-007): `git diff ..HEAD -- backend/infrahub/core/migrations/ backend/infrahub/api/auth.py backend/infrahub/api/oauth2.py backend/infrahub/api/oidc.py backend/infrahub/auth/` contains only comment/`noqa` additions; record the diff summary in the implementation report +- [X] T022 Run the complete quickstart.md top-to-bottom as a final pass and record each command's outcome (this is the evidence table for the implementation report; include the T014 component-test outcome or its CI-deferral note) --- From f32b310c5c0bccc375efa374ce10169b98cd0db1 Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 09:36:20 +0000 Subject: [PATCH 13/14] =?UTF-8?q?[Spec=20Kit]=20Add=20final=20implementati?= =?UTF-8?q?on=20report=20=E2=80=94=20all=20gates=20re-verified=20green=20(?= =?UTF-8?q?INBOX-19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verification + review tail for the ruff BLE re-enablement feature. All 22 tasks were already [X] on entry (completed by two prior, killed sessions); this run independently re-verified every gate rather than trusting the inherited state, then ran the review phase and wrote the final report. Re-confirmed fresh: BLE=0 repo-wide (--exclude python_sdk), full invoke backend.lint (ruff+ty+mypy), full ruff check/format --exclude python_sdk, E722=0, 70/70 noqa: BLE001 suppressions, SC-007 diff audit (migrations/auth annotation-only), SC-006 enforcement canary (1 x BLE001, reverted clean), tasks/release.py InvalidVersion + invoke --list sanity, full backend unit suite (1846 passed), and the two previously-flagged component tests (12 passed, 44.33s). Root-caused the "pytest component-test collection issue" flagged by prior runs: it was a false negative from the rtk CLI proxy's output summarizer ("Pytest: No tests collected" on a fully passing run), not a real collection defect — reproduced and confirmed via the raw (unfiltered) command path. No code change needed. Ran the error-handling and simplification review agents in parallel across the full feature diff (9247c676..06fd34d83). No CRITICAL/HIGH findings; 2 LOW/MEDIUM findings recorded (one is a pre-existing, deliberately accepted planning tradeoff already documented in data-model.md/ quickstart.md, not a new defect) plus 3 LOW simplify nits — none block merge, all deferred per the report. Co-Authored-By: Claude Sonnet 5 --- .../opsmill-implement-report.md | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 dev/specs/002-ruff-ble-reenable/opsmill-implement-report.md diff --git a/dev/specs/002-ruff-ble-reenable/opsmill-implement-report.md b/dev/specs/002-ruff-ble-reenable/opsmill-implement-report.md new file mode 100644 index 00000000000..7b148a8eaa5 --- /dev/null +++ b/dev/specs/002-ruff-ble-reenable/opsmill-implement-report.md @@ -0,0 +1,121 @@ +# Implementation Report: Re-enable ruff BLE (blind-except) rule + +**Spec dir**: `specs/002-ruff-ble-reenable` (`dev/specs/002-ruff-ble-reenable`) +**Base commit**: `9247c6763037f8158e0f192fc5f8cb69581a2118` (merge-base with `origin/develop`) +**Head commit**: `06fd34d83ee89873db82009040c921da3d07bcd8` +**Prep + implement wall-clock**: 2026-07-22 07:25:11 UTC → 08:57:04 UTC (~1h32m, across prep + all 6 implementation phases) +**This run (verification + review + report)**: 2026-07-22 ~09:00 UTC → 09:34 UTC + +**Context for this run**: this is the third `speckit.opsmill.implement` invocation for this feature. Two prior runs (each a separate session) completed the entire implementation loop — all 22 tasks in `tasks.md` were already `[X]` and committed on entry (last commit `06fd34d83`, "Polish: suppression audit, unit suite, SC-007 diff audit, quickstart replay"), and independent confirmation that BLE is removed from `pyproject.toml`'s ignore list and `ruff check --select=BLE .` is clean. Both prior runs were killed mid-review-phase before writing a final report. This run's job was: verify the inherited state fresh (not just trust it), re-run every verification gate independently, investigate a reported "pytest component-test collection issue" from scratch, execute the review phase, and produce this report. No new implementation chunks were dispatched — Phase 5 was already complete. + +--- + +## 1. Chunk-by-chunk ledger + +All chunks below were implemented and committed by prior (killed) sessions. I did not re-dispatch them; I independently re-verified their end state (gates, diffs, tests) rather than trusting the inherited commits at face value. Per-chunk "decisions flagged upward" are reconstructed from commit messages/diffs since the original subagents' live reports were lost when those sessions were killed. + +| # | Chunk (tasks.md phase) | Tasks | Outcome | Commit(s) | Notes | +|---|------------------------|-------|---------|-----------|-------| +| 1 | Phase 1: Setup | T001 | ✅ 1/1 | `e24482113` | Re-measured inventory: 78/78 sites reconciled against data-model.md, no drift, no new sites. | +| 2 | Phase 3: US3 — SUPPRESS batches (part 1) | T002–T004 | ✅ 3/3 | `0c1a9bdaa` | Migrations Batch A (30 sites across m014–m074 + shared.py) — annotation-only. | +| 3 | Phase 3: US3 — SUPPRESS batches (part 2) | T005–T007 | ✅ 3/3 | `5b438fe6e` | Auth Batch B (8 sites) + runtime/infra Batch C (16 sites) — annotation-only. | +| 4 | Phase 3: US3 — SUPPRESS batches (part 3) + checkpoint | T008–T010 | ✅ 3/3 | `7ee067033` | Test-helper Batch D (7 sites) + tooling Batch E (9 sites) + US3 story checkpoint green. | +| 5 | Phase 4: US2 — NARROW batches + checkpoint | T011–T014 | ✅ 4/4 | `68ca27a21` | 8 sites narrowed (schema-branch test helper ×4, git conftest poll loops ×2, release.py version probes ×2); US2 checkpoint: 0 BLE violations repo-wide. | +| 6 | Phase 5: US1 — enforcement flip | T015–T018 | ✅ 4/4 | `f648739a7` | `"BLE"` removed from `pyproject.toml` ignore list; changelog fragment added; full-gate + canary-mutation checks green. | +| 7 | Phase 6: Polish | T019–T022 | ✅ 4/4 | `06fd34d83` | Suppression audit, unit suite, SC-007 diff audit, quickstart replay recorded. | + +**Totals**: 22/22 tasks ✅, 0 ⚠️, 0 ❌. + +No fixup commits were needed — all `tasks.md` checkboxes were already `[X]` and matched the actual diff on inspection. + +--- + +## 2. Tasks not completed + +None. All 22 tasks (`T001`–`T022`) are `[X]` in `tasks.md` and verified against the actual repo state. + +--- + +## 3. Verification gates re-run this session (fresh evidence, not inherited) + +All commands below were re-run independently in this session (not assumed from prior commits): + +| Gate | Command | Result | +|------|---------|--------| +| BLE violations (repo, excl. submodule) | `ruff check --select=BLE . --exclude python_sdk` | 0 violations, exit 0 | +| BLE violations (bare, incl. submodule) | `ruff check --select=BLE .` | 4 findings — all in `python_sdk/infrahub_sdk/ctl/utils.py` (separate repo, out of scope; matches the discrepancy already documented in the `f648739a7` commit) | +| BLE removed from ignore list | `grep -n '"BLE"' pyproject.toml` | no match | +| Full backend lint | `invoke backend.lint` (ruff + ty + mypy) | all green, 1502 source files, 0 mypy issues | +| Full repo ruff check | `ruff check . --exclude python_sdk` | exit 0 | +| Full repo ruff format check | `ruff format --check --diff --exclude python_sdk .` | exit 0 | +| Bare-except backstop | `ruff check --select=E722 .` | 0 violations | +| Suppression audit | `grep -rn "noqa: BLE001" --include="*.py" . --exclude-dir=python_sdk --exclude-dir=.venv \| wc -l` | 70 (matches data-model.md SUPPRESS total exactly) | +| SC-007 diff audit | `git diff ..HEAD -- migrations/ auth.py oauth2.py oidc.py auth/` + line-level classification of every `+`/`-` line | every added line is a comment or a `# noqa: BLE001`-appended except line; every removed line is the pre-image of one of those same except lines — confirmed annotation-only | +| Enforcement canary (SC-006) | planted `except Exception: pass` in `tasks/utils.py`, ran `ruff check --select=BLE tasks/utils.py`, reverted | exactly 1×BLE001 reported; `git status` clean after revert | +| tasks/release.py narrow sanity | `python -c "from packaging.version import Version; Version('1.2.3-foo')"` | raises `InvalidVersion` as expected | +| tasks/release.py import sanity | `invoke --list` | exit 0, imports OK | +| Full backend unit suite (regression, SC-005) | `invoke backend.test-unit` | **1846 passed**, 32.19s | +| Two previously-flagged component tests | see §4 | **12 passed**, 44.33s | + +### On the "pytest component-test collection issue" + +Investigated fresh, per instructions, without assuming the prior runs' diagnosis. Root cause: **not a real pytest or code problem.** This repo's shell hook rewrites bare `pytest`/`uv run pytest` invocations through an `rtk` (Rust Token Killer) CLI proxy that summarizes pytest output to save tokens. In this session, that summarizer intermittently printed `Pytest: No tests collected` for these two files even when the underlying run fully succeeded. Bypassing the summarizer with `rtk proxy uv run pytest ...` (raw, unfiltered) on the identical command showed the true result every time: 12 items collected, 12 passed, exit 0 — reproduced 3 times (once via `--collect-only`, once via the hook-wrapped path directly, once via `rtk proxy` running the tests for real). A prior session was apparently misled by the summarizer's false-negative text into believing collection was broken. No code change was needed or made. + +### Deferred by design (pre-existing decision, not a gap introduced by this run) + +The two `except httpx.HTTPError:` narrowings in `backend/tests/integration/git/conftest.py` (T012, lines 31 and 53) back 5 integration test files (`test_auth_and_access.py`, `test_git_repository.py`, `test_delete_git_branch_gogs.py`, `test_readonly_repository.py`, `test_git_live_remote.py`) that require a live Gogs container over HTTP. `quickstart.md` §"CI-only verification (accepted)" and `data-model.md` Batch D already document this as intentionally deferred to CI's integration tier, with an explicit fallback (revert those 2 sites to SUPPRESS) if CI shows a gap. I did not attempt to stand up Gogs locally to force this — that was a considered planning-phase decision, not something left open by accident, and re-deciding it under this run would be scope creep. See §6. + +--- + +## 4. Local-pass evidence + +No new tests were added (per spec FR-008 — "no new tests are written; verification is lint-gate + existing-suite based"). Two existing test files had their exception-handling narrowed (T011); one fixture file used by 5 integration tests was narrowed (T012). Evidence for each: + +| Test id | Type | Run command | Passed at (UTC) | Environment context | Verbatim pass line | +|---------|------|--------------|------------------|----------------------|---------------------| +| `backend/tests/component/core/schema/schema_branch/test_process_idempotency.py::test_process_idempotency` | component | `uv run pytest backend/tests/component/core/schema/schema_branch/test_process_idempotency.py backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py` | 2026-07-22T09:17:15Z | Neo4j 2026.05.0-enterprise via testcontainers (image cached locally), Python 3.14.4, pytest-9.0.3 | `PASSED [ 8%]` | +| `backend/tests/component/core/schema/schema_branch/test_process_idempotency.py::test_process_idempotency_after_db_roundtrip` | component | (same command) | 2026-07-22T09:17:15Z | (same) | `PASSED [ 16%]` | +| `backend/tests/component/core/schema/schema_branch/test_uniqueness_propagation.py::TestSchemaProcessUniquenessIdempotent` (10 test methods) | component | (same command) | 2026-07-22T09:17:15Z | (same) | `PASSED` ×10, full session: `12 passed, 16 warnings in 44.33s` | +| `backend/tests/integration/git/conftest.py` poll-loop narrowings (back `test_auth_and_access.py`, `test_git_repository.py`, `test_delete_git_branch_gogs.py`, `test_readonly_repository.py`, `test_git_live_remote.py`) | integration | `uv run pytest backend/tests/integration/git` (CI integration tier) | deferred — local integration run requires a live Gogs container over HTTP; pre-existing planning decision (quickstart.md "CI-only verification (accepted)", data-model.md Batch D) to verify this tier in CI only, with a documented SUPPRESS fallback if CI disagrees | n/a locally | n/a | +| Full backend unit suite (regression check — no test files in this suite were modified by this feature; run as SC-005 evidence that the SUPPRESS/NARROW edits caused no regressions) | unit | `uv run invoke backend.test-unit` | 2026-07-22T09:2{0-1}:00Z (immediately following the component-test run) | Python 3.14.4, pytest-9.0.3, no DB required | `1846 passed, 17 warnings in 32.19s` | + +No `MISSING` rows. The one `deferred` row is a pre-existing, documented project decision (see §6), not an omission from this run. + +--- + +## 5. Review findings + +Two review agents ran in parallel across the full feature diff (`9247c676..06fd34d83`, 46 implementation files): **error-handling** (`speckit-review-errors`) and **simplification** (`speckit-review-simplify`). Both were instructed on the feature's design intent (SUPPRESS = annotation-only, NARROW = 8 specific sites) and given `data-model.md` as ground truth, so they could distinguish "diff doesn't match its own claim" from "pre-existing/accepted behavior working as designed." + +No CRITICAL or HIGH findings from either agent — nothing met the bar for an inline fix. + +| Severity | File:Line | Summary | Agent | Disposition | +|----------|-----------|---------|-------|-------------| +| MEDIUM | `backend/tests/integration/git/conftest.py:53` | Narrowed `except httpx.HTTPError:` doesn't cover `json.JSONDecodeError`/`KeyError` from `resp.json()["sha1"]` on a malformed 201 body — could turn a transient Gogs-startup race into a hard failure instead of a retry | error-handling | **Deferred, not fixed.** Already documented and explicitly accepted in `data-model.md` Batch D and `quickstart.md` ("medium confidence — fallback is SUPPRESS") during planning, with a stated fallback. Re-opening an already-critiqued planning tradeoff during implement-review would be scope creep; recorded here for visibility per instructions, matches CI-deferral in §3/§6. | +| LOW | `test_process_idempotency.py:159,165`, `test_uniqueness_propagation.py:43,49` | `except SchemaNotFoundError:` doesn't also catch the `ValueError` that `SchemaBranch.get()` can raise when a cache entry exists but its hash is missing | error-handling | **Deferred.** Verified impact is negligible: `_describe_hash_diff(...)` is only evaluated inside an already-failed `assert`'s message expression, so worst case is a less-helpful traceback on a test that was already red — not a masked defect or false pass. | +| LOW | `m059_fix_hfid_display_label_nulls.py:238,248` | Two adjacent `except` blocks (9 lines apart) share byte-identical justification comment text despite guarding different computations (`display_label` vs `human_friendly_id`), making the two `grep` hits indistinguishable | simplify | **Deferred** (advisory-only per skill scope; no behavior impact). | +| LOW | 4 files: `message_bus/operations/__init__.py:34`, `core/validators/tasks.py:85`, `tests/helpers/test_worker.py:107`, `tests/integration_docker/test_merge_kill_recovery.py:85` | New justification comments run 124–139 chars, wider than the files' normal ≤120-char wrap (still under the 150-char E501 ceiling, so lint-clean) | simplify | **Deferred** (cosmetic only). | +| LOW | `tests/integration_docker/test_merge_kill_recovery.py:85-89` | New one-line justification comment overlaps in content with the pre-existing 3-line body comment above the same `except` | simplify | **Deferred** (advisory; T008 explicitly instructed keeping the existing comment verbatim and only adding the new one). | +| info | `test_process_idempotency.py` / `test_uniqueness_propagation.py` — duplicated `_describe_hash_diff` helper | Pre-existing duplication (confirmed via `git show` against pre-image commits, predates this feature by several commits); this diff only touched the narrowed `except` line in both copies | simplify | Out of scope for this feature — noted for awareness only, not a finding against this diff. | + +**Verdicts from the two agents**: error-handling — "the diff delivers truthful, zero-behavior-change suppressions for all 70 sites and safe narrowings for 6 of 8; the remaining 2 NARROW sites... narrow to a real but demonstrably incomplete exception type — genuine low/medium-severity gaps confined to test code, not a broken delivery of the feature's core claim." Simplify — "clean, mechanical, and highly consistent — the only nits are two cosmetic wording/wrapping items and one pre-existing (untouched) duplication noted for awareness." + +No fixes were applied as a result of this review (nothing cleared the high-severity bar); HEAD remains `06fd34d83`, no new commit was needed for this phase. + +--- + +## 6. Autonomous decisions + +- **Did not re-dispatch Phase 5.** Two prior sessions already completed and committed all 22 tasks before this run started. Rather than trusting that inherited state, I independently re-ran every gate in `quickstart.md` fresh (§3) and re-derived the SC-007 diff-audit and suppression-count evidence myself rather than citing the prior commits' own claims about themselves. +- **Diagnosed, rather than assumed, the "pytest component-test collection issue."** Per instructions not to trust the prior runs' in-progress diagnosis, I reproduced it, bypassed the `rtk` CLI proxy's output filtering, and confirmed it was a summarizer false-negative (`Pytest: No tests collected` shown even on a fully passing run), not a real collection defect. No code change was made; this affects only how test output is read in this session's tooling, not the codebase. +- **Left the conftest.py `httpx.HTTPError` gap unfixed.** The error-handling reviewer flagged a real, but already-known-and-accepted, residual risk (§5). Since it was already surfaced and deliberately accepted during the critique/planning phase (with a documented fallback), and doesn't meet the "high severity or above" auto-fix bar, I recorded it rather than patching it, to avoid re-litigating a planning decision mid-review. +- **Did not attempt local Gogs-backed integration test execution.** The project's own quickstart.md already scopes this to "CI-only verification (accepted)." Standing up Gogs locally to second-guess an already-accepted plan decision would be scope creep for a review-and-report run; flagged here so a human can confirm the call still stands. +- **Ran the two review agents in parallel**, not sequentially. Both are read-only/advisory (no file edits), so unlike Phase 5 implementation chunks there's no shared-write conflict risk from concurrency. + +--- + +## 7. Suggested next steps + +1. Open a PR — the feature is complete, all gates are green, and no review finding met the bar for a blocking fix. +2. Optional polish (all advisory, non-blocking, listed in §5): disambiguate the two identical `m059` justification comments; rewrap the 4 over-120-char comments; trim the redundant sentence in `test_merge_kill_recovery.py`; widen the two `SchemaNotFoundError` catches to also catch `ValueError` if a cleaner diagnostic on already-failing tests is wanted. +3. Watch CI's integration tier for `backend/tests/integration/git/*.py` after merge. If either narrowed poll loop in `conftest.py` (lines 31/53) starts failing hard instead of retrying through a Gogs-startup race, fall back to the documented SUPPRESS treatment for those 2 sites per `data-model.md` Batch D — the code is correct either way (rollback note in `quickstart.md`). From 89e7ec451853c0f16147235574c50ad148b738eb Mon Sep 17 00:00:00 2001 From: Saltaferis Dimitrios Date: Wed, 22 Jul 2026 12:41:24 +0200 Subject: [PATCH 14/14] resolve conflicts --- .specify/feature.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.specify/feature.json b/.specify/feature.json index e9417158612..1be33c1f1c0 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/002-ruff-ble-reenable" + "feature_directory": "specs/001-entities-arch-migration" }