-
Notifications
You must be signed in to change notification settings - Fork 15
Matas/fix/pr 36 audit #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughRestructures repository analysis into a LangGraph agent, exposes a rate-limited unauthenticated public-repo recommendation endpoint, centralizes GitHub integration/service with anonymous-access support, consolidates tooling around Ruff, revises models/prompts, and adds integration tests and DI/rate-limit dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Layer
participant RateLimiter as Rate Limiter
participant Agent as LangGraph Agent
participant GitHub as GitHub API
participant LLM
Client->>API: POST /api/v1/rules/recommend
API->>RateLimiter: check(key: IP or user)
alt limit exceeded
RateLimiter-->>API: 429 + Retry-After
API-->>Client: 429 Too Many Requests
else allowed
RateLimiter-->>API: allowed
API->>Agent: execute(repo_full_name, is_public)
Agent->>GitHub: fetch_repository_metadata(repo)
GitHub-->>Agent: repository signals
Agent->>LLM: generate_rule_recommendations(system prompt + user prompt)
LLM-->>Agent: structured recommendations
Agent-->>API: recommendations
API-->>Client: 200 OK + AnalysisResponse
end
sequenceDiagram
participant Requestor
participant Dep as Dependencies
participant Auth as Auth Resolver
participant GitHub as GitHub Service
Requestor->>Dep: request with optional Authorization
Dep->>Auth: get_current_user_optional()
alt token present
Auth-->>Dep: User
else no token
Auth-->>Dep: None
end
Dep->>GitHub: get_repository(repo, user_token?, installation_id?)
alt public repo
GitHub->>GitHub: _get_auth_headers(allow_anonymous=true)
GitHub-->>Dep: repo metadata
else private + auth
GitHub->>GitHub: _get_auth_headers(user_token or install token)
GitHub-->>Dep: repo metadata
else private + no auth
GitHub-->>Dep: 404 / None
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @MT-superdev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant enhancements to the Watchflow project, focusing on improving code quality, security, and API accessibility. The core changes include refactoring the Repository Analysis Agent, updating dependencies for better linting and formatting, enabling unauthenticated access for public repositories, and improving the testing infrastructure. These changes collectively aim to make the project more robust, maintainable, and user-friendly. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant and impressive refactoring. The move to a langgraph-based agent architecture, consolidation of linting with ruff, and the addition of unauthenticated access with rate limiting are all major improvements. The code is cleaner, more structured, and the new integration tests are a great addition.
However, I've identified a few critical and high-severity issues that need to be addressed:
- A critical security flaw in the placeholder authentication logic.
- A bug in the new rate limiter that causes it to not work correctly for authenticated users.
- A design flaw in the error handling between the GitHub client and the analysis agent, which causes repository-not-found errors to be silently ignored.
- Several configuration removals in
pyproject.tomlthat could break coverage reporting, documentation builds, and package publishing.
I've left detailed comments on these points. Once these are addressed, this will be an excellent contribution to the codebase.
I am having trouble creating individual review comments. Click here to see my feedback.
src/api/dependencies.py (39-40)
This placeholder authentication logic presents a critical security vulnerability. It creates a mock User object for any request that includes an Authorization: Bearer <any-string> header, without validating the token. This would allow any user to bypass authentication and gain access to protected resources simply by providing a non-empty token. While the TODO comment acknowledges this is a placeholder, it's crucial to replace this with proper token validation (e.g., JWT signature verification against a public key from your IdP) before this code reaches a production environment.
src/api/rate_limit.py (20)
There's a bug in how the user is obtained in this rate limiter. The dependency user: User | None = Depends(lambda: None) will always result in user being None. This means the logic will never use the authenticated user's email for rate limiting, and all users will be treated as anonymous and subjected to the lower IP-based limit.
To fix this, the dependency should be changed to use the optional user dependency you've already created. This will allow FastAPI to correctly resolve the nested get_current_user_optional dependency when rate_limiter is used.
async def rate_limiter(request: Request, user: User | None = Depends(get_current_user_optional)):
src/integrations/github/api.py (114-129)
There's a design issue here with error handling. Methods like list_directory_any_auth and get_repository return an empty list or None when the GitHub API returns a non-200 status (like a 404 Not Found). This swallows the error, and the calling code (the RepositoryAnalysisAgent) has no way of knowing that the repository doesn't exist. The agent then proceeds with an analysis based on empty data, leading to a successful but meaningless result. The error handling in the API layer that expects a 'not found' message from the agent will never be triggered.
Consider refactoring the client methods to raise specific exceptions (e.g., GitHubResourceNotFoundError) on 404 errors, so that the agent can catch these and return a proper failure AgentResult.
pyproject.toml (99-108)
The keywords and classifiers sections have been removed from the project metadata. This information is important for package discovery and classification if you plan to publish this project to PyPI. If publishing is a goal, it would be best to restore these fields.
pyproject.toml (141-147)
The [project.optional-dependencies.docs] section, which includes mkdocs and its plugins, has been removed. This will likely break the documentation build process. If the project still maintains documentation with mkdocs, these dependencies should be restored.
pyproject.toml (213-234)
The [tool.coverage.run] and [tool.coverage.report] sections have been removed. While pytest-cov has defaults, this removes specific configurations like omit paths and lines to exclude. Was this intentional? If this configuration is still needed for accurate coverage reports in CI, it should be restored or moved to a different configuration file (e.g., .coveragerc).
src/integrations/github/service.py (1-115)
This new GitHubService file appears to be unused in the current PR and introduces some inconsistencies. It uses httpx directly, while github_client has been refactored to consistently use aiohttp. It also seems to duplicate some analysis logic (e.g., checking for CODEOWNERS) that is now handled by the RepositoryAnalysisAgent. To avoid confusion and dead code, could you either integrate this service fully into the application's architecture (and align it with aiohttp) or remove it if it's a remnant of a previous approach?
src/main.py (93)
The CORS configuration has been changed to allow all origins ("*"). While the comment notes this is for development, this is a security risk in a production environment. It's better to use the environment-based configuration that was previously in place, allowing you to maintain a strict allowlist for production while using a more permissive one for development.
allow_origins=config.cors.origins, # Read allowed origins from config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/rules/utils/contributors.py (1)
55-55: Potential timezone mismatch will causeTypeErrorat runtime.
datetime.now()returns a timezone-naive datetime, but the dates parsed in_has_recent_activity(lines 196, 206) andget_user_contribution_stats(lines 146, 151, 156) use.replace("Z", "+00:00")producing timezone-aware datetimes. Comparing naive and aware datetimes raisesTypeError.🐛 Proposed fix
- cutoff_date = datetime.now() - timedelta(days=days_back) + cutoff_date = datetime.now(tz=timezone.utc) - timedelta(days=days_back)Update the import at line 9:
-from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezonesrc/integrations/providers/bedrock_provider.py (3)
239-246: Async method blocks the event loop.
_ageneratesimply delegates to the synchronous_generatemethod, which will block the event loop when called in async contexts. If async execution is expected, this should either useasyncio.to_thread()to run in a thread pool, or the Anthropic client's async API if available.🔧 Suggested fix using asyncio.to_thread
async def _agenerate( self, messages: list[BaseMessage], stop: list[str] | None = None, run_manager: Any | None = None, ) -> ChatResult: """Async generate using the Anthropic client.""" - return self._generate(messages, stop, run_manager) + import asyncio + return await asyncio.to_thread(self._generate, messages, stop, run_manager)
202-204:with_structured_outputstub silently ignores the schema.Returning
selfwithout processingoutput_modelmeans structured output requests are silently ignored. Callers expecting JSON schema enforcement or Pydantic model parsing will receive raw text instead, which could cause downstream failures.Consider either implementing actual structured output handling or raising
NotImplementedErrorto make the limitation explicit.
213-224: System messages mapped incorrectly for Anthropic API.Mapping
"system"type messages to role"user"(line 220) will not work as intended. The Anthropic API expects system prompts to be passed via a separatesystemparameter inmessages.create(), not as user messages. This could lead to incorrect model behavior.🔧 Suggested approach
Extract system messages and pass them separately:
system_content = None anthropic_messages = [] for msg in messages: if msg.type == "system": system_content = msg.content elif msg.type == "human": anthropic_messages.append({"role": "user", "content": msg.content}) elif msg.type == "ai": anthropic_messages.append({"role": "assistant", "content": msg.content}) else: anthropic_messages.append({"role": "user", "content": msg.content}) create_kwargs = { "model": self.model_id, "max_tokens": self.max_tokens, "temperature": self.temperature, "messages": anthropic_messages, } if system_content: create_kwargs["system"] = system_content response = self.anthropic_client.messages.create(**create_kwargs)src/rules/validators.py (3)
189-199: FilePatternCondition always fails because changed files are never extracted.
For bothpull_requestandpush,_get_changed_filesreturns[], sovalidate()returnsFalsefor every event. Populate from the event payload (consistent withCodeOwnersCondition) to make the rule functional.🛠️ Proposed fix (align with existing event structure)
def _get_changed_files(self, event: dict[str, Any]) -> list[str]: """Extracts the list of changed files from the event.""" - event_type = event.get("event_type", "") - if event_type == "pull_request": - # TODO: Pull request—fetch changed files via GitHub API. Placeholder for now. - return [] - elif event_type == "push": - # Push event—files in commits, not implemented. - return [] - else: - return [] + files = event.get("files", []) + if files: + return [file.get("filename", "") for file in files if file.get("filename")] + + pull_request = event.get("pull_request_details", {}) + if pull_request: + return pull_request.get("changed_files", []) + + return []
218-226: Replace hardcoded contributor list with the contributor analyzer used byPastContributorApprovalCondition.The hardcoded list
["new-user-1", "new-user-2", "intern-dev"]will silently miss any real first-time contributors not in that list. Sincesrc/rules/utils.contributors.is_new_contributor()is already available and used byPastContributorApprovalCondition, apply the same pattern here: extractrepo,github_client, andinstallation_idfrom the event and call the analyzer instead of checking against a static list.
126-147: AuthorTeamCondition is production-accessible with no gating and cannot be configured with authoritative team data.The
author_team_isvalidator is registered inVALIDATOR_REGISTRYand directly invoked inengine_agent/nodes.pywithout any environment-based gating or demo-only checks. There is no mechanism to inject authoritative GitHub team membership data at runtime—configuration, environment variables, or initialization parameters. The hardcoded memberships (2 teams, 4 users) will misclassify any team or user not explicitly listed, allowing or denying access incorrectly.Either:
- Disable this validator until it can call the real GitHub API, or
- Add a configuration mechanism to load team memberships from an external source (GitHub API or configuration file).
pyproject.toml (1)
31-109: Bump Ruff minimum version to support configured options.The
docstring-code-formatkey requires Ruff v0.1.8+, andindent-widthrequires v0.2.0+. Update the constraint fromruff>=0.1.0to at leastruff>=0.2.0in both[project.optional-dependencies]and[tool.uv]sections to ensure CI and development environments use a version that actually supports these configuration keys.
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 238-245: The README incorrectly lists the repository analysis
endpoint as /v1/rules/recommend; update the documentation to use the actual
mounted path /api/v1/rules/recommend. Edit the README.md entry under
"Unauthenticated Analysis & Rate Limiting" to replace `/v1/rules/recommend` with
`/api/v1/rules/recommend` (consistent with the API router mount via
app.include_router and the integration tests), and verify any other README
references to the endpoint include the /api prefix.
In `@src/agents/repository_analysis_agent/agent.py`:
- Around line 18-21: The constructor for repository_analysis agent (__init__)
currently hardcodes super().__init__(agent_name="repository_analysis") and drops
BaseAgent kwargs; update __init__ to accept and forward parameter kwargs (e.g.,
**kwargs or explicit max_retries, retry_delay) and pass them into
super().__init__ along with agent_name="repository_analysis" so existing
callers' configuration (max_retries, retry_delay, etc.) is preserved.
In `@src/agents/repository_analysis_agent/models.py`:
- Around line 22-39: The model currently lets both repository_full_name and
repository_url be empty; update RepositoryAnalysisRequest.normalize_repo_name to
validate early: after trying to normalize (using parse_github_repo_identifier
when repository_url is provided and repository_full_name is empty), if
repository_full_name is still empty then raise a validation error (e.g., raise
ValueError("repository identifier required: provide repository_full_name or
repository_url")) so Pydantic returns a 422 instead of allowing downstream
ValueError.
In `@src/agents/repository_analysis_agent/nodes.py`:
- Around line 138-151: The current RecommendationsList class inherits
AnalysisState (which requires repo_full_name) causing Pydantic validation to
fail; replace it with a minimal Pydantic model that only defines
recommendations: list[RuleRecommendation] (do not inherit AnalysisState) and use
that model with structured_llm = llm.with_structured_output(RecommendationsList)
so the LLM output validates against the simple schema; update any imports if
needed to reference pydantic.BaseModel (or your project's lightweight model
base) and ensure response.recommendations access remains the same.
- Around line 24-83: The node silently fails for private repos because
fetch_repository_metadata and this code path call
github_client.list_directory_any_auth and github_client.get_file_content with
installation_id=None (and no user_token), ignoring AnalysisState.is_public and
lacking auth info; update AnalysisState to include installation_id and
user_token, propagate those fields into fetch_repository_metadata and into this
node, and pass the appropriate installation_id and/or user_token into
github_client.list_directory_any_auth and github_client.get_file_content calls
(prefer user_token when present, fallback to installation_id) so private repos
are accessed with proper auth instead of anonymous.
In `@src/api/dependencies.py`:
- Around line 24-43: The current get_current_user_optional function returns a
User for any Bearer token, which effectively bypasses auth; change it to either
validate the token against the real IdP (call your token
introspection/verification method or SDK and construct User from its claims) or
gate the current stub behind an explicit development-only flag (e.g., check a
DEV_MODE or FEATURE_FLAG) so the stub path is only executed in non-prod; ensure
the production path fails closed (return None or raise) when verification fails,
and log failures via logger with clear context; update get_current_user_optional
(and any helper like the User construction) to reflect this change.
In `@src/api/rate_limit.py`:
- Around line 8-27: The rate_limiter function currently uses Depends(lambda:
None) so user is always None; replace that dependency with
Depends(get_current_user_optional) to allow authenticated users to be detected:
update the function signature in rate_limiter to use
Depends(get_current_user_optional) and add an import for
get_current_user_optional from src.api.dependencies (and ensure time is imported
if missing) so key selection uses the real User object and AUTH_LIMIT can apply.
In `@src/api/recommendations.py`:
- Around line 103-106: Change the logged user identifier to a non-sensitive
field instead of email: in the handler where client_ip, user and repo_url_str
are set (the lines assigning client_ip and user_id and the subsequent
logger.info call), replace user.email with a non-PII attribute such as user.id
or user.username (e.g., user_id = user.id if user else "Anonymous") and keep the
log format using that new user_id variable so emails are not written to logs.
In `@src/core/config/settings.py`:
- Around line 133-150: The provider string comparison is case-sensitive;
normalize provider to lowercase before validation so checks work for values like
"Bedrock" or "VERTEX_AI". Update either the ProviderConfig initialization to set
ai.provider = ai.provider.lower() (or equivalent normalizing method) or modify
the validation here to use self.ai.provider.lower() for the comparisons and
membership test (affecting the openai check, the bedrock check, and the
vertex_aliases membership test). Ensure vertex_aliases remains lowercase and
perform comparisons against the lowered provider consistently.
In `@src/core/models.py`:
- Around line 7-18: The User model currently stores OAuth tokens as plain
strings (github_token) which can be serialized/logged; change the github_token
field to use pydantic SecretStr (import SecretStr from pydantic) and mark the
Field with parameters that prevent exposure (e.g., repr=False and exclude=True
or the pydantic v2 equivalent) so model_dump()/FastAPI responses and repr()
won't include the secret; also update any code that reads github_token to call
the SecretStr accessor (e.g., .get_secret_value() or .secret) instead of
treating it as a plain str.
In `@src/core/utils/caching.py`:
- Around line 161-164: The ternary currently uses truthiness on ttl which treats
ttl=0 as missing; update the logic around cache assignment to check explicitly
for None instead (use "if ttl is not None" or invert with "if ttl is None" and a
default), so that AsyncCache(maxsize=maxsize, ttl=ttl) receives explicit falsy
values like 0 and TTLCache(..., ttl=(ttl if ttl is not None else 3600)) only
falls back to 3600 when ttl is None; adjust the cache assignment line that
references AsyncCache and TTLCache accordingly.
In `@src/integrations/github/api.py`:
- Around line 168-229: The list_pull_requests function currently returns [] when
both installation_id and user_token are absent; update its call to
_get_auth_headers in list_pull_requests to pass allow_anonymous=True so
anonymous requests to public repos are permitted (matching
get_repository/list_directory_any_auth/get_file_content). Specifically, in
list_pull_requests change headers = await
self._get_auth_headers(installation_id=installation_id, user_token=user_token)
to include allow_anonymous=True and keep the rest of the logic (URL,
session.get, response handling) unchanged.
In `@src/integrations/github/service.py`:
- Around line 51-105: The loop in analyze_repository_rules silently treats
403/429 rate-limit responses as "not found" and yields misleading
recommendations; update the HTTP handling inside the for filepath loop (the
AsyncClient.get call and its response processing) to detect resp.status_code 403
or 429 and/or check headers "X-RateLimit-Remaining" and "X-RateLimit-Reset", and
when remaining == "0" or status is 403/429, either back off/retry after the
reset time or raise a specific exception to surface rate-limiting instead of
treating the file as absent; ensure successful 200 responses still append to
found_files and other non-200 non-rate-limit responses are handled/logged
appropriately.
- Around line 32-49: Update get_repo_metadata to accept an optional token
parameter (e.g., token: Optional[str] = None) like analyze_repository_rules,
build an Authorization header when token is provided (e.g., "Authorization":
f"token {token}"), and pass those headers into the httpx.AsyncClient.get call so
private repos can be accessed; keep existing error handling (404 ->
GitHubResourceNotFoundError, 403 rate-limit check, response.raise_for_status())
and reuse existing helpers like _parse_url and BASE_URL to locate the repo
endpoint.
In `@src/integrations/providers/bedrock_provider.py`:
- Around line 156-159: The profile matching is too broad: update the
is_anthropic check to remove the generic "general" and "default" keywords so it
only looks for explicit Anthropic identifiers; change the any(...) invocation
that builds is_anthropic (which currently checks profile_name for
["claude","anthropic","general","default"]) to only check
["claude","anthropic"], keeping the subsequent guard that also verifies
model_lower contains "anthropic" or "claude" before returning profile_arn.
In `@src/main.py`:
- Around line 88-97: Replace the hardcoded CORS settings in the
app.add_middleware(CORSMiddleware, ...) call by importing and using the
application's Config object (e.g., import Config or the instantiated config) and
pass config.cors (instead of allow_origins=["*"]) and config.cors_headers
(instead of allow_headers=["*"]) to the CORSMiddleware parameters; keep
allow_credentials and allow_methods as needed but source origins/headers from
the config so environment variables (CORS_ORIGINS / CORS_HEADERS) are respected.
In `@src/rules/validators.py`:
- Around line 238-243: The validate method currently always returns True,
bypassing the intended min-approval logic; update the validate function in
ApprovalCountCondition to read min_approvals from parameters (e.g.,
parameters.get("min_approvals", 1")), obtain actual review/approval count from
the incoming event (or delegate to the existing MinApprovalsCondition
class/method), and return True only if the approval count meets or exceeds
min_approvals; ensure you reference and call MinApprovalsCondition if you choose
delegation so the approval enforcement is restored.
In `@src/webhooks/handlers/issue_comment.py`:
- Around line 35-39: The current bot guard uses substring checks against
bot_usernames causing false positives; change the logic in the issue comment
handler where bot_usernames and commenter are used to perform exact,
case-insensitive matching instead (e.g., build a set of lowercased bot_usernames
and compare commenter.strip().lower() with that set) and return the same ignored
response when there is an exact match; update the conditional that now uses
any(bot_name.lower() in commenter.lower() ...) to use membership against the
normalized set instead.
In `@tests/conftest.py`:
- Around line 37-46: Replace the literal base64 private key in the
Helpers.mock_env call by using a clearly non-secret test placeholder (e.g., set
PRIVATE_KEY_BASE64_GITHUB to "TEST_ONLY_BASE64" or "BASE64_PLACEHOLDER") so
scanners won't flag it, and add a short inline comment next to the
Helpers.mock_env invocation indicating this value is test-only and safe to
allowlist; update any tests that depend on that exact value to decode or
generate their own mock key at runtime if needed.
- Around line 54-61: The event_loop fixture creates a new asyncio event loop but
does not set it as the current loop, causing libraries that call
asyncio.get_event_loop() to fail; modify the event_loop fixture (the function
named event_loop that uses asyncio.get_event_loop_policy() and loop =
policy.new_event_loop()) to call asyncio.set_event_loop(loop) after creating the
loop and before yielding, and ensure the loop is closed after yield (you may
optionally restore the previous event loop if desired).
🧹 Nitpick comments (11)
src/integrations/providers/bedrock_provider.py (3)
151-167: Movemodel_lowercomputation outside the loop.
model_id.lower()is computed on every iteration butmodel_idnever changes within the loop. Move it before the loop to avoid redundant string operations.♻️ Suggested improvement
+ model_lower = model_id.lower() for profile in profiles: profile_name = profile.get("name", "").lower() profile_arn = profile.get("arn", "") - model_lower = model_id.lower()
233-235: UseAIMessageinstead ofBaseMessagefor response.Creating
BaseMessage(content=content, type="assistant")directly is non-idiomatic. LangChain providesAIMessagespecifically for assistant responses, which ensures proper type handling throughout the framework.♻️ Suggested fix
+from langchain_core.messages import AIMessage ... - message = BaseMessage(content=content, type="assistant") + message = AIMessage(content=content)
40-45: Bare exception handler hides root cause of failures.Catching all
Exceptiontypes and silently falling back (lines 42-45) makes it difficult to diagnose why the standard client failed. Consider logging the exception or narrowing the catch to expected exception types.♻️ Suggested improvement
try: return self._get_standard_bedrock_client() - except Exception: + except Exception as e: # If standard client fails, fall back to Anthropic client + import logging + logging.debug(f"Standard Bedrock client failed, falling back to Anthropic client: {e}") client = self._get_anthropic_bedrock_client() return self._wrap_anthropic_client(client, model_id)src/rules/validators.py (2)
168-182: Prefer the shared glob matcher to avoid fragile regex conversion.
_glob_to_regexonly escapes a subset of regex metacharacters and doesn’t support**semantics;_matches_anyalready handles variants and caching.♻️ Suggested refactor
- regex_pattern = FilePatternCondition._glob_to_regex(pattern) - - # Pattern match—performance: optimize if file count high. - matching_files = [file for file in changed_files if re.match(regex_pattern, file)] + # Pattern match—reuse shared glob matcher with ** support. + matching_files = [file for file in changed_files if _matches_any(file, [pattern])]Also applies to: 202-206
246-260: Consider making weekend evaluation timezone-aware.
datetime.now()uses server-local time; using UTC or a supplied timezone would avoid inconsistent behavior across deployments.src/api/rules.py (1)
14-27: Consider adding error handling for agent execution.The new
feedbackfield is a good addition that provides valuable information to the frontend. However, the comment on line 19 notes the agent may throw if the rule is malformed, but there's no try/except to handle this gracefully.While FastAPI will return a 500 error, a more user-friendly response might be beneficial.
♻️ Suggested error handling
`@router.post`("/rules/evaluate") async def evaluate_rule(request: RuleEvaluationRequest): # Agent: uses central config—change here affects all rule evals. agent = get_agent("feasibility") - # Async call—agent may throw if rule malformed. - result = await agent.execute(rule_description=request.rule_text) + try: + result = await agent.execute(rule_description=request.rule_text) + except Exception as e: + return { + "supported": False, + "snippet": "", + "feedback": f"Rule evaluation failed: {str(e)}", + } # Output: keep format stable for frontend. Brittle if agent changes keys. return { "supported": result.data.get("is_feasible", False), "snippet": result.data.get("yaml_content", ""), "feedback": result.message, }src/integrations/github/api.py (1)
22-142: Avoid silent auth fallback when explicit auth was requested.When
installation_idoruser_tokenis provided but token retrieval fails, the method should returnNonerather than silently falling back to anonymous access. This masks auth failures and consumes the public rate limit (60 req/hr) instead of authenticated limits.Suggested fix
- if allow_anonymous: + if allow_anonymous and installation_id is None and not user_token: # Public access (Subject to 60 req/hr rate limit per IP) return {"Accept": accept, "User-Agent": "Watchflow-Analyzer/1.0"}src/api/dependencies.py (1)
8-9: Remove duplicate logger initialization.
Line 8 duplicates Line 9; keeping a single assignment avoids confusion and accidental divergence later.♻️ Suggested cleanup
-logger = logging.getLogger(__name__) # Logger: keep at module level for reuse. -logger = logging.getLogger(__name__) +logger = logging.getLogger(__name__) # Logger: keep at module level for reuse.src/api/rate_limit.py (1)
12-39: In-memory limiter is per-process and unbounded.
This will diverge across workers and can grow without bound under many unique IPs. Consider Redis (or another shared cache) with TTLs or periodic eviction to keep limits consistent and memory bounded.src/api/recommendations.py (2)
21-76: Repo URL schema and parser are out of sync.
AnalyzeRepoRequest.repo_urlisHttpUrl, so shorthandowner/reponever reachesparse_repo_from_url, even though the parser explicitly supports it. Either remove the shorthand branch or relax the schema and validate inparse_repo_from_url.
157-191: Prefer a request model for the stub endpoint.
Using a plaindictloses schema validation and OpenAPI clarity. A small Pydantic model would make this endpoint self-documenting and safer to evolve.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
.pre-commit-config.yamlREADME.mddocs/features.mdlinting_commands.txtpyproject.tomlsrc/__init__.pysrc/agents/repository_analysis_agent/agent.pysrc/agents/repository_analysis_agent/models.pysrc/agents/repository_analysis_agent/nodes.pysrc/agents/repository_analysis_agent/prompts.pysrc/api/dependencies.pysrc/api/rate_limit.pysrc/api/recommendations.pysrc/api/rules.pysrc/core/config/settings.pysrc/core/models.pysrc/core/utils/caching.pysrc/core/utils/logging.pysrc/integrations/github/api.pysrc/integrations/github/service.pysrc/integrations/providers/bedrock_provider.pysrc/integrations/providers/factory.pysrc/main.pysrc/rules/utils/contributors.pysrc/rules/validators.pysrc/tasks/scheduler/deployment_scheduler.pysrc/tasks/task_queue.pysrc/webhooks/auth.pysrc/webhooks/dispatcher.pysrc/webhooks/handlers/deployment.pysrc/webhooks/handlers/deployment_status.pysrc/webhooks/handlers/issue_comment.pysrc/webhooks/handlers/pull_request.pysrc/webhooks/router.pytests/conftest.pytests/integration/test_recommendations.pytests/integration/test_rules_api.py
🧰 Additional context used
🧬 Code graph analysis (12)
src/webhooks/dispatcher.py (9)
src/webhooks/handlers/deployment.py (1)
handle(17-46)src/webhooks/handlers/deployment_status.py (1)
handle(17-50)src/webhooks/handlers/issue_comment.py (2)
handle(25-147)event_type(19-20)src/webhooks/handlers/pull_request.py (1)
handle(18-45)src/webhooks/handlers/base.py (1)
handle(16-26)src/webhooks/handlers/deployment_protection_rule.py (1)
handle(16-33)src/webhooks/handlers/check_run.py (1)
handle(16-29)src/webhooks/handlers/deployment_review.py (1)
handle(16-33)src/webhooks/handlers/push.py (1)
handle(16-29)
src/api/rate_limit.py (1)
src/core/models.py (1)
User(7-18)
src/tasks/task_queue.py (2)
src/webhooks/handlers/issue_comment.py (1)
event_type(19-20)src/core/models.py (1)
repo_full_name(56-58)
src/api/dependencies.py (2)
src/core/models.py (1)
User(7-18)src/integrations/github/service.py (1)
GitHubService(19-115)
src/webhooks/router.py (2)
src/webhooks/handlers/issue_comment.py (1)
event_type(19-20)src/core/models.py (1)
EventType(24-38)
src/api/rules.py (3)
src/agents/factory.py (1)
get_agent(20-52)src/agents/base.py (1)
execute(111-113)src/agents/feasibility_agent/agent.py (1)
execute(57-123)
src/webhooks/handlers/deployment.py (2)
src/core/utils/caching.py (1)
get(44-66)src/core/models.py (1)
repo_full_name(56-58)
src/integrations/providers/factory.py (1)
src/core/config/provider_config.py (2)
get_max_tokens_for_agent(57-63)get_temperature_for_agent(65-71)
src/api/recommendations.py (6)
src/agents/repository_analysis_agent/test_agent.py (1)
agent(18-20)src/agents/repository_analysis_agent/agent.py (2)
RepositoryAnalysisAgent(13-62)execute(39-62)src/agents/repository_analysis_agent/models.py (1)
RuleRecommendation(68-78)src/api/dependencies.py (1)
get_current_user_optional(24-43)src/api/rate_limit.py (1)
rate_limiter(20-40)src/core/models.py (2)
User(7-18)repo_full_name(56-58)
src/agents/repository_analysis_agent/agent.py (4)
src/agents/base.py (4)
AgentResult(16-29)BaseAgent(32-113)_build_graph(55-57)execute(111-113)src/agents/repository_analysis_agent/models.py (1)
AnalysisState(81-106)src/agents/repository_analysis_agent/nodes.py (1)
fetch_repository_metadata(13-104)src/core/models.py (1)
repo_full_name(56-58)
src/agents/repository_analysis_agent/models.py (1)
src/core/models.py (1)
repo_full_name(56-58)
src/integrations/github/api.py (2)
src/core/models.py (1)
repo_full_name(56-58)src/core/utils/caching.py (1)
get(44-66)
🪛 Gitleaks (8.30.0)
tests/conftest.py
[high] 42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/integrations/github/api.py
[high] 103-103: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 119-119: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (49)
src/integrations/providers/factory.py (2)
8-9: LGTM!Adding
from __future__ import annotationsis good practice for modern Python codebases—it enables postponed evaluation of annotations and aligns well with the existing use of Python 3.10+ type hint syntax throughout the file.
78-79: LGTM!The inline ternary expressions correctly use
is not Nonechecks, which preserves the ability to explicitly pass0values formax_tokensortemperature. The fallback logic to agent-specific and then global config is cleanly delegated to the helper methods.src/core/utils/logging.py (2)
8-18: LGTM!Clean adoption of modern Python typing patterns. Using
from __future__ import annotationswithTYPE_CHECKINGforCallablecorrectly avoids runtime import overhead while preserving full type-checker support. Consolidatinginspectat module level is idiomatic.
148-149: LGTM!The comment accurately explains why this pattern is safe. With postponed annotations enabled, the
Callable[..., Any]hint remains a string at runtime, avoiding anyNameError.src/rules/utils/contributors.py (1)
61-65: LGTM! Good refactoring.The combined condition is correct and preserves Python's short-circuit evaluation, ensuring the async
_has_recent_activitycall is skipped whencontributions < min_contributions. This is both cleaner and more efficient.src/core/utils/caching.py (2)
169-171: LGTM: key generation simplification.
Clear and concise.
173-175: LGTM: unified cache get path.
Consolidation is clean and reduces branching.src/rules/validators.py (1)
82-91: LGTM — metadata note improves clarity.src/webhooks/handlers/pull_request.py (1)
22-23: Comment update looks good.No behavior changes; the note adds helpful context.
src/__init__.py (1)
1-1: LGTM.Clearer, concise module note.
src/tasks/scheduler/deployment_scheduler.py (1)
40-47: Cancellation handling is clean and idiomatic.Using
contextlib.suppressmakes the intent clear.linting_commands.txt (1)
1-9: Nice, concise linting checklist.This is straightforward and actionable.
docs/features.md (1)
279-286: Documentation update is clear and aligned with behavior.Nice addition for user expectations.
src/webhooks/handlers/deployment.py (1)
27-36: Helpful inline notes.Good to document fragility/async enqueue caveats.
src/webhooks/auth.py (1)
32-45: LGTM!The clarified comments improve code readability and correctly document:
- Raw bytes usage for signature verification
- HMAC-SHA256 as the GitHub standard
- Constant-time comparison for timing attack prevention
The implementation is correct and follows GitHub webhook security best practices.
src/webhooks/handlers/deployment_status.py (1)
27-42: LGTM!The updated comments accurately document the brittleness of relying on GitHub's payload structure and the async nature of the enqueue operation. The handler logic is sound with proper error handling for missing installation ID.
.pre-commit-config.yaml (2)
6-14: LGTM on pre-commit-hooks update.Good additions:
- Bumping to v6.0.0
- Adding
check-added-large-fileshook to prevent accidentally committing large files
26-31: LGTM on conventional-pre-commit update.The bump to v4.3.0 keeps the conventional commit enforcement up to date.
src/agents/repository_analysis_agent/prompts.py (1)
3-35: LGTM — prompt split is clear and role‑appropriate.
Clean separation of system vs. user context.src/webhooks/handlers/issue_comment.py (3)
43-143: Command flow and no‑match guard look good.
The help/ack/eval/validate branching is clear and the final ignore path reduces noise.
155-169: Regex variants + empty‑reason guard are solid.
193-194: any() matcher is clean and readable.src/core/models.py (2)
24-38: EventType expansion looks good.
41-63: WebhookEvent attribute initialization is clear.src/integrations/github/service.py (2)
7-30: Service skeleton and custom exceptions look good.
107-115: URL parsing helper is straightforward.src/integrations/github/api.py (3)
230-252: Helper extraction and error handling are clear.
268-290: Head‑SHA based check lookup is solid.
297-309: Using config base URL here is consistent.src/api/dependencies.py (2)
14-18: DI factory looks good.
Simple, test-friendly service wiring.
46-54: Strict auth dependency is clear.
The required-auth guard is concise and aligned with the optional dependency.src/webhooks/router.py (1)
13-34: Comment-only tweaks; no functional changes to review.Also applies to: 53-65
src/webhooks/dispatcher.py (1)
15-17: Comment-only edits; nothing to review.Also applies to: 52-60
src/agents/repository_analysis_agent/models.py (2)
8-19: Normalization helper looks good.
Simple, deterministic cleanup for URL-derived identifiers.
42-106: Model shape updates look consistent.
The new features/response/state structures read clearly and align with the refactor goals.src/agents/repository_analysis_agent/agent.py (2)
22-37: Graph wiring is clear and easy to follow.
39-62: Execution flow and state rehydration look solid.tests/integration/test_rules_api.py (1)
28-28: Conditional simplification preserves behavior.Also applies to: 71-71, 106-106
tests/integration/test_recommendations.py (4)
13-32: Mock response is deterministic and readable.
35-79: Public-repo integration scenario is well covered.
81-102: Anonymous private-repo fallback path is covered.
104-145: Authenticated private-repo path is covered.tests/conftest.py (1)
18-30: Helper-based env mocking is tidy and reusable.src/main.py (4)
16-18: Import wrapping looks fine.
101-104: Router prefix wiring looks consistent.
121-135: Task status aggregation looks good.
34-65: Verify scheduler start is non-blocking.
await get_deployment_scheduler().start()will block startup ifstart()runs the scheduler loop. Please confirm it returns quickly or move it back to a background task.src/api/recommendations.py (2)
82-89: Confirm auth-aware rate limiting is applied.
Depends(rate_limiter)doesn’t pass the authenticateduser, so the limiter may treat all requests as anonymous. If user-based limits are intended, wireget_current_user_optionalintorate_limiteritself or pass user context explicitly.
108-154: End-to-end mapping toAnalysisResponseis clear.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| ## Unauthenticated Analysis & Rate Limiting | ||
|
|
||
| - The repository analysis endpoint `/v1/rules/recommend` now supports unauthenticated access for public GitHub repositories. | ||
| - Anonymous users are limited to 5 requests per hour per IP. Authenticated users are limited to 100 requests per hour. | ||
| - Exceeding the limit returns a 429 error with a `Retry-After` header. | ||
| - For private repositories, authentication is required. | ||
| - The frontend is now fully connected to the backend and no longer uses mock data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the recommend endpoint definition in the API code
rg -n "rules/recommend|/recommend" --type=py -C3Repository: warestack/watchflow
Length of output: 3906
🏁 Script executed:
#!/bin/bash
# Find where the recommendations router is mounted
rg -n "include_router|recommendations|APIRouter" --type=py src/api/ -B2 -A2 | head -50Repository: warestack/watchflow
Length of output: 2253
🏁 Script executed:
#!/bin/bash
# Find the main app configuration
fd -n "main.py|app.py" -t f | head -5
# Then search for include_router calls
rg -n "include_router" --type=pyRepository: warestack/watchflow
Length of output: 719
Update the endpoint path in the documentation to match the actual API.
The endpoint in the README should be /api/v1/rules/recommend, not /v1/rules/recommend. The full path includes the /api prefix, as confirmed by the router mounting in src/main.py (line 103) and the integration tests.
The rest of the documentation is clear and accurate—rate limits, authentication requirements, and the 429 response handling are all correctly described.
🤖 Prompt for AI Agents
In `@README.md` around lines 238 - 245, The README incorrectly lists the
repository analysis endpoint as /v1/rules/recommend; update the documentation to
use the actual mounted path /api/v1/rules/recommend. Edit the README.md entry
under "Unauthenticated Analysis & Rate Limiting" to replace
`/v1/rules/recommend` with `/api/v1/rules/recommend` (consistent with the API
router mount via app.include_router and the integration tests), and verify any
other README references to the endpoint include the /api prefix.
| def __init__(self): | ||
| # We use 'repository_analysis' to look up config like max_tokens | ||
| super().__init__(agent_name="repository_analysis") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve BaseAgent configuration parameters in the constructor.
The new __init__ drops support for BaseAgent kwargs (e.g., max_retries, retry_delay), which will break existing call sites that pass them.
🔧 Proposed fix
- def __init__(self):
- # We use 'repository_analysis' to look up config like max_tokens
- super().__init__(agent_name="repository_analysis")
+ def __init__(self, **kwargs):
+ # We use 'repository_analysis' to look up config like max_tokens
+ super().__init__(agent_name="repository_analysis", **kwargs)🤖 Prompt for AI Agents
In `@src/agents/repository_analysis_agent/agent.py` around lines 18 - 21, The
constructor for repository_analysis agent (__init__) currently hardcodes
super().__init__(agent_name="repository_analysis") and drops BaseAgent kwargs;
update __init__ to accept and forward parameter kwargs (e.g., **kwargs or
explicit max_retries, retry_delay) and pass them into super().__init__ along
with agent_name="repository_analysis" so existing callers' configuration
(max_retries, retry_delay, etc.) is preserved.
| class RepositoryAnalysisRequest(BaseModel): | ||
| """ | ||
| Input request for analyzing a repository. | ||
| Automatically normalizes the repository URL into a full name. | ||
| """ | ||
|
|
||
| class RuleRecommendation(BaseModel): | ||
| """A recommended Watchflow rule with confidence and reasoning.""" | ||
| repository_full_name: str = Field(default="", description="GitHub repository in 'owner/repo' format") | ||
| repository_url: str = Field(default="", description="Full GitHub repository URL (optional, will be normalized)") | ||
| installation_id: int | None = Field( | ||
| default=None, description="GitHub App installation ID for authenticated requests" | ||
| ) | ||
|
|
||
| yaml_rule: str = Field(description="Valid Watchflow rule YAML content") | ||
| confidence: float = Field(description="Confidence score (0.0-1.0)", ge=0.0, le=1.0) | ||
| reasoning: str = Field(description="Short explanation of why this rule is recommended") | ||
| strategy_used: str = Field(description="Strategy used (static, hybrid, llm)") | ||
| @model_validator(mode="after") | ||
| def normalize_repo_name(self) -> "RepositoryAnalysisRequest": | ||
| """Normalize repository URL to full name format.""" | ||
| if not self.repository_full_name and self.repository_url: | ||
| self.repository_full_name = parse_github_repo_identifier(self.repository_url) | ||
| return self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Require a repository identifier to avoid late failures.
If both repository_full_name and repository_url are empty, the model currently accepts the request and downstream nodes raise a ValueError. Prefer a 422 at validation time.
✅ Suggested validation
@@
`@model_validator`(mode="after")
def normalize_repo_name(self) -> "RepositoryAnalysisRequest":
"""Normalize repository URL to full name format."""
if not self.repository_full_name and self.repository_url:
self.repository_full_name = parse_github_repo_identifier(self.repository_url)
+ if not self.repository_full_name:
+ raise ValueError("repository_full_name or repository_url is required")
return self🤖 Prompt for AI Agents
In `@src/agents/repository_analysis_agent/models.py` around lines 22 - 39, The
model currently lets both repository_full_name and repository_url be empty;
update RepositoryAnalysisRequest.normalize_repo_name to validate early: after
trying to normalize (using parse_github_repo_identifier when repository_url is
provided and repository_full_name is empty), if repository_full_name is still
empty then raise a validation error (e.g., raise ValueError("repository
identifier required: provide repository_full_name or repository_url")) so
Pydantic returns a 422 instead of allowing downstream ValueError.
| # 1. Fetch File Tree (Root) | ||
| try: | ||
| files = await github_client.list_directory_any_auth(repo_full_name=repo, path="") | ||
| except Exception as e: | ||
| logger.error(f"Failed to fetch file tree for {repo}: {e}") | ||
| files = [] | ||
|
|
||
| file_names = [f["name"] for f in files] if files else [] | ||
|
|
||
| # 2. Heuristic Language Detection | ||
| languages = [] | ||
| if "pom.xml" in file_names: | ||
| languages.append("Java") | ||
| if "package.json" in file_names: | ||
| languages.append("JavaScript/TypeScript") | ||
| if "requirements.txt" in file_names or "pyproject.toml" in file_names: | ||
| languages.append("Python") | ||
| if "go.mod" in file_names: | ||
| languages.append("Go") | ||
| if "Cargo.toml" in file_names: | ||
| languages.append("Rust") | ||
|
|
||
| # 3. Check for CI/CD presence | ||
| has_ci = ".github" in file_names | ||
|
|
||
| # 4. Fetch Documentation Snippets (for Context) | ||
| readme_content = "" | ||
| target_files = ["README.md", "readme.md", "CONTRIBUTING.md"] | ||
| for target in target_files: | ||
| if target in file_names: | ||
| content = await github_client.get_file_content(repo_full_name=repo, file_path=target, installation_id=None) | ||
| if content: | ||
| readme_content = content[:2000] | ||
| break | ||
|
|
||
| # 5. CODEOWNERS detection (root, .github/, docs/) | ||
| codeowners_paths = ["CODEOWNERS", ".github/CODEOWNERS", "docs/CODEOWNERS"] | ||
| has_codeowners = False | ||
| for copath in codeowners_paths: | ||
| try: | ||
| co_content = await github_client.get_file_content( | ||
| repo_full_name=repo, file_path=copath, installation_id=None | ||
| ) | ||
| if co_content and len(co_content.strip()) > 0: | ||
| has_codeowners = True | ||
| break | ||
| except Exception: | ||
| continue | ||
|
|
||
| # 6. Analyze workflows for CI patterns | ||
| workflow_patterns = [] | ||
| try: | ||
| workflow_files = await github_client.list_directory_any_auth(repo_full_name=repo, path=".github/workflows") | ||
| for wf in workflow_files: | ||
| wf_name = wf["name"] | ||
| if wf_name.endswith(".yml") or wf_name.endswith(".yaml"): | ||
| content = await github_client.get_file_content( | ||
| repo_full_name=repo, file_path=f".github/workflows/{wf_name}", installation_id=None | ||
| ) | ||
| if content: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, explore the directory structure
find src -type f -name "*.py" | grep -E "(github|client)" | head -20Repository: warestack/watchflow
Length of output: 232
🏁 Script executed:
# Search for the github_client implementation and method signatures
rg -n "def list_directory_any_auth|def get_file_content" -A 3 src/Repository: warestack/watchflow
Length of output: 781
🏁 Script executed:
# Check AnalysisState definition
rg -n "class AnalysisState" -A 15 src/agents/Repository: warestack/watchflow
Length of output: 1392
🏁 Script executed:
# Look for github_client initialization and usage
rg -n "github_client\s*=" src/agents/repository_analysis_agent/Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check imports in the nodes.py file
head -25 src/agents/repository_analysis_agent/nodes.pyRepository: warestack/watchflow
Length of output: 949
🏁 Script executed:
# Get full implementation of github_client methods
cat -n src/integrations/github/api.py | sed -n '114,160p'Repository: warestack/watchflow
Length of output: 2583
🏁 Script executed:
# Check if AnalysisState has user_token or other auth fields
cat -n src/agents/repository_analysis_agent/models.py | sed -n '81,120p'Repository: warestack/watchflow
Length of output: 1131
🏁 Script executed:
# Look for user_token usage in the repository
rg -n "user_token" src/agents/repository_analysis_agent/Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Find and inspect _get_auth_headers implementation
rg -n "_get_auth_headers" -A 20 src/integrations/github/api.py | head -60Repository: warestack/watchflow
Length of output: 2863
🏁 Script executed:
# Check if fetch_repository_metadata uses state.is_public anywhere
rg -n "is_public" src/agents/repository_analysis_agent/nodes.pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Look for where user_token might be passed to fetch_repository_metadata
rg -n "fetch_repository_metadata" -B 5 -A 5 src/agents/Repository: warestack/watchflow
Length of output: 1933
Private repositories will fail silently without auth propagation.
The current implementation calls list_directory_any_auth and get_file_content with installation_id=None and no user_token, falling back to anonymous access via allow_anonymous=True. While this works for public repositories, private repos will return empty results (HTTP 401/403) without proper authentication.
AnalysisState tracks is_public but never uses it to determine auth requirements. Neither installation_id nor user_token is carried through the state, and fetch_repository_metadata has no mechanism to access them. The github_client API supports both auth methods (see _get_auth_headers priority: user_token → installation_id → anonymous), but the node fails to utilize them.
Pass user_token and/or installation_id through AnalysisState and update fetch_repository_metadata to use these fields when analyzing private repositories.
🤖 Prompt for AI Agents
In `@src/agents/repository_analysis_agent/nodes.py` around lines 24 - 83, The node
silently fails for private repos because fetch_repository_metadata and this code
path call github_client.list_directory_any_auth and
github_client.get_file_content with installation_id=None (and no user_token),
ignoring AnalysisState.is_public and lacking auth info; update AnalysisState to
include installation_id and user_token, propagate those fields into
fetch_repository_metadata and into this node, and pass the appropriate
installation_id and/or user_token into github_client.list_directory_any_auth and
github_client.get_file_content calls (prefer user_token when present, fallback
to installation_id) so private repos are accessed with proper auth instead of
anonymous.
| class RecommendationsList(AnalysisState): | ||
| # We strictly want the list, reusing the model definition | ||
| recommendations: list[RuleRecommendation] | ||
|
|
||
| yaml_content = f"""description: "Require tests when code changes" | ||
| enabled: true | ||
| severity: medium | ||
| event_types: | ||
| - pull_request | ||
| parameters: | ||
| source_patterns: | ||
| {source_patterns_yaml} | ||
| test_patterns: | ||
| {test_patterns_yaml} | ||
| """ | ||
| structured_llm = llm.with_structured_output(RecommendationsList) | ||
|
|
||
| confidence = 0.74 | ||
| if pr_issues.get("missing_tests", 0) > 0: | ||
| confidence = 0.85 | ||
| if state.contributing_analysis.content and state.contributing_analysis.requires_tests: | ||
| confidence = min(0.95, confidence + 0.1) | ||
|
|
||
| recommendations.append( | ||
| RuleRecommendation( | ||
| yaml_rule=yaml_content.strip(), | ||
| confidence=confidence, | ||
| reasoning=test_reasoning, | ||
| strategy_used="hybrid", | ||
| response = await structured_llm.ainvoke( | ||
| [SystemMessage(content=REPOSITORY_ANALYSIS_SYSTEM_PROMPT), HumanMessage(content=user_prompt)] | ||
| ) | ||
| ) | ||
|
|
||
| # Require description in PR body. | ||
| # Increase confidence if we detect short titles in PR history (indicator of missing context) | ||
| desc_reasoning = f"Repository analysis for {state.repository_full_name}." | ||
| if pr_issues.get("short_titles", 0) > 0: | ||
| desc_reasoning += f" Detected {pr_issues['short_titles']} PRs with very short titles (likely missing context)." | ||
| else: | ||
| desc_reasoning += " Encourages context for reviewers; lightweight default." | ||
|
|
||
| desc_confidence = 0.68 | ||
| if pr_issues.get("short_titles", 0) > 0: | ||
| desc_confidence = 0.80 | ||
|
|
||
| recommendations.append( | ||
| RuleRecommendation( | ||
| yaml_rule="""description: "Ensure PRs include context" | ||
| enabled: true | ||
| severity: low | ||
| event_types: | ||
| - pull_request | ||
| parameters: | ||
| min_description_length: 50 | ||
| """.strip(), | ||
| confidence=desc_confidence, | ||
| reasoning=desc_reasoning, | ||
| strategy_used="static", | ||
| # The response is already a Pydantic object (RecommendationsList or similar) | ||
| # We extract the list of recommendations | ||
| valid_recs = response.recommendations if hasattr(response, "recommendations") else [] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -e py -t f nodes.pyRepository: warestack/watchflow
Length of output: 179
🏁 Script executed:
cat -n src/agents/repository_analysis_agent/nodes.py | head -200Repository: warestack/watchflow
Length of output: 7977
🏁 Script executed:
rg -n "class AnalysisState" -A 15Repository: warestack/watchflow
Length of output: 1392
🏁 Script executed:
rg -n "RecommendationsList" -B 5 -A 10Repository: warestack/watchflow
Length of output: 2588
Structured output model will fail validation due to inherited required fields.
RecommendationsList(AnalysisState) inherits repo_full_name (a required field with no default), but the LLM is only asked to generate recommendations. The LLM's response will fail schema validation and trigger the fallback path. Define RecommendationsList as a minimal Pydantic model instead.
Suggested fix
-from langchain_core.messages import HumanMessage, SystemMessage
+from langchain_core.messages import HumanMessage, SystemMessage
+from pydantic import BaseModel
@@
- class RecommendationsList(AnalysisState):
- # We strictly want the list, reusing the model definition
- recommendations: list[RuleRecommendation]
+ class RecommendationsList(BaseModel):
+ recommendations: list[RuleRecommendation]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class RecommendationsList(AnalysisState): | |
| # We strictly want the list, reusing the model definition | |
| recommendations: list[RuleRecommendation] | |
| yaml_content = f"""description: "Require tests when code changes" | |
| enabled: true | |
| severity: medium | |
| event_types: | |
| - pull_request | |
| parameters: | |
| source_patterns: | |
| {source_patterns_yaml} | |
| test_patterns: | |
| {test_patterns_yaml} | |
| """ | |
| structured_llm = llm.with_structured_output(RecommendationsList) | |
| confidence = 0.74 | |
| if pr_issues.get("missing_tests", 0) > 0: | |
| confidence = 0.85 | |
| if state.contributing_analysis.content and state.contributing_analysis.requires_tests: | |
| confidence = min(0.95, confidence + 0.1) | |
| recommendations.append( | |
| RuleRecommendation( | |
| yaml_rule=yaml_content.strip(), | |
| confidence=confidence, | |
| reasoning=test_reasoning, | |
| strategy_used="hybrid", | |
| response = await structured_llm.ainvoke( | |
| [SystemMessage(content=REPOSITORY_ANALYSIS_SYSTEM_PROMPT), HumanMessage(content=user_prompt)] | |
| ) | |
| ) | |
| # Require description in PR body. | |
| # Increase confidence if we detect short titles in PR history (indicator of missing context) | |
| desc_reasoning = f"Repository analysis for {state.repository_full_name}." | |
| if pr_issues.get("short_titles", 0) > 0: | |
| desc_reasoning += f" Detected {pr_issues['short_titles']} PRs with very short titles (likely missing context)." | |
| else: | |
| desc_reasoning += " Encourages context for reviewers; lightweight default." | |
| desc_confidence = 0.68 | |
| if pr_issues.get("short_titles", 0) > 0: | |
| desc_confidence = 0.80 | |
| recommendations.append( | |
| RuleRecommendation( | |
| yaml_rule="""description: "Ensure PRs include context" | |
| enabled: true | |
| severity: low | |
| event_types: | |
| - pull_request | |
| parameters: | |
| min_description_length: 50 | |
| """.strip(), | |
| confidence=desc_confidence, | |
| reasoning=desc_reasoning, | |
| strategy_used="static", | |
| # The response is already a Pydantic object (RecommendationsList or similar) | |
| # We extract the list of recommendations | |
| valid_recs = response.recommendations if hasattr(response, "recommendations") else [] | |
| from pydantic import BaseModel | |
| class RecommendationsList(BaseModel): | |
| recommendations: list[RuleRecommendation] | |
| structured_llm = llm.with_structured_output(RecommendationsList) | |
| response = await structured_llm.ainvoke( | |
| [SystemMessage(content=REPOSITORY_ANALYSIS_SYSTEM_PROMPT), HumanMessage(content=user_prompt)] | |
| ) | |
| # The response is already a Pydantic object (RecommendationsList or similar) | |
| # We extract the list of recommendations | |
| valid_recs = response.recommendations if hasattr(response, "recommendations") else [] |
🤖 Prompt for AI Agents
In `@src/agents/repository_analysis_agent/nodes.py` around lines 138 - 151, The
current RecommendationsList class inherits AnalysisState (which requires
repo_full_name) causing Pydantic validation to fail; replace it with a minimal
Pydantic model that only defines recommendations: list[RuleRecommendation] (do
not inherit AnalysisState) and use that model with structured_llm =
llm.with_structured_output(RecommendationsList) so the LLM output validates
against the simple schema; update any imports if needed to reference
pydantic.BaseModel (or your project's lightweight model base) and ensure
response.recommendations access remains the same.
| # We explicitly allow all origins ("*") to prevent the browser from blocking requests | ||
| # from your local file system or different localhost ports. | ||
|
|
||
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=config.cors.origins, | ||
| allow_origins=["*"], # Explicitly allow all for development | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=config.cors.headers, | ||
| allow_headers=["*"], | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, check if src/main.py exists and read lines around 88-97
wc -l src/main.pyRepository: warestack/watchflow
Length of output: 78
🏁 Script executed:
cat -n src/main.py | sed -n '85,100p'Repository: warestack/watchflow
Length of output: 560
🏁 Script executed:
# Search for any environment-based conditionals around the CORS middleware setup
rg -A 10 -B 10 "CORSMiddleware" src/main.pyRepository: warestack/watchflow
Length of output: 1364
🏁 Script executed:
# Search for any environment variables or configuration checks
rg "environ|getenv|ENV|env\." src/main.py | head -20Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there's any conditional logic before the CORS setup
rg -B 30 "CORSMiddleware" src/main.py | grep -E "if|environ|getenv|dev|prod|DEBUG"Repository: warestack/watchflow
Length of output: 145
🏁 Script executed:
# Search for environment variable usage and configuration throughout the file
rg "^import|^from" src/main.py | head -30Repository: warestack/watchflow
Length of output: 1208
🏁 Script executed:
# Look for any config files that might conditionally apply CORS settings
fd -t f "\.(py|env|toml|yaml|json)$" | grep -E "config|settings|env" | head -20Repository: warestack/watchflow
Length of output: 318
🏁 Script executed:
# Check if there's any conditional middleware addition based on environment
rg "add_middleware" src/main.pyRepository: warestack/watchflow
Length of output: 82
🏁 Script executed:
# Check the CORS config file to see if there's environment-based gating
cat -n src/core/config/cors_config.pyRepository: warestack/watchflow
Length of output: 324
🏁 Script executed:
# Check the settings file for environment detection
cat -n src/core/config/settings.pyRepository: warestack/watchflow
Length of output: 7419
🏁 Script executed:
# Look for environment variable definitions or usage throughout the config directory
rg "environ|getenv|ENV|dev|prod|environment" src/core/config/Repository: warestack/watchflow
Length of output: 4219
🏁 Script executed:
# Check if Config is imported or used in main.py
rg "Config|config|from src.core.config" src/main.pyRepository: warestack/watchflow
Length of output: 83
🏁 Script executed:
# Check the full main.py to see where app is created and if Config is initialized
cat -n src/main.py | head -50Repository: warestack/watchflow
Length of output: 2330
🏁 Script executed:
# Search for any other references to CORS in main.py or if Config is used elsewhere
rg -i "cors\|allow_credentials\|allow_origins" src/main.pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there are any deployment configs or conditional logic we might have missed
find . -name "*.py" -path "*/config/*" -type f | xargs wc -l | sort -rn | head -20Repository: warestack/watchflow
Length of output: 394
🏁 Script executed:
# Look for any environment checks or conditional CORS setup in the entire codebase
rg "allow_credentials.*True|allow_origins.*\[\"\*\"\]" --type pyRepository: warestack/watchflow
Length of output: 179
Replace hardcoded CORS with environment-based configuration.
The allow_origins=["*"] with allow_credentials=True bypasses the existing safe Config system (already defined in src/core/config/settings.py). Import Config from src.core.config.settings and use config.cors instead of hardcoded settings, which will respect environment variables CORS_ORIGINS and CORS_HEADERS with sensible defaults for production.
🤖 Prompt for AI Agents
In `@src/main.py` around lines 88 - 97, Replace the hardcoded CORS settings in the
app.add_middleware(CORSMiddleware, ...) call by importing and using the
application's Config object (e.g., import Config or the instantiated config) and
pass config.cors (instead of allow_origins=["*"]) and config.cors_headers
(instead of allow_headers=["*"]) to the CORSMiddleware parameters; keep
allow_credentials and allow_methods as needed but source origins/headers from
the config so environment variables (CORS_ORIGINS / CORS_HEADERS) are respected.
| async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool: | ||
| # Remove unused variable assignment | ||
| # Unused: min_approvals—left for future logic. | ||
| # min_approvals = parameters.get("min_approvals", 1) | ||
|
|
||
| # Placeholder logic - in production, this would check the actual PR reviews | ||
| # TODO: Check actual PR reviews. Always returns True—demo only. | ||
| return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ApprovalCountCondition now always passes, bypassing min-approval rules.
This disables enforcement for has_min_approvals. Please restore the approval count logic (or delegate to MinApprovalsCondition).
🐛 Suggested fix
async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool:
- # Unused: min_approvals—left for future logic.
- # min_approvals = parameters.get("min_approvals", 1)
-
- # TODO: Check actual PR reviews. Always returns True—demo only.
- return True
+ min_approvals = parameters.get("min_approvals", 1)
+ reviews = event.get("reviews", [])
+ approved_count = sum(1 for review in reviews if review.get("state") == "APPROVED")
+ logger.debug(
+ "ApprovalCountCondition: PR has %d approvals, requires %d",
+ approved_count,
+ min_approvals,
+ )
+ return approved_count >= min_approvals📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool: | |
| # Remove unused variable assignment | |
| # Unused: min_approvals—left for future logic. | |
| # min_approvals = parameters.get("min_approvals", 1) | |
| # Placeholder logic - in production, this would check the actual PR reviews | |
| # TODO: Check actual PR reviews. Always returns True—demo only. | |
| return True | |
| async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool: | |
| min_approvals = parameters.get("min_approvals", 1) | |
| reviews = event.get("reviews", []) | |
| approved_count = sum(1 for review in reviews if review.get("state") == "APPROVED") | |
| logger.debug( | |
| "ApprovalCountCondition: PR has %d approvals, requires %d", | |
| approved_count, | |
| min_approvals, | |
| ) | |
| return approved_count >= min_approvals |
🤖 Prompt for AI Agents
In `@src/rules/validators.py` around lines 238 - 243, The validate method
currently always returns True, bypassing the intended min-approval logic; update
the validate function in ApprovalCountCondition to read min_approvals from
parameters (e.g., parameters.get("min_approvals", 1")), obtain actual
review/approval count from the incoming event (or delegate to the existing
MinApprovalsCondition class/method), and return True only if the approval count
meets or exceeds min_approvals; ensure you reference and call
MinApprovalsCondition if you choose delegation so the approval enforcement is
restored.
| # Bot self-reply guard—avoids infinite loop, spam. | ||
| bot_usernames = ["watchflow[bot]", "watchflow-bot", "watchflow", "watchflowbot", "watchflow_bot"] | ||
| if commenter and any(bot_name.lower() in commenter.lower() for bot_name in bot_usernames): | ||
| logger.info(f"🤖 Ignoring comment from bot user: {commenter}") | ||
| return {"status": "ignored", "reason": "Bot comment"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use exact bot‑username matching to avoid false positives.
Substring checks can ignore legitimate users whose names contain “watchflow”.
🛠️ Suggested fix
- if commenter and any(bot_name.lower() in commenter.lower() for bot_name in bot_usernames):
+ if commenter and commenter.lower() in {name.lower() for name in bot_usernames}:🤖 Prompt for AI Agents
In `@src/webhooks/handlers/issue_comment.py` around lines 35 - 39, The current bot
guard uses substring checks against bot_usernames causing false positives;
change the logic in the issue comment handler where bot_usernames and commenter
are used to perform exact, case-insensitive matching instead (e.g., build a set
of lowercased bot_usernames and compare commenter.strip().lower() with that set)
and return the same ignored response when there is an exact match; update the
conditional that now uses any(bot_name.lower() in commenter.lower() ...) to use
membership against the normalized set instead.
| with Helpers.mock_env( | ||
| { | ||
| "APP_CLIENT_ID_GITHUB": "mock-client-id", | ||
| "APP_CLIENT_SECRET_GITHUB": "mock-client-secret", | ||
| "WEBHOOK_SECRET_GITHUB": "mock-webhook-secret", | ||
| "PRIVATE_KEY_BASE64_GITHUB": "bW9jay1rZXk=", # "mock-key" in base64 | ||
| "AI_PROVIDER": "openai", | ||
| "OPENAI_API_KEY": "sk-mock-key", | ||
| "ENVIRONMENT": "test", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secret scanner will likely flag the mock private key.
Gitleaks already reports Line 42 as a generic key; this will keep failing CI unless suppressed or allowlisted.
🛡️ Suggested suppression for test-only key
- "PRIVATE_KEY_BASE64_GITHUB": "bW9jay1rZXk=", # "mock-key" in base64
+ "PRIVATE_KEY_BASE64_GITHUB": "bW9jay1rZXk=", # "mock-key" in base64 # gitleaks:allow📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with Helpers.mock_env( | |
| { | |
| "APP_CLIENT_ID_GITHUB": "mock-client-id", | |
| "APP_CLIENT_SECRET_GITHUB": "mock-client-secret", | |
| "WEBHOOK_SECRET_GITHUB": "mock-webhook-secret", | |
| "PRIVATE_KEY_BASE64_GITHUB": "bW9jay1rZXk=", # "mock-key" in base64 | |
| "AI_PROVIDER": "openai", | |
| "OPENAI_API_KEY": "sk-mock-key", | |
| "ENVIRONMENT": "test", | |
| } | |
| with Helpers.mock_env( | |
| { | |
| "APP_CLIENT_ID_GITHUB": "mock-client-id", | |
| "APP_CLIENT_SECRET_GITHUB": "mock-client-secret", | |
| "WEBHOOK_SECRET_GITHUB": "mock-webhook-secret", | |
| "PRIVATE_KEY_BASE64_GITHUB": "bW9jay1rZXk=", # "mock-key" in base64 # gitleaks:allow | |
| "AI_PROVIDER": "openai", | |
| "OPENAI_API_KEY": "sk-mock-key", | |
| "ENVIRONMENT": "test", | |
| } |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In `@tests/conftest.py` around lines 37 - 46, Replace the literal base64 private
key in the Helpers.mock_env call by using a clearly non-secret test placeholder
(e.g., set PRIVATE_KEY_BASE64_GITHUB to "TEST_ONLY_BASE64" or
"BASE64_PLACEHOLDER") so scanners won't flag it, and add a short inline comment
next to the Helpers.mock_env invocation indicating this value is test-only and
safe to allowlist; update any tests that depend on that exact value to decode or
generate their own mock key at runtime if needed.
| @pytest.fixture(scope="session") | ||
| def event_loop(): | ||
| import asyncio | ||
|
|
||
| policy = asyncio.get_event_loop_policy() | ||
| loop = policy.new_event_loop() | ||
| yield loop | ||
| loop.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set the new event loop as the current event loop.
Without asyncio.set_event_loop(loop), libraries calling asyncio.get_event_loop() will fail to find the loop, resulting in RuntimeError or unexpected behavior in pytest-asyncio.
🔧 Proposed fix
def event_loop():
import asyncio
policy = asyncio.get_event_loop_policy()
loop = policy.new_event_loop()
+ asyncio.set_event_loop(loop)
yield loop
loop.close()🤖 Prompt for AI Agents
In `@tests/conftest.py` around lines 54 - 61, The event_loop fixture creates a new
asyncio event loop but does not set it as the current loop, causing libraries
that call asyncio.get_event_loop() to fail; modify the event_loop fixture (the
function named event_loop that uses asyncio.get_event_loop_policy() and loop =
policy.new_event_loop()) to call asyncio.set_event_loop(loop) after creating the
loop and before yielding, and ensure the loop is closed after yield (you may
optionally restore the previous event loop if desired).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Around line 103-111: The tool.uv.dev-dependencies list is missing the respx
package (it exists in project.optional-dependencies.dev), so add "respx" to the
tool.uv.dev-dependencies array to ensure uv sync installs it by default; update
the tool.uv.dev-dependencies entry (referencing the tool.uv.dev-dependencies
section) to include respx with the same version constraint used in
project.optional-dependencies.dev (or a compatible one) so local test runs that
use uv without --all-extras have respx available.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml
🔇 Additional comments (3)
pyproject.toml (3)
31-39: LGTM on respx addition and Ruff transition.The addition of
respx>=0.20.0for HTTP mocking aligns with the PR objective for CI compatibility. The comment clarifying that Ruff replaces black, isort, and flake8 is helpful for documentation.
44-77: Well-structured Ruff configuration.The configuration is comprehensive and the inline comments explaining each rule category are helpful for maintainability. The
B008ignore is correctly documented as necessary for FastAPI'sDepends()pattern.
79-89: Good strictness increase for type safety.The stricter mypy settings (
disallow_untyped_defs,disallow_incomplete_defs) align with the PR objective of enforcing strict Pydantic models and type hints. The exception fordisallow_untyped_decorators = falseis pragmatic given FastAPI and LangChain usage.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| [tool.uv] | ||
| dev-dependencies = [ | ||
| "pytest>=7.4.0", | ||
| "pytest-asyncio>=0.21.0", | ||
| "pytest-cov>=4.1.0", | ||
| "black>=23.0.0", | ||
| "isort>=5.12.0", | ||
| "flake8>=6.0.0", | ||
| "mypy>=1.7.0", | ||
| "pre-commit>=3.5.0", | ||
| "mkdocs-material>=9.5.0", | ||
| "mkdocs-git-revision-date-localized-plugin>=1.2.0", | ||
| "mkdocs-minify-plugin>=0.7.0", | ||
| "pymdown-extensions>=10.0", | ||
| "ruff>=0.1.0", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
respx is missing from tool.uv.dev-dependencies.
The respx package was added to project.optional-dependencies.dev (line 35) but not to tool.uv.dev-dependencies. If developers use uv sync without --all-extras, respx won't be installed, potentially causing test failures locally.
Proposed fix
[tool.uv]
dev-dependencies = [
"pytest>=7.4.0",
"pytest-asyncio>=0.21.0",
"pytest-cov>=4.1.0",
+ "respx>=0.20.0",
"mypy>=1.7.0",
"pre-commit>=3.5.0",
"ruff>=0.1.0",
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tool.uv] | |
| dev-dependencies = [ | |
| "pytest>=7.4.0", | |
| "pytest-asyncio>=0.21.0", | |
| "pytest-cov>=4.1.0", | |
| "black>=23.0.0", | |
| "isort>=5.12.0", | |
| "flake8>=6.0.0", | |
| "mypy>=1.7.0", | |
| "pre-commit>=3.5.0", | |
| "mkdocs-material>=9.5.0", | |
| "mkdocs-git-revision-date-localized-plugin>=1.2.0", | |
| "mkdocs-minify-plugin>=0.7.0", | |
| "pymdown-extensions>=10.0", | |
| "ruff>=0.1.0", | |
| ] | |
| [tool.uv] | |
| dev-dependencies = [ | |
| "pytest>=7.4.0", | |
| "pytest-asyncio>=0.21.0", | |
| "pytest-cov>=4.1.0", | |
| "respx>=0.20.0", | |
| "mypy>=1.7.0", | |
| "pre-commit>=3.5.0", | |
| "ruff>=0.1.0", | |
| ] |
🤖 Prompt for AI Agents
In `@pyproject.toml` around lines 103 - 111, The tool.uv.dev-dependencies list is
missing the respx package (it exists in project.optional-dependencies.dev), so
add "respx" to the tool.uv.dev-dependencies array to ensure uv sync installs it
by default; update the tool.uv.dev-dependencies entry (referencing the
tool.uv.dev-dependencies section) to include respx with the same version
constraint used in project.optional-dependencies.dev (or a compatible one) so
local test runs that use uv without --all-extras have respx available.
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (63.8%) is below the target coverage (80.0%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #39 +/- ##
=======================================
+ Coverage 32.8% 42.5% +9.6%
=======================================
Files 85 101 +16
Lines 5162 5982 +820
=======================================
+ Hits 1698 2547 +849
+ Misses 3464 3435 -29 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
RepositoryAnalysisRequest,RepositoryFeatures, and helper functions inrepository_analysis_agent/models.py./api/rulesto/api/v1) and added stub forproceed-with-pr..get()with attribute access).Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.