Matas/fix/fix actions #40
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors repository analysis into a LangGraph-driven agent, adds API auth and per-IP/user rate limiting, restructures GitHub integrations and models, centralizes Ruff tooling in project config, and adds integration tests and supporting utilities. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Layer (FastAPI)
participant RateLimit as Rate Limiter
participant Agent as RepositoryAnalysisAgent
participant Graph as LangGraph
participant Fetch as fetch_repository_metadata
participant PRSignals as fetch_pr_signals
participant Gen as generate_rule_recommendations
participant GitHub as GitHub Service
participant LLM as LLM Provider
Client->>API: POST /rules/recommend
API->>RateLimit: enforce limit (ip:user)
alt rate limited
RateLimit-->>Client: 429 Retry-After
else allowed
API->>Agent: execute(repo_full_name, is_public)
Agent->>Graph: run(state) [timeout configured]
Graph->>Fetch: fetch_repository_metadata(state)
Fetch->>GitHub: list files, read README, detect CI/codeowners
GitHub-->>Fetch: metadata/signals
Fetch->>Graph: update state
Graph->>PRSignals: fetch_pr_signals(state)
PRSignals->>GitHub: fetch recent PRs
GitHub-->>PRSignals: pr data
PRSignals->>Graph: compute hygiene metrics
Graph->>Gen: generate_rule_recommendations(state)
Gen->>LLM: prompt with signals & hygiene
LLM-->>Gen: structured RuleRecommendation JSON
Gen->>Graph: attach recommendations
Graph-->>Agent: final state
Agent-->>API: AnalysisResponse (200)
API-->>Client: 200 with recommendations
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 (14)
✏️ 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 focuses on enhancing the stability and functionality of the CI/CD pipeline and the core repository analysis service. It addresses critical build and deployment issues for documentation, while also modernizing the Python development environment through updated linting and formatting tools. A major architectural change involves refactoring the repository analysis agent to use LangGraph, which streamlines the process of generating governance rule recommendations. Furthermore, new API capabilities have been introduced to support unauthenticated analysis of public repositories with robust rate limiting, improving accessibility and user experience. Highlights
Ignored Files
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
|
|
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 #40 +/- ##
=======================================
+ 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:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the repository analysis and rule recommendation system, moving from a procedural approach to a LangGraph-based agent architecture for improved modularity and LLM integration. Key changes include updating pre-commit hooks and Ruff configurations, removing pyupgrade, and streamlining pyproject.toml by consolidating linting/formatting tools under Ruff. The RepositoryAnalysisAgent now uses a StateGraph with dedicated nodes for fetching repository metadata and generating rule recommendations via an LLM, replacing previous static/heuristic methods. Corresponding Pydantic models and LLM prompts have been updated to support this new architecture. Additionally, the API now supports unauthenticated access for public GitHub repositories with defined rate limits, and new dependencies for rate limiting and user authentication have been introduced. Documentation has been updated to reflect these changes, including details on unauthenticated analysis and rate limiting. Review comments highlight critical TODO items and potential issues, such as the need for robust authentication (IdP integration, token signature verification), distributed rate limiting (Redis), complete implementation of GitHub API calls for PR operations and file pattern matching, and ensuring production-ready CORS policies. Several comments also point out hardcoded values and placeholder logic that need to be replaced with real-world implementations for validators to be effective.
| if scheme.lower() != "bearer": | ||
| return None | ||
|
|
||
| # TODO: Wire to real IdP (Supabase/Auth0). For now, fake user if token present. WARNING: Must verify signature in prod. |
There was a problem hiding this comment.
The TODO comment about wiring to a real IdP and verifying signature in prod is critical. For a production system, relying on a fake user and not verifying the token signature is a significant security vulnerability. This should be addressed before deployment to a production environment.
| # We explicitly allow all origins ("*") to prevent the browser from blocking requests | ||
| # from your local file system or different localhost ports. |
There was a problem hiding this comment.
The comment regarding explicitly allowing all origins for development is helpful. However, allow_origins=["*"] is a security risk in production environments as it allows any domain to make requests to the API. This should be restricted to known origins in production. Consider adding a check for the ENVIRONMENT variable to apply stricter CORS policies when not in development.
| if event_type == "pull_request": | ||
| # For pull requests, we'd need to get this from the GitHub API | ||
| # For now, return a placeholder | ||
| # TODO: Pull request—fetch changed files via GitHub API. Placeholder for now. |
There was a problem hiding this comment.
The TODO comment about fetching changed files via GitHub API for pull requests is critical. The current implementation returns an empty list, making this validator ineffective for PR events. This should be a high priority to implement.
| # 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. |
There was a problem hiding this comment.
The TODO comment about checking actual PR reviews is critical. The current implementation always returns True, making this validator ineffective. This should be a high priority to implement.
| from src.core.models import User | ||
|
|
||
| # In-memory store: { key: [timestamps] } | ||
| _RATE_LIMIT_STORE: dict[str, list[float]] = {} |
There was a problem hiding this comment.
The comment # In-memory store: { key: [timestamps] } is good. However, using an in-memory store for rate limiting in a distributed system (e.g., multiple FastAPI instances) will not provide accurate global rate limiting. Consider using a distributed cache like Redis for _RATE_LIMIT_STORE to ensure consistent rate limiting across all instances.
| regex_pattern = FilePatternCondition._glob_to_regex(pattern) | ||
|
|
||
| # Check if any files match the pattern | ||
| # Pattern match—performance: optimize if file count high. |
There was a problem hiding this comment.
The comment # Pattern match—performance: optimize if file count high. is a good performance consideration. If repositories have a very large number of files, this could become a bottleneck. Consider optimizing the matching algorithm or using a more efficient data structure for file paths.
src/api/rate_limit.py
Outdated
| ANON_LIMIT = 5 # requests per hour | ||
| AUTH_LIMIT = 100 # requests per hour | ||
| WINDOW = 3600 # seconds |
There was a problem hiding this comment.
The rate limits ANON_LIMIT, AUTH_LIMIT, and WINDOW are hardcoded. It would be better to make these configurable via environment variables or a settings file, allowing for easier adjustment without code changes.
| # .model_dump() is required because LangGraph expects a dict input | ||
| result_dict = await self.graph.ainvoke(initial_state.model_dump()) |
There was a problem hiding this comment.
The comment # .model_dump() is required because LangGraph expects a dict input is helpful. However, ainvoke can often accept Pydantic models directly if they are compatible with the graph's state. If model_dump() is strictly necessary, it might indicate a slight impedance mismatch or a specific LangGraph version behavior. It's worth verifying if ainvoke(initial_state) would work directly.
|
|
||
| class WeekendCondition(Condition): | ||
| """Validates if the current time is during a weekend.""" | ||
| """Validates if the current time is during a weekend.""" # Time-based logic—fragile if timezone not handled. |
There was a problem hiding this comment.
The comment # Time-based logic—fragile if timezone not handled. is a good warning. It's important to ensure consistent timezone handling throughout the application to avoid unexpected behavior.
|
|
||
| # The shared instance remains the same | ||
| dispatcher = WebhookDispatcher() | ||
| dispatcher = WebhookDispatcher() # Singleton—shared for app lifetime. Thread safety: not guaranteed. |
There was a problem hiding this comment.
The comment # Singleton—shared for app lifetime. Thread safety: not guaranteed. is a crucial note. If the application is deployed in a multi-threaded or asynchronous environment where multiple requests might access the dispatcher concurrently, thread safety should be explicitly ensured or documented as a known limitation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/integrations/github/api.py (1)
114-145: URL-encode repository paths to avoid 404s on special characters.
path/file_pathare interpolated directly into the Contents API URL. Spaces,#, or other reserved characters will break the request. Encoding the path (while preserving/) makes the calls robust.🔧 Suggested fix
- url = f"{config.github.api_base_url}/repos/{repo_full_name}/contents/{path}" + path_encoded = quote(path.lstrip("/"), safe="/") + url = f"{config.github.api_base_url}/repos/{repo_full_name}/contents/{path_encoded}" @@ - url = f"{config.github.api_base_url}/repos/{repo_full_name}/contents/{file_path}" + file_path_encoded = quote(file_path.lstrip("/"), safe="/") + url = f"{config.github.api_base_url}/repos/{repo_full_name}/contents/{file_path_encoded}"Add this import near the top of the file:
from urllib.parse import quote
🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Around line 16-25: The pre-commit hook for Ruff is pinned to an old rev (rev:
v0.3.0); update the ruff-pre-commit hook to a recent release (e.g., rev:
v0.14.13) so the formatter options in pyproject.toml (like [tool.ruff.format]
and docstring-code-format) are supported and you get recent fixes; locate the
block with repo: https://github.com/astral-sh/ruff-pre-commit and the hooks with
id: ruff and id: ruff-format and replace the rev value accordingly, keeping the
existing args (--fix, --exit-non-zero-on-fix) and hook ids unchanged.
In `@docs/benchmarks.md`:
- Line 9: Replace the accidental "70 +" with the intended "70+" to restore
consistency with other occurrences (e.g., the later "70+" at line 58) in the
sentence "Our analysis of 70 + enterprise policies..." so it reads "Our analysis
of 70+ enterprise policies..." throughout the document.
In `@pyproject.toml`:
- Around line 35-38: Update the declared Ruff minimum version to match the
configuration: replace the existing "ruff>=0.1.0" dependency entries with
"ruff>=0.1.8" wherever Ruff is listed (the dependency line in the main
dependencies block and the ruff entry under [tool.uv].dev-dependencies) so the
parser supports the docstring-code-format setting; ensure both occurrences are
changed to "ruff>=0.1.8".
In `@src/agents/feasibility_agent/agent.py`:
- Line 2: Summary: Fix the double space in the module docstring for the Rule
Feasibility Agent. Locate the top-of-file docstring in agent.py (the Rule
Feasibility Agent implementation) and remove the extra space between "handling"
and "and" so the phrase reads "error handling and retry logic" (update the
docstring text accordingly), then save and run linters to confirm spacing is
corrected.
In `@src/agents/repository_analysis_agent/models.py`:
- Around line 3-39: The parse_github_repo_identifier function and
RepositoryAnalysisRequest.normalize_repo_name fail to handle SSH URLs (e.g.,
[email protected]:owner/repo.git), repository_full_name provided as a URL, and
extra path segments like /tree/main; update parse_github_repo_identifier to
robustly extract the "owner/repo" portion by matching patterns for
https?://github.com/... and [email protected]:... and then strip any .git suffix
and any trailing path segments (tree, blob, /branches, etc.) keeping only the
first two path components; then update the model_validator normalize_repo_name
(RepositoryAnalysisRequest.normalize_repo_name) to run this parser whether
repository_url or repository_full_name is populated so both forms get normalized
into owner/repo.
In `@src/agents/repository_analysis_agent/nodes.py`:
- Around line 138-142: The structured output model RecommendationsList currently
inherits from AnalysisState (which enforces fields like repo_full_name) causing
validation errors; change RecommendationsList to inherit from pydantic BaseModel
(or the project's equivalent BaseModel) instead of AnalysisState so it only
requires recommendations: list[RuleRecommendation], then recreate structured_llm
= llm.with_structured_output(RecommendationsList) to use the corrected model.
In `@src/api/dependencies.py`:
- Around line 24-41: The get_current_user_optional function currently returns a
hardcoded User for any Bearer token; change it so the stub user is only returned
in development: read a runtime flag (e.g., IS_DEV from env or settings) inside
get_current_user_optional and if not true either return None (fail-closed) or
raise an HTTPException for unverified tokens; if you keep the dev stub, emit a
clear runtime warning to logs when the stub is used. Update references in the
same function (auth_header parsing and the TODO block) to perform the IS_DEV
check before constructing User(id=123, ...) and ensure tests/dev runs set the
flag appropriately.
In `@src/api/rate_limit.py`:
- Around line 20-27: In rate_limiter, guard against request.client being None
before accessing request.client.host: when user is unauthenticated use a safe
fallback (e.g. request.headers.get('x-forwarded-for') or a literal like
"unknown-ip") to build the key instead of directly using request.client.host;
update the branch that sets key = f"ip:{request.client.host}" (and keep limit =
ANON_LIMIT) so it checks request.client and uses the fallback when missing to
avoid AttributeError and preserve a stable rate-limit key.
In `@src/api/recommendations.py`:
- Around line 21-31: Change AnalyzeRepoRequest.repo_url from HttpUrl to a plain
str so Pydantic accepts shorthand "owner/repo" inputs; update the Field
declaration on the AnalyzeRepoRequest class to use type str (keeping the same
description) so the existing parse_repo_from_url logic (including its shorthand
branch) can run at the endpoint instead of being blocked by HttpUrl validation.
In `@src/core/utils/caching.py`:
- Around line 161-163: The current ternary uses a truthy check so ttl==0 falls
back to TTLCache with a 3600 default and flips cache type; change the logic to
check for ttl is None instead of truthiness. Specifically, in the block that
sets cache when cache is None, select AsyncCache when ttl is None and otherwise
construct TTLCache with the provided ttl (preserving ttl=0), passing maxsize and
ttl unchanged to the chosen constructor (references: AsyncCache, TTLCache,
cache, ttl, maxsize).
In `@src/core/utils/logging.py`:
- Around line 15-19: The runtime NameError is caused by only importing Callable
under TYPE_CHECKING while code or tools use typing.get_type_hints() (and with
from __future__ import annotations) so the annotation string needs a real
Callable in globals; fix by importing Callable at runtime (remove it from the
TYPE_CHECKING-only block or add a separate runtime import) or by changing the
annotated types to fully-qualified strings backed by an actual import; update
the top-level imports where TYPE_CHECKING and Callable are declared and any
annotations referenced in get_type_hints() (references around the annotations at
lines ~148-149) so Callable is available at runtime.
In `@src/integrations/github/service.py`:
- Around line 107-115: The _parse_url function currently splits the raw URL and
takes the last two segments which breaks for URLs with extra path parts or .git
suffix; update _parse_url to parse the URL path (use urlparse on the input),
strip any trailing ".git" from the repo segment, split the path into segments
and return the first two non-empty segments as (owner, repo) (or raise
ValueError if fewer than two segments); ensure this logic is implemented inside
the _parse_url method to correctly handle forms like /owner/repo, /owner/repo/,
/owner/repo.git, and /owner/repo/tree/main.
In `@src/main.py`:
- Around line 88-96: The CORS setup in app.add_middleware using CORSMiddleware
is too permissive: do not combine allow_origins=["*"] with
allow_credentials=True; instead load a whitelist of allowed origins from
configuration/env (e.g., ALLOWED_ORIGINS) and set allow_origins to that list for
production, and only use allow_origins=["*"] in development with
allow_credentials=False; update the middleware initialization to read the
environment flag (dev vs prod) or ALLOWED_ORIGINS and conditionally set
allow_origins and allow_credentials accordingly.
In `@src/webhooks/router.py`:
- Around line 62-65: The except HTTPException handler currently returns a normal
dict which yields a 200; instead preserve the original 202 status by returning
an HTTP response with status_code set to e.status_code (or re-raising the
HTTPException) so clients receive 202 for unsupported events; update the except
block that catches "HTTPException as e" in router.py to return a response object
(or raise e) containing {"status":"event received but not supported","detail":
e.detail} and the status_code e.status_code.
In `@tests/conftest.py`:
- Around line 32-49: The tests create a Config() at import time so module-level
assigns like GITHUB_WEBHOOK_SECRET in webhooks/auth.py are evaluated before the
autouse fixture; to fix, set the environment defaults as real os.environ entries
at module import time (before any app imports) and include the missing
APP_NAME_GITHUB key so Config.validate() sees it; update tests/conftest.py to
populate os.environ (e.g., os.environ.setdefault(...) or os.environ.update({...
including "APP_NAME_GITHUB": "mock-app"})) at top-level before defining or using
the mock_settings fixture so Config() and module-level assignments read the
mocked values.
- Around line 54-60: The session-scoped pytest fixture event_loop currently
creates a new loop but does not set it as the current event loop; modify the
fixture to call asyncio.set_event_loop(loop) after creating loop so code using
asyncio.get_event_loop() at import/session time works, and on teardown (after
yield) call asyncio.set_event_loop(None) and close the loop (loop.close()) to
clean up; keep references to the existing symbols (policy, loop, event_loop) so
the change is local to that fixture.
🧹 Nitpick comments (10)
src/webhooks/handlers/pull_request.py (1)
22-30: Comment improves clarity.The updated comment effectively documents both the intent (rules.yaml validation on PR open) and the brittleness concern. The reliance on GitHub's "opened" action name is indeed brittle, though GitHub's webhook API is generally stable and breaking changes would be versioned.
Optional: Consider defensive logging
If you want to make the brittleness more visible in production, you could add logging for unexpected actions:
# PR opened—trigger rules.yaml validation. Brittle if GitHub changes event action names. action = event.payload.get("action") if action == "opened": pr_number = event.payload.get("pull_request", {}).get("number") if pr_number: await validate_rules_yaml_from_repo( repo_full_name=event.repo_full_name, installation_id=event.installation_id, pr_number=pr_number, ) elif action: logger.debug(f"Skipping rules validation for PR action: {action}")This would make it easier to spot if GitHub changes action names in the future.
src/rules/validators.py (2)
138-147: Acknowledge the TODO for production readiness.The hardcoded team memberships are clearly marked as test/demo only. Consider tracking this as an issue to ensure the real GitHub API integration is implemented before production use.
Would you like me to open an issue to track the GitHub API integration for team membership checking?
246-260: Timezone handling note is accurate.The
WeekendConditionusesdatetime.now()without timezone awareness, which will use the server's local timezone. The fragility comment is appropriate. For production, consider usingdatetime.now(timezone.utc)or accepting a timezone parameter similar toAllowedHoursCondition(lines 615-637).src/api/rules.py (1)
14-27: Newfeedbackfield adds useful context to the response.The response now includes
result.messageasfeedback, which aligns with theAgentResultstructure from the feasibility agent.Consider adding error handling. The comment on line 19 notes the agent may throw, but exceptions will propagate as unhandled 500 errors. Other agents (see
feasibility_agent/agent.pylines 99-122) return structuredAgentResulton failure, but malformed input or unexpected errors could still raise.💡 Optional: Add try/except for graceful 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) - - # 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, - } + try: + result = await agent.execute(rule_description=request.rule_text) + return { + "supported": result.data.get("is_feasible", False), + "snippet": result.data.get("yaml_content", ""), + "feedback": result.message, + } + except Exception as e: + return { + "supported": False, + "snippet": "", + "feedback": f"Evaluation failed: {str(e)}", + }src/integrations/github/api.py (1)
230-234: Set explicit aiohttp timeouts to prevent hung requests.The default ClientTimeout (total=300s with unset socket timeouts) can leave requests hanging indefinitely. Specifically, socket-level timeouts (sock_read, sock_connect) must be set explicitly to avoid epoll waiting indefinitely on network issues. Set all timeout fields, not just total:
♻️ Suggested update
- if self._session is None or self._session.closed: - self._session = aiohttp.ClientSession() + if self._session is None or self._session.closed: + timeout = aiohttp.ClientTimeout(total=30, connect=5, sock_connect=5, sock_read=20) + self._session = aiohttp.ClientSession(timeout=timeout) return self._sessionAdjust timeout values (total, sock_read, etc.) based on the GitHub API's actual response latency distribution. Consider adding retries with exponential backoff for transient failures.
src/api/rate_limit.py (1)
12-40: In‑memory store is per‑process and unbounded.
This limiter won’t enforce limits across multiple workers and the dict can grow without cleanup. Consider a shared store (e.g., Redis) or at least evict empty/expired keys to cap memory.src/api/dependencies.py (1)
8-9: Remove duplicate logger assignment.
Redundant line adds noise and can confuse future edits.♻️ 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/core/models.py (1)
4-18: Prevent accidental serialization of OAuth tokens.
BaseModelwill includegithub_tokenin dumps/reprs by default, which increases leakage risk if this model is logged or serialized. ConsiderSecretStrplusexclude=True/repr=False.🔒 Proposed hardening
-from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, SecretStr @@ - github_token: str | None = Field(None, description="OAuth token for GitHub API access") + github_token: SecretStr | None = Field( + default=None, + description="OAuth token for GitHub API access", + exclude=True, + repr=False, + )src/integrations/github/service.py (2)
40-41: Add explicit timeouts for outbound GitHub calls.
Relying on defaults can lead to long hangs under network issues. A short, explicit timeout keeps request threads healthy.🛠️ Suggested update
- async with httpx.AsyncClient() as client: + timeout = httpx.Timeout(10.0, connect=5.0) + async with httpx.AsyncClient(timeout=timeout) as client: @@ - async with httpx.AsyncClient() as client: + timeout = httpx.Timeout(10.0, connect=5.0) + async with httpx.AsyncClient(timeout=timeout) as client:
66-73: Surface rate-limit/permission errors during file checks.
analyze_repository_rulescurrently treats 403/429 as “missing file,” which can silently produce misleading recommendations. Consider raisingGitHubRateLimitError(or propagating other errors) so callers can return 429.🛠️ Suggested handling
- resp = await client.get(check_url, headers=headers) - if resp.status_code == 200: - found_files.append(filepath) + resp = await client.get(check_url, headers=headers) + if resp.status_code == 403 and "rate limit" in resp.text.lower(): + raise GitHubRateLimitError("GitHub API rate limit exceeded") + if resp.status_code == 404: + continue + resp.raise_for_status() + if resp.status_code == 200: + found_files.append(filepath)
📜 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 (40)
.github/workflows/docs.yaml.pre-commit-config.yamlREADME.mddocs/benchmarks.mddocs/features.mdlinting_commands.txtpyproject.tomlsrc/__init__.pysrc/agents/feasibility_agent/agent.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 (15)
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/webhooks/handlers/deployment.py (1)
src/core/models.py (1)
repo_full_name(56-58)
tests/integration/test_recommendations.py (1)
src/core/utils/caching.py (1)
get(44-66)
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/rate_limit.py (2)
src/core/models.py (1)
User(7-18)src/core/utils/caching.py (1)
get(44-66)
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/check_run.py (1)
handle(16-29)src/webhooks/handlers/deployment_protection_rule.py (1)
handle(16-33)src/webhooks/handlers/push.py (1)
handle(16-29)src/webhooks/handlers/deployment_review.py (1)
handle(16-33)
src/api/dependencies.py (2)
src/core/models.py (1)
User(7-18)src/integrations/github/service.py (1)
GitHubService(19-115)
src/agents/repository_analysis_agent/nodes.py (6)
src/agents/repository_analysis_agent/models.py (2)
AnalysisState(81-106)RuleRecommendation(68-78)src/integrations/providers/bedrock_provider.py (2)
get_chat_model(24-48)with_structured_output(202-204)src/integrations/providers/factory.py (1)
get_chat_model(97-130)src/core/models.py (1)
repo_full_name(56-58)src/integrations/github/api.py (2)
list_directory_any_auth(114-129)get_file_content(131-161)src/integrations/providers/vertex_ai_provider.py (1)
ainvoke(109-131)
src/main.py (3)
src/webhooks/handlers/deployment_protection_rule.py (1)
DeploymentProtectionRuleEventHandler(10-33)src/tasks/scheduler/deployment_scheduler.py (2)
get_deployment_scheduler(364-369)stop(40-48)src/tasks/task_queue.py (1)
stop_workers(143-149)
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/recommendations.py (6)
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)tests/integration/test_rules_api.py (1)
client(21-23)
src/api/rules.py (6)
src/agents/factory.py (1)
get_agent(20-52)src/agents/feasibility_agent/agent.py (1)
execute(57-123)src/agents/repository_analysis_agent/agent.py (1)
execute(39-62)src/agents/base.py (1)
execute(111-113)src/agents/acknowledgment_agent/agent.py (1)
execute(212-217)src/agents/engine_agent/agent.py (1)
execute(75-173)
src/agents/repository_analysis_agent/agent.py (5)
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 (2)
fetch_repository_metadata(13-104)generate_rule_recommendations(107-168)src/core/models.py (1)
repo_full_name(56-58)src/integrations/providers/vertex_ai_provider.py (1)
ainvoke(109-131)
src/agents/repository_analysis_agent/models.py (1)
src/core/models.py (1)
repo_full_name(56-58)
src/rules/validators.py (2)
src/core/utils/caching.py (1)
get(44-66)src/webhooks/handlers/issue_comment.py (1)
event_type(19-20)
🪛 Gitleaks (8.30.0)
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)
🪛 markdownlint-cli2 (0.18.1)
README.md
238-238: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
244-244: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (26)
src/__init__.py (1)
1-1: LGTM — comment is clear and harmless.No concerns with this change.
src/webhooks/handlers/pull_request.py (1)
22-22: Inconsistency: Comment change doesn't align with PR objectives.The PR objectives describe CI/CD fixes (mkdocs dependencies, fork deployment restrictions), but this file change is just a comment update in a webhook handler. While the improved comment adds useful context about brittleness, it's unclear how this relates to the stated goal of fixing GitHub Actions failures.
.github/workflows/docs.yaml (1)
71-71: LGTM!The condition correctly restricts Pages deployment to the upstream repository while still allowing the build job to run on forks and PRs for validation. This aligns with the PR objective to prevent 404 errors on forked repositories.
src/webhooks/auth.py (1)
32-42: LGTM!The updated comments are concise and informative. The implementation correctly follows security best practices: raw body bytes for signature verification, HMAC-SHA256 as per GitHub's standard, and constant-time comparison to prevent timing attacks.
docs/features.md (1)
279-287: LGTM!The new documentation section clearly describes the rate limiting behavior and authentication requirements. The content aligns with the API implementation and provides useful information for users about access limits.
src/integrations/providers/factory.py (2)
8-9: LGTM!Adding
from __future__ import annotationsenables PEP 563 postponed evaluation of annotations, which is a good practice for forward compatibility.
78-79: LGTM!The ternary expressions are cleaner than multi-line conditionals. Using
is not Nonecorrectly distinguishes between explicitly passing0/0.0and omitting the parameter, maintaining the documented precedence: explicit params > agent config > global config.src/tasks/scheduler/deployment_scheduler.py (1)
45-47: LGTM!Using
contextlib.suppress(asyncio.CancelledError)is more idiomatic than a try-except-pass block and aligns with Ruff's SIM105 recommendation. The behavior remains the same: cancel the task, await its completion, and suppress the expectedCancelledError.src/core/config/settings.py (1)
133-150: LGTM — clearer provider-scoped validation.src/tasks/task_queue.py (1)
35-98: Comment updates look good.README.md (1)
239-245: All rate limit values in the documentation are accurate and match the implementation. The 5 requests/hour for anonymous users, 100 requests/hour for authenticated users, 429 status code, and Retry-After header behavior are all correctly implemented insrc/api/rate_limit.py.src/webhooks/handlers/deployment_status.py (1)
27-42: LGTM!The comment updates clearly document potential fragility points (payload structure dependency, queue overload risk). The enqueue call signature aligns with other handlers (push.py, check_run.py, deployment.py) passing
event_type,repo_full_name,installation_id, andpayload.src/webhooks/dispatcher.py (2)
60-60: LGTM!The comment accurately documents that thread safety is not guaranteed for the singleton. Since FastAPI typically uses asyncio (single-threaded event loop), this is generally acceptable, but the documentation serves as a good warning for future maintainers.
16-17: Helpful documentation.The comment clarifies the registry's purpose and highlights useful properties (hot-swap, test injection).
src/rules/utils/contributors.py (1)
61-65: LGTM!The combined conditional correctly leverages short-circuit evaluation—
_has_recent_activityis only awaited whencontributions >= min_contributionsis true, preserving both semantics and performance. Good SIM102 refactor.src/rules/validators.py (3)
223-226: Acknowledge the TODO for contributor history.Similar to the team membership, this hardcoded new contributor list needs real implementation. The comment makes the limitation clear.
239-243: Placeholder logic acknowledged.The
ApprovalCountConditionalways returnsTrueand ignoresmin_approvals. The comments clearly document this is demo-only, but this validator will pass all PR reviews regardless of actual approval count.
85-91: Documentation improvements look good.The added comments throughout the validators clearly document fragility points, placeholder behavior, and areas needing improvement. This improves maintainability.
Also applies to: 132-136, 168-179, 181-187, 193-199, 204-206
src/integrations/providers/bedrock_provider.py (2)
152-167: Cleaner profile matching with case-insensitive comparison.The refactored logic with
is_anthropic,is_amazon,is_metaflags improves readability. Computingmodel_loweronce and reusing it is efficient.Minor note: The
# SIM102comment on line 156 appears misplaced—this isn't combining nestedifstatements but rather using flag-based conditionals.
180-180: Import moved inside method.Moving
BaseChatModelimport inside_wrap_anthropic_clientis acceptable for lazy loading, avoiding import errors when the package isn't installed. This follows the same pattern asAnthropicBedrockimport in_get_anthropic_bedrock_client(line 61).src/webhooks/router.py (1)
13-34: Normalization and defensive handling look good.
Stripping dot suffixes and treating unknown events as non-fatal improves robustness.src/webhooks/handlers/issue_comment.py (1)
35-143: LGTM.
The added no‑match default and refactored pattern checks improve clarity without altering core behavior.Also applies to: 155-169, 193-194
src/agents/repository_analysis_agent/prompts.py (1)
3-35: LGTM.
Clear system/user prompt separation and explicit JSON requirement are well‑stated.src/main.py (1)
16-78: LGTM.
Lifecycle logging, handler registration, and health metrics updates look solid.Also applies to: 101-105, 121-135
src/agents/repository_analysis_agent/agent.py (1)
22-58: Looks solid.
The execution flow, error handling, and result mapping are clear and cohesive.src/agents/repository_analysis_agent/nodes.py (1)
13-104: Nice defensive handling and logging in metadata collection.
The staged fallbacks keep analysis resilient while still capturing useful signals.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Ruff: The One Tool to Rule Them All | ||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||
| rev: v0.9.9 | ||
| rev: v0.3.0 | ||
| hooks: | ||
| # First, run ruff format - won't change imports but will format them | ||
| - id: ruff-format | ||
| name: "Format code (without changing imports)" | ||
|
|
||
| # Then, use a single ruff pass for both import sorting and linting | ||
| # Linter (Fixes imports, modernizes syntax, checks bugs) | ||
| - id: ruff | ||
| name: "Linting and import sorting" | ||
| args: ["--fix"] | ||
|
|
||
| # Pyupgrade: Check and fix Python version incompatibilities and outdated syntax | ||
| - repo: https://github.com/asottile/pyupgrade | ||
| rev: v3.15.2 | ||
| hooks: | ||
| - id: pyupgrade | ||
| name: "Upgrade syntax for Python 3.12+" | ||
| args: [--py312-plus] | ||
| # Auto-fixes outdated syntax to Python 3.12+ compatible code | ||
| args: [--fix, --exit-non-zero-on-fix] | ||
| # Formatter (Replaces Black) | ||
| - id: ruff-format | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ruff-pre-commit latest version 2024 2025
💡 Result:
The latest ruff-pre-commit release is v0.14.13 (released Jan 15, 2026). [1]
For context, earlier recent releases include v0.14.11 (Jan 8, 2026) and v0.14.10 (Dec 18, 2025). [1][2]
Sources:
- GitHub releases for astral-sh/ruff-pre-commit. [1]
- ruff PyPI page (example pre-commit rev references). [2]
🏁 Script executed:
# Search for Ruff configuration files in the repository
fd -e toml -e cfg | grep -E "(pyproject|ruff|setup)" | head -20Repository: warestack/watchflow
Length of output: 182
🏁 Script executed:
# Look for any ruff configuration in common locations
find . -maxdepth 2 -type f \( -name "pyproject.toml" -o -name "ruff.toml" -o -name "setup.cfg" -o -name ".pre-commit-config.yaml" \)Repository: warestack/watchflow
Length of output: 105
🏁 Script executed:
cat pyproject.tomlRepository: warestack/watchflow
Length of output: 2990
🏁 Script executed:
cat .pre-commit-config.yamlRepository: warestack/watchflow
Length of output: 898
🌐 Web query:
ruff-format hook when introduced ruff-pre-commit v0.3.0 release date
💡 Result:
-
ruff-pre-commit v0.3.0 release date: Feb 29, 2024. [1]
-
ruff-format pre-commit hook: documented in the ruff-pre-commit README (the formatter hook "ruff-format" / formatter support is noted as available once Ruff added a formatter — referenced in the ruff-pre-commit repo README). Usage/examples appear in docs and issues from early 2024. [2][3]
Sources:
- PyPI release files for ruff 0.3.0 (uploaded Feb 29, 2024). [turn6search5]
- ruff-pre-commit README showing the ruff-format hook. [turn8search1]
- ruff-pre-commit issue / discussion showing ruff-format in use (Jan–Feb 2024). [turn8search6]
Update ruff-pre-commit from v0.3.0 to a recent version.
v0.3.0 (released Feb 2024) is significantly outdated—the latest version is v0.14.13 (Jan 2026). The repository's pyproject.toml specifies ruff>=0.1.0 and includes modern formatter configuration ([tool.ruff.format] with options like docstring-code-format). Update to a recent release to ensure the formatter configuration is fully supported and to receive bug fixes and improvements from the past 2 years of development.
🤖 Prompt for AI Agents
In @.pre-commit-config.yaml around lines 16 - 25, The pre-commit hook for Ruff
is pinned to an old rev (rev: v0.3.0); update the ruff-pre-commit hook to a
recent release (e.g., rev: v0.14.13) so the formatter options in pyproject.toml
(like [tool.ruff.format] and docstring-code-format) are supported and you get
recent fixes; locate the block with repo:
https://github.com/astral-sh/ruff-pre-commit and the hooks with id: ruff and id:
ruff-format and replace the rev value accordingly, keeping the existing args
(--fix, --exit-non-zero-on-fix) and hook ids unchanged.
| ### Context Dependency in Enterprise Policies | ||
|
|
||
| Our analysis of 70+ enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules. | ||
| Our analysis of 70 + enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules. |
There was a problem hiding this comment.
Unintentional typo introduces inconsistency.
The change from 70+ to 70 + appears accidental. Line 58 still uses 70+ (without space), creating inconsistency. The space changes the meaning from "more than 70" to a mathematical expression.
Suggested fix
-Our analysis of 70 + enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules.
+Our analysis of 70+ enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules.📝 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.
| Our analysis of 70 + enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules. | |
| Our analysis of 70+ enterprise policies from major tech companies revealed a critical insight: **85% of real-world governance policies require context** and cannot be effectively enforced with traditional static rules. |
🤖 Prompt for AI Agents
In `@docs/benchmarks.md` at line 9, Replace the accidental "70 +" with the
intended "70+" to restore consistency with other occurrences (e.g., the later
"70+" at line 58) in the sentence "Our analysis of 70 + enterprise policies..."
so it reads "Our analysis of 70+ enterprise policies..." throughout the
document.
| "respx>=0.20.0", | ||
| "mypy>=1.7.0", | ||
| "pre-commit>=3.5.0", | ||
| "ruff>=0.1.0", # Replaces black, isort, flake8 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the pyproject.toml file
git ls-files | grep -E "pyproject\.toml$"Repository: warestack/watchflow
Length of output: 77
🌐 Web query:
Ruff minimum version requirements [tool.ruff.format] indent-width docstring-code-format
💡 Result:
- indent-width: supported in Ruff configuration (see Settings). [1]
- docstring-code-format (and docstring-code-line-length): introduced via config PR (
#8854); it's an opt-in formatter setting documented in the Ruff formatter docs. Use a Ruff build that includes PR#8854(see the PR/commit history) or any recent release that contains the formatter updates. [2][3]
Sources:
[1] Ruff Settings — indent-width. [docs.astral.sh/ruff/settings] (docs.astral.sh)
[2] The Ruff formatter docs (docstring-code-format / docstring-code-line-length). [docs.astral.sh/ruff/formatter] (docs.astral.sh)
[3] GitHub: config: add new docstring-code-format knob (PR/commit reference). (git.joshthomas.dev)
If you want I can look up the exact Ruff release/version that first included PR #8854 and give the minimum version number.
Citations:
- 1: https://docs.astral.sh/ruff/settings/?utm_source=openai
- 2: https://docs.astral.sh/ruff/formatter/?utm_source=openai
- 3: https://git.joshthomas.dev/language-servers/ruff/commits/commit/db38078ca3e724f3fa62cfd800ce4e73833ebe00/docs/formatter.md?utm_source=openai
🌐 Web query:
Ruff changelog version 0.1.0 format configuration support
💡 Result:
Short answer: In Ruff 0.1.0 the old "format" configuration was removed — you must use the new "output-format" option (and RUFF_OUTPUT_FORMAT / --output-format) to configure output format. [1][2]
Sources:
- Ruff CHANGELOG (0.1.0) — notes that the deprecated
formatsetting was removed and to useoutput-format/RUFF_OUTPUT_FORMAT/--output-formatinstead. [1] - Ruff 0.1.0 release summary (release notes). [2]
🌐 Web query:
Ruff rule selectors TCH when introduced minimum version
💡 Result:
Short answer: The "TCH" selector comes from Ruff's flake8-type-checking rules (used in 2023) and was remapped to "TC" in Ruff v0.8.0 — Ruff 0.8.0 is the minimum release that documents the remapping (TCH -> TC) and emits a deprecation warning when you use the old TCH codes. [1][2]
Sources:
- Ruff v0.8.0 release notes (remapped flake8-type-checking: TCH -> TC). [1]
- GitHub issue showing TCH usage/discussion (Dec 2023). [2]
🏁 Script executed:
cat pyproject.tomlRepository: warestack/watchflow
Length of output: 2990
🌐 Web query:
Ruff [tool.ruff.format] section minimum version when introduced
💡 Result:
The [tool.ruff.format] section was introduced with the Ruff formatter (beta) — added in Ruff v0.1.2 (Oct 24, 2023). [1][2]
Sources:
- Ruff v0.1.2 release notes (announces the Beta formatter). [1]
- Ruff configuration / formatter docs showing [tool.ruff.format] settings. [2]
🌐 Web query:
Ruff 0.1.0 vs 0.2.0 vs 0.3.0 format section support release notes
💡 Result:
Summary of format-section support by Ruff releases
-
v0.1.0 — initial Ruff formatter and basic pyproject configuration support (formatter settings live under [tool.ruff.format] / [format]). [1][2]
-
v0.2.x (notably 0.2.1 / 0.2.2) — formatter feature additions:
- Added range formatting (ruff format --range) to format specific line ranges.
- Formatting improvements for module-level docstrings and not trimming last empty line in docstrings.
- Early f-string formatting support in preview builds and other formatter fixes. [2]
-
v0.3.0 — first stable/stabilizing formatter release: introduced the “Ruff formatter 2024.2” style and added a lint rule detecting invalid formatter-suppression comments; continued formatter stabilizations and style changes. [3]
Sources
- Ruff formatter docs (configuration / [tool.ruff.format]). [docs.astral.sh]. (docs.astral.sh)
- Ruff changelog (0.2.x — range formatting, docstring fixes, f-string preview). (fossies.org)
- Ruff v0.3.0 release summary (formatter 2024.2 style, suppression-comment rule). (newreleases.io)
If you want, I can copy the exact pyproject keys available in each version or link the specific GitHub changelog entries.
Citations:
- 1: https://docs.astral.sh/ruff/formatter/?utm_source=openai
- 2: https://fossies.org/linux/ruff/changelogs/0.2.x.md?utm_source=openai
- 3: https://newreleases.io/project/pypi/ruff/release/0.3.0?utm_source=openai
🌐 Web query:
Ruff docstring-code-format PR #8854 which release version minimum
💡 Result:
The feature from PR #8854 (docstring-code-format) first appears in ruff v0.1.8. [1][2]
Sources:
- ruff changelog (v0.1.8) — lists "Add docstring-code-format knob" (
#8854). [1] - ruff CHANGELOG in repo — same entry. [2]
🌐 Web query:
Ruff changelog 0.1.2 0.1.3 0.1.4 0.1.5 0.2.0 docstring-code-format when added
💡 Result:
Summary: The docstring-code-format option was introduced in Ruff v0.1.8 (so it was not present in v0.1.2–v0.1.5 and is available in later releases, including 0.2.0). [1][2][3]
Sources:
- PR adding the config knob (config: add new
docstring-code-format(#8854)). [1] - Ruff 0.1.x changelog noting "Add
docstring-code-formatknob" in v0.1.8. [2] - PyPI upload metadata for ruff 0.1.8 (upload date Dec 13, 2023). [3]
Bump Ruff minimum version to 0.1.8 to support docstring-code-format.
The configuration uses docstring-code-format = true in [tool.ruff.format], which was introduced in Ruff v0.1.8. Setting ruff>=0.1.0 creates a version mismatch—if an older version is installed, config parsing will fail. Update both occurrences (lines 35–38 and in [tool.uv].dev-dependencies) to ruff>=0.1.8.
🤖 Prompt for AI Agents
In `@pyproject.toml` around lines 35 - 38, Update the declared Ruff minimum
version to match the configuration: replace the existing "ruff>=0.1.0"
dependency entries with "ruff>=0.1.8" wherever Ruff is listed (the dependency
line in the main dependencies block and the ruff entry under
[tool.uv].dev-dependencies) so the parser supports the docstring-code-format
setting; ensure both occurrences are changed to "ruff>=0.1.8".
| @@ -1,5 +1,5 @@ | |||
| """ | |||
| Rule Feasibility Agent implementation with error handling and retry logic. | |||
| Rule Feasibility Agent implementation with error handling and retry logic. | |||
There was a problem hiding this comment.
Fix double space in docstring.
There's an extra space between “handling” and “and”.
✏️ Suggested tweak
-Rule Feasibility Agent implementation with error handling and retry logic.
+Rule Feasibility Agent implementation with error handling and retry logic.📝 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.
| Rule Feasibility Agent implementation with error handling and retry logic. | |
| Rule Feasibility Agent implementation with error handling and retry logic. |
🤖 Prompt for AI Agents
In `@src/agents/feasibility_agent/agent.py` at line 2, Summary: Fix the double
space in the module docstring for the Rule Feasibility Agent. Locate the
top-of-file docstring in agent.py (the Rule Feasibility Agent implementation)
and remove the extra space between "handling" and "and" so the phrase reads
"error handling and retry logic" (update the docstring text accordingly), then
save and run linters to confirm spacing is corrected.
| from typing import Any | ||
|
|
||
| from pydantic import BaseModel, Field, field_validator, model_validator | ||
| from pydantic import BaseModel, Field, model_validator | ||
|
|
||
|
|
||
| def parse_github_repo_identifier(value: str) -> str: | ||
| def parse_github_repo_identifier(identifier: str) -> str: | ||
| """ | ||
| Normalize a GitHub repository identifier. | ||
|
|
||
| Accepts: | ||
| - owner/repo | ||
| - https://github.com/owner/repo | ||
| - https://github.com/owner/repo.git | ||
| - owner/repo/ | ||
| Normalizes various GitHub identifiers into 'owner/repo' format. | ||
| Used by tests to verify repository strings. | ||
| """ | ||
| raw = (value or "").strip() | ||
| if not raw: | ||
| return "" | ||
|
|
||
| if raw.startswith("https://") or raw.startswith("http://"): | ||
| parts = raw.split("/") | ||
| try: | ||
| gh_idx = parts.index("github.com") | ||
| except ValueError: | ||
| # Could be enterprise; keep as-is and let API validation fail. | ||
| return raw.rstrip("/").removesuffix(".git") | ||
| # Remove protocol and domain | ||
| clean_id = identifier.replace("https://github.com/", "").replace("http://github.com/", "") | ||
|
|
||
| owner = parts[gh_idx + 1] if len(parts) > gh_idx + 1 else "" | ||
| repo = parts[gh_idx + 2] if len(parts) > gh_idx + 2 else "" | ||
| return f"{owner}/{repo}".rstrip("/").removesuffix(".git") | ||
| # Remove .git suffix and trailing slashes | ||
| clean_id = clean_id.replace(".git", "").strip("/") | ||
|
|
||
| return raw.rstrip("/").removesuffix(".git") | ||
| return clean_id | ||
|
|
||
|
|
||
| class PullRequestSample(BaseModel): | ||
| """Minimal PR snapshot used for recommendations.""" | ||
|
|
||
| number: int | ||
| title: str | ||
| state: str | ||
| merged: bool = False | ||
| additions: int | None = None | ||
| deletions: int | None = None | ||
| changed_files: int | None = None | ||
|
|
||
| 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.
Harden repository identifier normalization for common URL/SSH forms.
The current normalizer only handles http(s)://github.com/... and .git. SSH URLs (e.g., [email protected]:owner/repo.git) and extra path segments (e.g., /tree/main) will slip through and produce invalid owner/repo values. Also, if repository_full_name is a URL, it won’t be normalized.
🔧 Suggested fix
-from typing import Any
+from typing import Any
+import re
@@
-def parse_github_repo_identifier(identifier: str) -> str:
+def parse_github_repo_identifier(identifier: str) -> str:
@@
- # Remove protocol and domain
- clean_id = identifier.replace("https://github.com/", "").replace("http://github.com/", "")
-
- # Remove .git suffix and trailing slashes
- clean_id = clean_id.replace(".git", "").strip("/")
-
- return clean_id
+ clean_id = identifier.strip()
+ clean_id = re.sub(r"^(?:https?://|git@)github\.com[:/]", "", clean_id)
+ clean_id = clean_id.split("?", 1)[0].split("#", 1)[0]
+ clean_id = clean_id.removesuffix(".git").strip("/")
+ parts = clean_id.split("/")
+ return "/".join(parts[:2]) if len(parts) >= 2 else clean_id
@@
- if not self.repository_full_name and self.repository_url:
- self.repository_full_name = parse_github_repo_identifier(self.repository_url)
+ value = self.repository_full_name or self.repository_url
+ if value:
+ self.repository_full_name = parse_github_repo_identifier(value)
return self🤖 Prompt for AI Agents
In `@src/agents/repository_analysis_agent/models.py` around lines 3 - 39, The
parse_github_repo_identifier function and
RepositoryAnalysisRequest.normalize_repo_name fail to handle SSH URLs (e.g.,
[email protected]:owner/repo.git), repository_full_name provided as a URL, and
extra path segments like /tree/main; update parse_github_repo_identifier to
robustly extract the "owner/repo" portion by matching patterns for
https?://github.com/... and [email protected]:... and then strip any .git suffix
and any trailing path segments (tree, blob, /branches, etc.) keeping only the
first two path components; then update the model_validator normalize_repo_name
(RepositoryAnalysisRequest.normalize_repo_name) to run this parser whether
repository_url or repository_full_name is populated so both forms get normalized
into owner/repo.
| def _parse_url(self, url: str) -> tuple[str, str]: | ||
| """ | ||
| Extracts owner and repo from https://github.com/owner/repo | ||
| """ | ||
| clean_url = str(url).rstrip("/") | ||
| parts = clean_url.split("/") | ||
| if len(parts) < 2: | ||
| raise ValueError("Invalid GitHub URL") | ||
| return parts[-2], parts[-1] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
find . -type f -name "service.py" | head -20Repository: warestack/watchflow
Length of output: 99
🏁 Script executed:
# Check if the specific file exists
ls -la src/integrations/github/service.py 2>/dev/null || echo "File not found at expected path"Repository: warestack/watchflow
Length of output: 146
🏁 Script executed:
# If found, read the relevant lines
if [ -f "src/integrations/github/service.py" ]; then
wc -l src/integrations/github/service.py
fiRepository: warestack/watchflow
Length of output: 101
🏁 Script executed:
cat -n src/integrations/github/service.pyRepository: warestack/watchflow
Length of output: 4815
_parse_url fails on common GitHub URL patterns.
Taking the last two path segments breaks URLs like https://github.com/owner/repo/tree/main (returns tree/main instead of owner/repo) or those with .git suffix (returns repo.git instead of repo). This causes false 404 errors since the API receives incorrect owner/repo values.
🐛 Suggested fix
+from urllib.parse import urlparse
@@
- clean_url = str(url).rstrip("/")
- parts = clean_url.split("/")
- if len(parts) < 2:
- raise ValueError("Invalid GitHub URL")
- return parts[-2], parts[-1]
+ clean_url = str(url).strip().rstrip("/")
+ # Allow shorthand "owner/repo"
+ if "github.com" not in clean_url and len(clean_url.split("/")) == 2:
+ owner, repo = clean_url.split("/", 1)
+ return owner, repo.removesuffix(".git")
+
+ parsed = urlparse(clean_url)
+ if parsed.netloc != "github.com":
+ raise ValueError("Invalid GitHub URL")
+ parts = [p for p in parsed.path.split("/") if p]
+ if len(parts) < 2:
+ raise ValueError("Invalid GitHub URL")
+ owner, repo = parts[0], parts[1].removesuffix(".git")
+ return owner, repo🤖 Prompt for AI Agents
In `@src/integrations/github/service.py` around lines 107 - 115, The _parse_url
function currently splits the raw URL and takes the last two segments which
breaks for URLs with extra path parts or .git suffix; update _parse_url to parse
the URL path (use urlparse on the input), strip any trailing ".git" from the
repo segment, split the path into segments and return the first two non-empty
segments as (owner, repo) (or raise ValueError if fewer than two segments);
ensure this logic is implemented inside the _parse_url method to correctly
handle forms like /owner/repo, /owner/repo/, /owner/repo.git, and
/owner/repo/tree/main.
| # 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.
CORS is overly permissive with credentials enabled.
allow_origins=["*"] combined with allow_credentials=True is unsafe for production and can leak auth data to any origin. Consider environment‑based allowed origins and disable credentials when wildcard is used.
🔧 Example tightening
+import os
@@
-app.add_middleware(
- CORSMiddleware,
- allow_origins=["*"], # Explicitly allow all for development
- allow_credentials=True,
- allow_methods=["*"],
- allow_headers=["*"],
-)
+origins = [o.strip() for o in os.getenv("WATCHFLOW_CORS_ORIGINS", "*").split(",") if o.strip()]
+allow_all = "*" in origins
+app.add_middleware(
+ CORSMiddleware,
+ allow_origins=origins,
+ allow_credentials=not allow_all,
+ allow_methods=["*"],
+ allow_headers=["*"],
+)🤖 Prompt for AI Agents
In `@src/main.py` around lines 88 - 96, The CORS setup in app.add_middleware using
CORSMiddleware is too permissive: do not combine allow_origins=["*"] with
allow_credentials=True; instead load a whitelist of allowed origins from
configuration/env (e.g., ALLOWED_ORIGINS) and set allow_origins to that list for
production, and only use allow_origins=["*"] in development with
allow_credentials=False; update the middleware initialization to read the
environment flag (dev vs prod) or ALLOWED_ORIGINS and conditionally set
allow_origins and allow_credentials accordingly.
| except HTTPException as e: | ||
| # This allows us to gracefully handle unsupported events without | ||
| # treating them as server errors. | ||
| # Don't 500 on unknown event—keeps GitHub happy, avoids alert noise. | ||
| if e.status_code == 202: | ||
| return {"status": "event received but not supported", "detail": e.detail} |
There was a problem hiding this comment.
Preserve the intended 202 status for unsupported events.
The current return path converts the 202 into a 200. If clients/tests expect 202, this changes behavior.
🔧 Suggested fix
-from fastapi import APIRouter, Depends, HTTPException, Request
+from fastapi import APIRouter, Depends, HTTPException, Request
+from fastapi.responses import JSONResponse
@@
- if e.status_code == 202:
- return {"status": "event received but not supported", "detail": e.detail}
+ if e.status_code == 202:
+ return JSONResponse(
+ status_code=202,
+ content={"status": "event received but not supported", "detail": e.detail},
+ )🤖 Prompt for AI Agents
In `@src/webhooks/router.py` around lines 62 - 65, The except HTTPException
handler currently returns a normal dict which yields a 200; instead preserve the
original 202 status by returning an HTTP response with status_code set to
e.status_code (or re-raising the HTTPException) so clients receive 202 for
unsupported events; update the except block that catches "HTTPException as e" in
router.py to return a response object (or raise e) containing {"status":"event
received but not supported","detail": e.detail} and the status_code
e.status_code.
| # 3. Mock Environment Variables (Security First) | ||
| # We do this BEFORE importing app code to ensure no real secrets are read | ||
| @pytest.fixture(autouse=True) | ||
| def mock_settings(): | ||
| """Forces the test environment to use dummy values.""" | ||
| 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", | ||
| } | ||
| ): | ||
| yield | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the full conftest.py file to understand the fixture setup
cat -n tests/conftest.pyRepository: warestack/watchflow
Length of output: 2012
🏁 Script executed:
# Check for module-level imports in test files that might read env during collection
rg -n --glob 'tests/**/*.py' '^(from\s+src|import\s+src)'Repository: warestack/watchflow
Length of output: 1436
🏁 Script executed:
# Find and examine the Config class to understand validate() requirements
find . -type f -name '*.py' -path '*/src/*' | xargs grep -l "class Config" | head -5Repository: warestack/watchflow
Length of output: 92
🏁 Script executed:
# Examine Helpers.mock_env to understand how it works
find . -type f -name '*.py' | xargs grep -l "class Helpers" | head -5Repository: warestack/watchflow
Length of output: 82
🏁 Script executed:
# Examine the Config class
cat -n src/core/config/settings.py | head -100Repository: warestack/watchflow
Length of output: 4927
🏁 Script executed:
# Check what the app does when imported
cat -n src/main.py | head -50Repository: warestack/watchflow
Length of output: 2330
🏁 Script executed:
# Look for where Config is initialized or validated
rg -n "Config\(" src/ | head -20Repository: warestack/watchflow
Length of output: 936
🏁 Script executed:
# Check if APP_NAME_GITHUB is referenced anywhere
rg -n "APP_NAME_GITHUB" . --type pyRepository: warestack/watchflow
Length of output: 241
🏁 Script executed:
# Find GitHubConfig class
cat -n src/core/config/github_config.py | head -50Repository: warestack/watchflow
Length of output: 467
🏁 Script executed:
# Check if validate() is called at initialization or later
rg -n "validate\(\)" src/core/config/ -A 2 -B 2Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check the full Config class to see if there's a validate method
cat -n src/core/config/settings.py | tail -80Repository: warestack/watchflow
Length of output: 3434
🏁 Script executed:
# Run the actual tests to see if they pass or fail
cd tests && python -m pytest --collect-only 2>&1 | head -50Repository: warestack/watchflow
Length of output: 288
🏁 Script executed:
# Check if validate() is called in any test or source code
rg -n "\.validate\(\)" . --type pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there are any actual test failures related to config
rg -n "config" tests/ --type py -l | head -10 | xargs -I {} grep -n "config\." {}Repository: warestack/watchflow
Length of output: 47
🏁 Script executed:
# Try to run a simple test to see if conftest works correctly
cd tests && python -m pytest unit/test_mastra_rules_sample.py -v 2>&1 | head -100Repository: warestack/watchflow
Length of output: 288
🏁 Script executed:
# Search for validate() calls - be more specific
rg "validate\(\)" . --type py -iRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if config instance is used after module import in any critical path
rg "from src.core.config" src/ --type pyRepository: warestack/watchflow
Length of output: 1088
🏁 Script executed:
# Check what imports src.main which instantiates config
rg "from src.main import" tests/ --type pyRepository: warestack/watchflow
Length of output: 254
🏁 Script executed:
# Look at when Config() gets instantiated - trace the import chain
cat -n src/core/config/__init__.py 2>/dev/null || echo "No __init__.py in config"Repository: warestack/watchflow
Length of output: 377
🏁 Script executed:
# Check if any handler or router calls validate() when imported
grep -r "validate" src/core/config/ --include="*.py"Repository: warestack/watchflow
Length of output: 122
🏁 Script executed:
# Final check: search for any runtime usage of config that might validate implicitly
rg "config\." src/ --type py | grep -i "validate\|github\|auth" | head -20Repository: warestack/watchflow
Length of output: 2392
🏁 Script executed:
# Check pyproject.toml for test configuration to understand test setup
cat -n pyproject.toml | grep -A 20 "\[tool.pytest"Repository: warestack/watchflow
Length of output: 598
Module-level config initialization reads real environment before autouse fixture patches it.
The Config() instance is created at import time (settings.py:159), and multiple modules assign config values to module-level variables (e.g., GITHUB_WEBHOOK_SECRET = config.github.webhook_secret in webhooks/auth.py). These assignments happen during test collection, before the mock_settings fixture runs. Additionally, the mock environment dict is missing APP_NAME_GITHUB, which will be empty when Config.validate() is called.
The suggested fix is correct: set environment defaults at module level before any imports to ensure config reads capture mocked values.
🤖 Prompt for AI Agents
In `@tests/conftest.py` around lines 32 - 49, The tests create a Config() at
import time so module-level assigns like GITHUB_WEBHOOK_SECRET in
webhooks/auth.py are evaluated before the autouse fixture; to fix, set the
environment defaults as real os.environ entries at module import time (before
any app imports) and include the missing APP_NAME_GITHUB key so
Config.validate() sees it; update tests/conftest.py to populate os.environ
(e.g., os.environ.setdefault(...) or os.environ.update({... including
"APP_NAME_GITHUB": "mock-app"})) at top-level before defining or using the
mock_settings fixture so Config() and module-level assignments read the mocked
values.
| @pytest.fixture(scope="session") | ||
| def event_loop(): | ||
| import asyncio | ||
|
|
||
| policy = asyncio.get_event_loop_policy() | ||
| loop = policy.new_event_loop() | ||
| yield loop |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the conftest.py file
find . -name conftest.py -type f | head -20Repository: warestack/watchflow
Length of output: 82
🏁 Script executed:
# Check the full content of tests/conftest.py
cat tests/conftest.pyRepository: warestack/watchflow
Length of output: 1585
🏁 Script executed:
# Check if pytest-asyncio is in dependencies
find . -name "*.txt" -o -name "*.toml" -o -name "*.cfg" | xargs grep -l pytest-asyncio 2>/dev/null | head -5Repository: warestack/watchflow
Length of output: 79
🏁 Script executed:
# Search for usage of event_loop fixture in tests
rg "event_loop" --type py -B 2 -A 2 | head -50Repository: warestack/watchflow
Length of output: 463
🏁 Script executed:
# Check pyproject.toml for pytest and asyncio configuration
cat pyproject.toml | grep -A 10 "\[tool.pytest"Repository: warestack/watchflow
Length of output: 286
🏁 Script executed:
# Check if pytest-asyncio is listed as a dependency
cat pyproject.toml | grep -i "pytest-asyncio"Repository: warestack/watchflow
Length of output: 122
🏁 Script executed:
# Search for any get_event_loop() calls in tests
rg "get_event_loop\(\)" --type pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check Python version requirements
cat pyproject.toml | grep -A 5 "python ="Repository: warestack/watchflow
Length of output: 248
🏁 Script executed:
# Check if event_loop fixture is actually used in any tests
rg "def test_.*event_loop" --type pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Search for `@pytest.mark.asyncio` decorators to see how async tests are structured
rg "@pytest.mark.asyncio" --type py -B 2 -A 5 | head -40Repository: warestack/watchflow
Length of output: 2964
🏁 Script executed:
# Check if there are any async tests defined
rg "async def test_" --type py | head -10Repository: warestack/watchflow
Length of output: 1132
🏁 Script executed:
# Check pytest-asyncio version history and behavior with session scope
# Also check if there's any issue with not setting event loop
rg "set_event_loop" --type pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check the full content of a test file that uses async tests
head -100 tests/unit/test_rule_engine_agent.pyRepository: warestack/watchflow
Length of output: 3850
🏁 Script executed:
# Look for any direct asyncio.get_event_loop() calls in the source code
rg "asyncio\.get_event_loop\(\)" --type py -B 2 -A 2Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there are any GitHub issues or documentation about asyncio mode
find . -name "*.md" -type f | xargs grep -l "asyncio\|event_loop" 2>/dev/nullRepository: warestack/watchflow
Length of output: 107
🏁 Script executed:
# Let's check the pytest-asyncio documentation behavior with auto mode
# First, check if there are any fixture dependencies on event_loop
rg "event_loop" --type py -B 5 -A 5 | grep -E "(def test|@pytest|event_loop)"Repository: warestack/watchflow
Length of output: 265
🏁 Script executed:
# Check if event_loop fixture is actually passed as a parameter to any test
rg "def test_.*\(.*event_loop" --type pyRepository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there are any conftest.py files in subdirectories that might override this
find . -name conftest.py -type fRepository: warestack/watchflow
Length of output: 82
🏁 Script executed:
# Look at pytest-asyncio behavior - check if there's documentation or comments about set_event_loop
cat pyproject.toml | grep -A 20 "\[tool.pytest"Repository: warestack/watchflow
Length of output: 458
🏁 Script executed:
# Search for any imports or usage of asyncio in test files
rg "import asyncio" --type py -B 2 -A 2 | head -50Repository: warestack/watchflow
Length of output: 2157
🏁 Script executed:
# Check if the event_loop fixture might be called by pytest-asyncio internally
# or if it's truly unused
rg "conftest\|event_loop" --type py tests/ | grep -i fixture | head -20Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check pytest-asyncio documentation for session scope behavior
# Let's also see if there's any test that would benefit from explicit set_event_loop
python3 << 'EOF'
import sys
# Check Python version behavior with event loops
print(f"Python version: {sys.version}")
# In Python 3.12+, get_event_loop() behavior:
# - If called from async context: returns running loop
# - If called from sync context: returns the current event loop set via set_event_loop()
# - If no current loop: raises RuntimeError (this is the breaking change)
EOFRepository: warestack/watchflow
Length of output: 128
🏁 Script executed:
# Search for any library code that might call asyncio.get_event_loop()
rg "get_event_loop\(\)" --type py src/Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if the event_loop fixture is referenced anywhere
rg "event_loop" tests/ src/ --type py | grep -v "conftest.py" | grep -v "\.pyc"Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Look for any FastAPI TestClient or async test client setup that might need the loop
rg "TestClient\|AsyncClient\|@app" --type py tests/ | head -20Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check the full tests directory structure to understand test organization
find tests -type f -name "*.py" | head -20Repository: warestack/watchflow
Length of output: 560
🏁 Script executed:
# Review the comment in conftest.py more carefully
sed -n '45,60p' tests/conftest.pyRepository: warestack/watchflow
Length of output: 463
🏁 Script executed:
# Let's check if there are any integration tests or if any test files import from app code that might call get_event_loop()
rg "from src\|import src" tests/ --type py | head -20Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check the integration tests specifically
cat tests/integration/test_recommendations.py | head -50Repository: warestack/watchflow
Length of output: 2004
🏁 Script executed:
# Verify the fixture closing behavior - check if loop.close() is actually called
sed -n '54,70p' tests/conftest.pyRepository: warestack/watchflow
Length of output: 245
🏁 Script executed:
# Let's check if the event_loop fixture is actually used by pytest-asyncio
# by looking at pytest-asyncio usage documentation and behavior
python3 << 'EOF'
# With asyncio_mode="auto", pytest-asyncio 0.21.0+ automatically discovers
# and manages event loops for async tests. The session-scoped fixture
# definition is the old pattern.
# Check if there's a conflict between explicit session fixture and auto mode
print("pytest-asyncio auto mode behavior:")
print("- Automatically detects async tests")
print("- Creates appropriate scope event loops")
print("- If a session-scoped event_loop fixture exists, pytest uses it")
print("\nPotential issue:")
print("- If the fixture doesn't set the loop as current,")
print(" code importing at session start might fail if it calls get_event_loop()")
EOFRepository: warestack/watchflow
Length of output: 379
🏁 Script executed:
# Let's verify if there are imports that happen at module level in src/main
head -50 src/main.pyRepository: warestack/watchflow
Length of output: 1980
🏁 Script executed:
# Let's check if there are any lazy imports or module-level code that might call get_event_loop()
# Search in src/ directory for potential get_event_loop() calls that might happen at import time
rg "asyncio\." src/ --type py -B 1 -A 1 | grep -A 2 -B 2 "get_event_loop\|new_event_loop\|set_event_loop" | head -30Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Check if there's any top-level async code or task creation at import time
rg "asyncio\.create_task\|asyncio\.run" src/ --type py | head -10Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
# Final check: see if the current code runs without issues
# by checking if there are any reported test failures in CI or docs
find . -name "*.md" -o -name "*.yml" -o -name "*.yaml" | xargs grep -l "test\|asyncio\|event.loop" 2>/dev/null | head -5Repository: warestack/watchflow
Length of output: 225
🏁 Script executed:
# Check the GitHub Actions test workflow to understand how tests are run
cat .github/workflows/tests.yamlRepository: warestack/watchflow
Length of output: 1360
Add asyncio.set_event_loop() to ensure the session loop is available as the current event loop.
With asyncio_mode = "auto", pytest-asyncio manages loops per test, but the session-scoped fixture should explicitly set the loop as current. In Python 3.12+, code that calls asyncio.get_event_loop() at module/session import time will raise RuntimeError if no current loop is set. The fixture should also clean up by setting the loop to None on teardown.
Suggested fix
policy = asyncio.get_event_loop_policy()
loop = policy.new_event_loop()
+ asyncio.set_event_loop(loop)
yield loop
loop.close()
+ asyncio.set_event_loop(None)🤖 Prompt for AI Agents
In `@tests/conftest.py` around lines 54 - 60, The session-scoped pytest fixture
event_loop currently creates a new loop but does not set it as the current event
loop; modify the fixture to call asyncio.set_event_loop(loop) after creating
loop so code using asyncio.get_event_loop() at import/session time works, and on
teardown (after yield) call asyncio.set_event_loop(None) and close the loop
(loop.close()) to clean up; keep references to the existing symbols (policy,
loop, event_loop) so the change is local to that fixture.
2 participants
Fix CI/CD Pipeline Failures
mkdocs: command not foundif: github.repository == 'warestack/watchflow'Summary by CodeRabbit
New Features
Documentation
Infrastructure
Tests
✏️ Tip: You can customize this high-level summary in your review settings.