fix: make linting deterministic and clear the 42 ruff findings - #30
Merged
Conversation
The lint gate was red on every PR, so it signalled nothing — a real regression
would have been indistinguishable from the standing failure, and `test` (which
`needs: lint`) never ran at all.
Root cause: nothing in this repo changed. Dev deps pinned `ruff>=0.4`, CI
resolved ruff 0.16.1, and ruff's DEFAULT rule set has expanded since 0.4. The
code was clean against the rules it was written for — proven by running 0.16.1
with the historical default set:
ruff check . --select E4,E7,E9,F -> All checks passed!
ruff check . -> Found 42 errors.
So the fix is two parts: stop the rule set from drifting, then fix what the
chosen rules actually find.
1. Declare the rule set explicitly in [tool.ruff.lint] instead of inheriting
whatever a future ruff decides, and upper-bound ruff and mypy. An open-ended
linter pin means CI can go red with no code change — which is exactly what
happened.
2. Curate that set for what this repo IS — a toolkit for inspecting and
exercising the network. Each exclusion is commented in pyproject.toml with
its reason; the notable ones:
• S (bandit) is not selected. It flags raw sockets, shell=True, binding
0.0.0.0 and reverse shells — i.e. the features. It would fight this repo
permanently.
• BLE001 is not selected. The blind `except Exception` blocks are
best-effort fallback chains; narrowing them would make the fallbacks MORE
brittle, since a malformed /proc/net/route line should fall through to
the default rather than raise.
• PLR2004 (magic values) is ignored: protocol code is made of numbers the
spec defines, and tests assert literal ports and lengths on purpose.
• PLR0912/PLR0915 are suppressed on the sniffer's main() alone, with a
noqa at that function, rather than relaxed repo-wide.
3. Fixed everything the curated set flags — import ordering, percent-format to
f-strings in the TCP proxy, collapsed nested conditionals in filters,
contextlib.suppress for a Windows-only teardown, list-unpacking over
concatenation, an unused unpacked `addr`, dict() literal, redundant .keys(),
an explicit check=False on a subprocess.run whose non-zero exit is a valid
outcome, and two over-length lines.
One is worth calling out: tests/test_cli.py had a closure reading
`fake_module` from the enclosing loop scope (B023). It is correct TODAY only
because the closure is invoked within the same iteration; any deferred call
would see the last iteration's value. Now bound per-iteration as a default
argument, so it cannot rot into a real bug.
Behaviour is unchanged throughout. Verified: ruff clean, mypy clean (23 source
files — it had never run, being gated behind lint), and pytest 180 passed. The
same suite reports 180 passed on main, so no test was lost, skipped, or altered
in substance.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ AgentGate: Passed18 files changed · +95 -56 lines No issues found. Generated by AgentGate |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The lint gate was red on every PR, so it signalled nothing — a real regression would have been indistinguishable from the standing failure. And because
testdeclaresneeds: lint, the test suite never ran in CI at all.Nothing in this repo changed — the linter did
Dev deps pinned
ruff>=0.4; CI resolved ruff 0.16.1, whose default rule set has expanded since 0.4. The code is clean against the rules it was written for:That reframes the job. Fixing 42 findings without addressing the pin just means the gate goes red again on the next ruff release.
The fix
1. Stop the drift. The rule set is now declared explicitly in
[tool.ruff.lint]rather than inherited, andruff/mypyare upper-bounded. An open-ended linter pin means CI can go red with no code change — which is precisely what happened.2. Curate the set for what this repo is. Every exclusion is commented in
pyproject.tomlwith its reason. The notable ones:S(bandit)shell=True, binding0.0.0.0, reverse shells — the features. It would fight this repo permanently.BLE001except Exceptionblocks are best-effort fallback chains. Narrowing them makes them more brittle — a malformed/proc/net/routeline should fall through to the default, not raise.PLR2004proto == 6), and tests assert literal ports on purpose.PLR0912/0915main()alone, via anoqaat that function, rather than relaxed repo-wide.3. Fixed everything the curated set flags — import ordering, percent-format → f-strings in the TCP proxy, collapsed nested conditionals,
contextlib.suppressfor a Windows-only teardown, list-unpacking over concatenation, an unused unpackedaddr, adict()literal, redundant.keys(), an explicitcheck=False, and two over-length lines.One worth your eye
tests/test_cli.pyhad a closure readingfake_modulefrom the enclosing loop scope (B023). It is correct today only because the closure happens to be invoked within the same iteration — any deferred call would see the last iterations value. Now bound per-iteration as a default argument so it cannot rot into a real bug. I am flagging it as a latent hazard rather than claiming I fixed a live bug, because it was not one.Verification
ruff check .mypy src/pytestCrucially, I ran the suite on a clean worktree of
mainas a baseline: also 180 passed. Same count before and after, so no test was lost, skipped, or altered in substance. Behaviour is unchanged throughout — this is a lint-and-config change, not a refactor.Note on the pins
ruff>=0.16,<0.17andmypy>=2.3,<3are deliberate. Dependabot will still offer the majors; those become a considered upgrade (bump the pin, re-run, adjust the rule set if defaults moved) rather than a surprise red build on an unrelated PR.