[agent] cleanup: Restored unused deps script, standardized TODO metadata, and fixed security rate limits - #384
Conversation
…ata, and fixed security rate limits * Restored scripts/find_stale_unused_deps.py * Updated scripts/extract_todos_structured.py to parse the owner field and exclude itself * Fixed RateLimitMiddleware IP extraction to gracefully handle default trusted proxy counting * Addressed `ruff` warnings and test mocking requirements
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Reviewer's GuideThis maintenance PR restores dependency-audit tooling, extends TODO extraction with owner metadata and self-scan exclusions, fixes trusted-proxy rate-limit configuration to be resolved dynamically, updates security tests to validate proxy/IP-boundary behavior, and applies Ruff cleanup throughout the backend scripts and test suite. Sequence diagram for dynamic trusted-proxy client IP extractionsequenceDiagram
participant Middleware as RateLimitMiddleware
participant Security as extract_client_ip_from_forwarded
participant Config as TRUSTED_PROXY_COUNT
Middleware->>Security: extract_client_ip_from_forwarded(forwarded, trusted_proxy_count, fallback_ip)
alt trusted_proxy_count is None
Security->>Config: Read current TRUSTED_PROXY_COUNT
Config-->>Security: proxy count
end
Security-->>Middleware: Extracted client IP or fallback_ip
Middleware-->>Middleware: Apply IP-bound rate limit
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/find_stale_unused_deps.py" line_range="70-102" />
<code_context>
+def check_frontend():
</code_context>
<issue_to_address>
**issue (bug_risk):** `check_frontend` resolves `frontend/package.json`, searches `frontend/src`, and writes `unused_deps_report.txt` relative to the process working directory rather than the repository or script location. Running the restored script from `scripts/` or another directory therefore skips the frontend and backend files and writes the report in the caller's directory.
**Triggers:** When the script is invoked from any working directory other than the repository root.
**Suggested fix:** Resolve the repository root from `__file__` (or accept an explicit root argument) and construct all package, source, and report paths from that root.
</issue_to_address>
### Comment 2
<location path="scripts/find_stale_unused_deps.py" line_range="6" />
<code_context>
+import subprocess
+import os
+import re
+from datetime import datetime, timedelta, timezone
+
+# --- CONFIG ---
</code_context>
<issue_to_address>
**nitpick:** `timedelta` is imported but never used, leaving the restored script with a dead dependency in its own implementation and causing the configured import-lint check to report an unused import if this script is linted under normal Ruff rules.
**Triggers:** When the repository lint configuration is applied to the restored script without the broad unused-import suppression used by the backend configuration.
**Suggested fix:** Remove `timedelta` from the import list.
```suggestion
from datetime import datetime, timezone
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def check_frontend(): | ||
| print("Checking frontend...") | ||
| pkg_json = "frontend/package.json" | ||
| if not os.path.exists(pkg_json): | ||
| return [] | ||
|
|
||
| with open(pkg_json, "r", encoding="utf-8") as f: | ||
| pkg = json.load(f) | ||
|
|
||
| deps = list(pkg.get("dependencies", {}).keys()) + list( | ||
| pkg.get("devDependencies", {}).keys() | ||
| ) | ||
|
|
||
| blame = get_file_blame_lines(pkg_json) | ||
|
|
||
| results = [] | ||
| for dep in deps: | ||
| # Exclude internal or highly common toolings if desired, but for MVP check all | ||
| if dep in ["react", "react-dom", "typescript", "vite"]: | ||
| continue | ||
|
|
||
| # Find oldest insertion of this dep in package.json | ||
| age = 0 | ||
| for b_line in blame: | ||
| if f'"{dep}"' in b_line["content"]: | ||
| age = max(age, get_age_days(b_line.get("time", 0))) | ||
|
|
||
| if age > AGE_THRESHOLD_DAYS: | ||
| # Check if used | ||
| if not is_used( | ||
| dep, ["frontend/src"], [".ts", ".tsx", ".js", ".jsx", ".css"] | ||
| ): | ||
| results.append(f"[Frontend] {dep} (Age: {age} days)") |
There was a problem hiding this comment.
issue (bug_risk): check_frontend resolves frontend/package.json, searches frontend/src, and writes unused_deps_report.txt relative to the process working directory rather than the repository or script location. Running the restored script from scripts/ or another directory therefore skips the frontend and backend files and writes the report in the caller's directory.
Triggers: When the script is invoked from any working directory other than the repository root.
Suggested fix: Resolve the repository root from __file__ (or accept an explicit root argument) and construct all package, source, and report paths from that root.
| import subprocess | ||
| import os | ||
| import re | ||
| from datetime import datetime, timedelta, timezone |
There was a problem hiding this comment.
nitpick: timedelta is imported but never used, leaving the restored script with a dead dependency in its own implementation and causing the configured import-lint check to report an unused import if this script is linted under normal Ruff rules.
Triggers: When the repository lint configuration is applied to the restored script without the broad unused-import suppression used by the backend configuration.
Suggested fix: Remove timedelta from the import list.
| from datetime import datetime, timedelta, timezone | |
| from datetime import datetime, timezone |
…ata, and fixed security rate limits * Restored scripts/find_stale_unused_deps.py * Updated scripts/extract_todos_structured.py to parse the owner field and exclude itself * Fixed RateLimitMiddleware IP extraction to gracefully handle default trusted proxy counting * Addressed `ruff` warnings and test mocking requirements * Resolved SonarCloud Quality Gate security failures (do not catch blind exceptions, handle subprocess inputs safely, format loggers properly)
…ata, and fixed security rate limits * Restored scripts/find_stale_unused_deps.py * Updated scripts/extract_todos_structured.py to parse the owner field and exclude itself * Fixed RateLimitMiddleware IP extraction to gracefully handle default trusted proxy counting * Addressed `ruff` warnings and test mocking requirements * Resolved SonarCloud Quality Gate security failures (do not catch blind exceptions, handle subprocess inputs safely, format loggers properly)
|
Agent Report Summary
scripts/find_stale_unused_deps.py, improvedscripts/extract_todos_structured.pyto handleownerfields and exclude parsing itself. Fixed rate limit default parameter typing inbackend/src/agent/security.py, refactored associated security tests to dynamically mockTRUSTED_PROXY_COUNT, and resolvedruffstyle violations.Scan Results
TODOs
ownermetadata compatibility.Convention Enforcement
ruff formatandruff check. Lambda functions in tests replaced by explicitdef. Test mocks of global configs shifted to usepatch.objectblock bindings.Verification
cd backend && uv run pytest tests/,cd backend && uv run ruff check src/ scripts/ tests/,cd backend && uv run ruff format src/ scripts/ tests/RateLimitMiddlewareadjustments.Risk Assessment
Next Steps
Machine Metadata
Checklist for reviewers
PR created automatically by Jules for task 915630393390433764 started by @MasumRab
Summary by Sourcery
Restore dependency and TODO maintenance tooling, correct forwarded-client rate-limit handling, and standardize the affected code and tests.
New Features:
Bug Fixes:
Enhancements:
Tests: