Skip to content

Clear the ruff 0.16 backlog (439 findings), then lift the pin — replaces the prematurely-closed #64 #75

Open

Description

@MSKazemi

ruff is pinned <0.16 in v4/pyproject.toml:25, and ruff>=0.16 is in .github/dependabot.yml's ignore list. That pin is deliberate — 0.16 turned on rules that report a large backlog — but it means CI is blind to every rule 0.16 added, and the pin cannot be lifted until the backlog is cleared.

#64 tracked this work and was closed prematurely when #70 cleared one of the four rule families. The rest was never re-filed, so it has been untracked since. This issue replaces it.

Current state, measured today (2026-08-10)

$ uvx ruff@latest check v4 --statistics

ruff 0.16.2, 439 findings across v4/:

Count Rule Autofix Notes
142 BLE001 blind-except — Judgment call per site; the biggest chunk
95 UP045 non-pep604-annotation-optional [*] ⚠️ SAFETY TRAP — read below before touching
75 I001 unsorted-imports [*] Mechanical — split out as its own issue
29 PLW1510 subprocess-run-without-check — Each needs a real decision about failure handling
17 UP035 deprecated-import [-]
11 RUF059 unused-unpacked-variable —
8 SIM117 multiple-with-statements —
7 S110 try-except-pass — Overlaps BLE001
~55 25 further rules, ≤5 each mixed

209 are autofixable. That number is a trap, not an invitation — see below.

⚠️ Never run ruff check --fix across this repo

UP045 rewrites Optional[X] → X | None. On an injected RunnableConfig parameter that silently disables RBAC and the human-in-the-loop approval gate.

langchain_core decides whether to inject the run config by matching the annotation by identity (type_ is RunnableConfig). Widen it and the parameter stops being injected: the tool receives config=None and loses user_role (RBAC) and hitl_bypass (the HITL gate). Every test still passes and the LLM-visible tool schema is unchanged, so it is invisible exactly where you would look first.

Affected files:

  • v4/packages/kubeintellect-server/app/tools/kubectl_tool.py
  • v4/packages/kubeintellect-server/app/tools/aci/read_verbs.py
  • v4/packages/kubeintellect-server/app/agent/nodes/coordinator.py

This is safety invariant #6 in AGENTS.md and is also warned about in v4/pyproject.toml. See #54 — the "obvious cleanup" there is the same defect.

How this should be done

One PR per rule family, so each is reviewable and revertible on its own. Do not bundle.

Suggested order, easiest and safest first:

  1. I001 (75) — split out, see the linked issue below
  2. RUF100 (5), UP017 (5), UP037 (3), RUF022 (2), UP041 (2) — small mechanical families
  3. UP006, UP035 — typing modernization, no runtime behaviour
  4. RUF059, SIM117, PIE810, and the other singletons
  5. PLW1510 (29) — needs a per-site decision: is a non-zero exit an error here or not?
  6. BLE001 (142) — the largest and the most judgment-heavy. Narrow the exception or add a justified # noqa with a reason. Do not blanket-noqa.
  7. UP045 (95) — last, and by hand. The three files above keep their annotations with # type: ignore[assignment] and a comment explaining why; everything else can be modernized.

Only then raise the bound in v4/pyproject.toml and drop the ruff>=0.16 entry from .github/dependabot.yml.

Verify each PR

make setup          # from the repo root — runs all six gates
uvx ruff@latest check v4 --statistics   # the family you fixed should be gone

Plus, for anything touching app/tools/ or app/agent/:

cd v4 && uv run pytest tests/test_kubectl_tool.py -q
uv run python -c "
from app.tools.kubectl_tool import run_kubectl
assert 'config' not in run_kubectl.get_input_schema().model_fields, 'config leaked into the tool schema — injection is broken'
print('config injection intact')"

That second check is the one that catches the UP045 regression; pytest alone does not, unless you run tests/test_kubectl_tool.py::TestAlwaysConfirm.

Scope

v1/–v3/ are excluded by design (ADR-001/002 — frozen so the published paper stays reproducible, and not built by CI). Findings there are not part of this issue.

Activity

  1. MSKazemi commented on Aug 10, 2026

    @MSKazemi
    OwnerAuthor

    Split out so far:

    Supersedes #64, which was closed on 2026-08-08 when #70 cleared EXE002 — but the remaining three families were never re-filed, so the work has been untracked since. Related: #54 is the same UP045 safety trap in miniature.

  2. MSKazemi commented on Aug 10, 2026

    @MSKazemi
    OwnerAuthor

    Item 1 is done: #76 (I001, 75 findings across 60 files) is cleared and merged via #77 by @shaurya703 (f0592ba).

    Re-measured on main just now:

    $ uvx ruff@latest check v4 --select I001 --statistics
    All checks passed!
    
    $ uvx ruff@latest check v4 --statistics | head -3
     142  BLE001    blind-except
      95  UP045     non-pep604-annotation-optional
      29  PLW1510   subprocess-run-without-check

    364 findings remain, so the ruff<0.16 pin and the dependabot ignore both stay for now.

    One thing from #77 worth carrying into the remaining families, because it will recur: ruff's autofix can silently detach a trailing # type: ignore comment. When it wrapped a long import into parenthesized form, the # type: ignore[import-not-found] moved off the from line and the suppression stopped applying — mypy went 0 → 1. Nothing about the ruff run reports this; only the mypy gate catches it. Expect the same in UP035 and UP006, which also rewrite import and annotation lines.

    Order from here is unchanged: the small mechanical families next, PLW1510 after that (each site needs a real decision about whether a non-zero exit is an error), BLE001 as the big judgement-heavy one, and UP045 last and by hand — the trap in the issue body still stands.

  3. MSKazemi commented on Aug 10, 2026

    @MSKazemi
    OwnerAuthor

    Split out: #79 — UP017 + UP041 + RUF022 (10 findings), labelled good first issue. Trialled before filing: the suites stay green, but the autofix leaves four F401 unused-timezone imports that the pinned ruff catches, so it is a two-step change. That is documented in the issue.

    One family to keep OUT of any "easy autofix" batch: RUF100 (unused-noqa, 5 findings). It looks trivial and it is a trap. Ruff reports those directives as unused because the rules they suppress are not currently enabled — but they are forward-looking:

    kube_q/cli/sessions_ui.py:67:31    non-enabled: BLE001
    kube_q/cli/v5_status_cmd.py:38:31  non-enabled: BLE001
    

    BLE001 is 142 of the findings in this very backlog. Deleting those noqas now would re-introduce the findings the moment BLE001 is enabled — and one of them is self-documenting (# noqa: BLE001 — surface any transport/HTTP error to the user). RUF100 should be run last, after the rule set is final, not as an easy early win.

  4. MSKazemi commented on Aug 21, 2026

    @MSKazemi
    OwnerAuthor

    Status re-measured on main today, and one correction to this thread.

    Correction

    The comment above records #135 as clearing the UP006 + UP035 family. #135 was closed on 2026-08-15 without being merged, so that work never landed. Nothing was lost by anyone's mistake — the author closed it — but the thread has been overstating progress since, and item 3 was still open.

    Where it actually stands

    #159 clears 362 → 210 and re-measures against ruff@latest, not against the pin:

    family was now
    UP045 92 0
    UP035 / UP006 / UP037 25 0
    I001 PIE810 RUF059 RUF012 FURB162 FURB188 RUF015 SIM102/113/114 C408 F401 F841 ISC004 PLR0124 ~45 0
    BLE001 142 141
    PLW1510 30 30

    Suites (1047 + 317), mypy and the pinned gate all stay green.

    The UP045 trap, which is worse than the RUF100 one flagged above

    Running ruff --select UP045 --fix rewrites the canary inside tests/test_injected_config_invariant.py:

    -def probe(q: str, config: Annotated[Optional[RunnableConfig], InjectedToolArg] = None) -> str:
    +def probe(q: str, config: Annotated[RunnableConfig | None, InjectedToolArg] = None) -> str:

    and the suite still reports 8 passed.

    That test is the negative control for AGENTS.md invariant #6 — it asserts the forbidden widened annotation is never injected. Neither spelling is in langchain_core's match list, so the canary reports None either way, and the autofix leaves a green test that no longer exercises the form it exists to forbid. It does not fail; it just stops guarding. Verified by running the autofix and re-running the suite, and now suppressed with the reasoning inline.

    This belongs on the same list as the RUF100 warning above: UP045 is not safe to batch-autofix in this repo. app/agent/nodes/coordinator.py was already suppressed for the live version of the same hazard.

    The decision this issue actually needs

    The remaining 171 are not cleanup:

    So lifting the pin means one of:

    1. Fix all 141 BLE001 — large, and mostly rewriting correct code.
    2. Lift the pin with a documented ignore for BLE001 (+ PLW1510 until those 30 are done). Trades a version pin for a rule ignore. Better than today only because it is narrow, greppable, and lets every other 0.16 rule become active — including EXE002, which currently needs its own dependency-free File modes gate to be seen at all.
    3. Leave the pin, and keep the blind spot.

    I lean to (2), because the value in this issue was never zero findings — it was CI being able to see. But it changes what the project enforces everywhere, so it is your call, not mine to assume.

    One more scope gap worth folding in: the lint gate only runs on packages/kubeintellect-server/app/ and packages/ki-protocol/. packages/kube-q/, tests/ and scripts/ are never linted in CI at all, whatever the pin says.

  5. MSKazemi commented on Sep 8, 2026

    @MSKazemi
    OwnerAuthor

    Re-measured 2026-09-09 with ruff 0.16.3, over the exact scope in AGENTS.md's ruff check command (app/, ki-protocol/, kube-q/, tests/, scripts/):

    Found 556 errors.
    [*] 233 fixable with the `--fix` option (58 hidden fixes with --unsafe-fixes)
    
     168  I001     [*] unsorted-imports
     165  BLE001   [ ] blind-except
      61  PLW1510  [ ] subprocess-run-without-check
      21  RUF100   [*] unused-noqa
      19  ISC004   [ ] implicit-string-concatenation-in-collection-literal
      16  C408     [ ] unnecessary-collection-call
      13  SIM117   [-] multiple-with-statements
      11  FURB167  [*] regex-flag-alias
    

    Narrowed to just the three packages (no tests/, no scripts/) it is 251. That reconciles the apparent jump from the 317 recorded in August — 317 was the three-package scope, and the same scope is 251 today. Both numbers were honest; neither said which scope it meant. AGENTS.md now states the scope next to the number (#188).

    UP045 is at zero — and that is the thing to understand before touching it

    It reports no findings at all now, and Optional[ is down to 8 occurrences in app/. The hand-audited PR this issue asked for already happened.

    The hazard has not gone away. It is at zero because it is suppressed, at three sites carrying an inline # noqa: UP045:

    app/agent/nodes/coordinator.py:736   coordinator(...)
    app/agent/nodes/coordinator.py:831   _direct_answer(...)
    tests/test_injected_config_invariant.py:112   the negative-control canary
    

    Verified by deleting the two in coordinator.py: UP045 fires again, and --fix rewrites both to RunnableConfig | None. The autofix is classified safe, so it applies without --unsafe-fixes. Reverted after measuring. So the standing rule is now narrower and more precise: never delete one of those three # noqa comments, rather than never run the family.

    One correction to what I wrote earlier in this thread and in AGENTS.md: I listed coordinator.py as a place where widening the annotation disables RBAC. It is not. LangGraph injects a node's config by parameter name, so every spelling receives it — measured:

    Optional[RunnableConfig]   -> user_role='admin'  OK
    RunnableConfig | None      -> user_role='admin'  OK
    RunnableConfig (bare)      -> user_role='admin'  OK
    

    Invariant #6's hazard is specific to tools, where langchain_core matches type_ is RunnableConfig by identity — kubectl_tool.py and the four ACI read verbs, all currently bare and correct, gated by test_injected_config_invariant.py. The coordinator.py suppressions are still worth keeping (they stop a batch --fix sweeping the file), but there is no RBAC bug there to find.

    PLW1510 is not a cosmetic family — read it, do not batch it

    61 findings, and the assumption that check=False is uniformly behaviour-preserving is wrong. Reading them surfaced a real bug, now filed as #189:

    $ kubeintellect service start
    Failed to start kubeintellect.service: Unit kubeintellect.service not found.
    $ echo $?
    0            # systemctl itself exited 5

    cmd_service() discards the exit code for start, stop, status and logs, while install/uninstall ten lines above handle it correctly. That is one lint family containing both 57 cosmetic findings and 4 genuine defects — which is the argument for per-rule PRs that read the findings, and against --fix as a strategy.

    app/cli.py holds 20 of the 61 on its own; the rest are spread thin across tests/.

    Where this leaves the decision

    The three options in my earlier comment still stand, and I still lean to (2) — lift the pin with a narrow, documented ignore for BLE001 — for the same reason: the value here was never "zero findings", it was CI being able to see. The re-measurement strengthens it, because I001 (168, all auto-fixable) and the smaller families are pure upside currently invisible to CI.

    Also still true and still worth fixing: the CI lint gate only covers app/ and ki-protocol/. kube-q/, tests/ and scripts/ are never linted in CI at all — which is why 305 of the 556 findings above are in files no gate has ever looked at.

    Good first issues split out of this: #189 (the real bug), and I001 is a clean 168-finding auto-fixable sweep if anyone wants a genuinely low-risk first PR — comment here and it is yours.

  6. MSKazemi commented on Sep 8, 2026

    @MSKazemi
    OwnerAuthor

    Correction to my comment above — the last paragraph was wrong, and I should have checked before repeating it.

    I wrote that the CI lint gate "only runs on packages/kubeintellect-server/app/ and packages/ki-protocol/" and that kube-q/, tests/ and scripts/ are "never linted in CI at all". That has not been true since 2026-08-24. I carried the claim forward from an earlier comment in this thread instead of reading the workflow.

    The Lint (ruff) job in .github/workflows/ci.yml today runs:

    - name: ruff check
      run: >-
        uv run ruff check
        packages/kubeintellect-server/app/ packages/ki-protocol/ packages/kube-q/
        tests/ scripts/

    with a comment above it recording exactly when it was widened and that doing so cost 31 fixes. So the scope already matches what AGENTS.md documents, and no tracked Python root is unlinted.

    What is true, and is the accurate version of the point I was reaching for: CI runs the pinned ruff 0.15.22 (ruff>=0.15,<0.16), and it passes clean over that full scope. The 556 findings I posted are from 0.16.3, which CI does not run. So those files are linted — just not by the ruleset that would report those 556. The gap is the ruleset, not the scope, and EXE002 remains the concrete example (which is why make check-modes exists as a separate gate).

    That makes the case for option (2) a little weaker than I implied: widening scope is already done, so lifting the pin buys new rules over an already-complete file set, not new coverage. Still worth doing, for a smaller reason than I gave.

    Everything else in that comment — the 556/251 measurement, the UP045 suppression finding, and #189 — was measured directly and stands.

  7. MSKazemi commented on Sep 8, 2026

    @MSKazemi
    OwnerAuthor

    Continuing to read families rather than batch-fix them — the S110/S112 pair (5 findings) produced a second real defect, filed as #190.

    Assessment of all five:

    Site Verdict
    main.py:27 ✅ deliberate, commented "never let a diagnostic log break startup"
    workflow.py:664 ✅ close_session cleanup in a finally
    chat_completions.py:294 ✅ intentional post-task.cancel() drain — and the odd-looking except (asyncio.CancelledError, Exception) is necessary, since CancelledError derives from BaseException
    kubectl_tool.py:1333 🔴 drops an object from the rollback capture
    kubectl_tool.py:1388 🔴 same, via timeout/exception

    The last two, plus a silent targets[:5] cap, let _capture_rollback_point record restorable: true for a capture covering 3 of 7 objects — reproduced, with the postmortem then rendering it as plainly [restorable]. Details in #190.

    Also checked and not filed, to keep the noise down:

    • PLR0124 engine.py:381 — val != val is the intentional NaN test, already commented # NaN / Inf. A contributor "fixing" this would break NaN filtering. Needs a # noqa when the pin lifts, not a change.
    • RUF059 coordinator.py:1121 — the unused has_warnings is deliberate; the adjacent comment says why ("Stale events are not regressions"). Cosmetic _ rename at most.

    Running total from reading this backlog: two genuine defects (#189, #190) out of ~10 findings read. That is a much better yield than I expected and is the strongest argument yet for per-rule PRs by a human over ruff --fix.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/serverkubeintellect-serverhelp wantedExtra attention is needed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions