Repository navigation
Release 0.2.1: resolve security & CI blockers (PR #356 review findings) - #357
canstralian wants to merge 26 commits into
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: WhacktheJacker <8595080+canstralian@users.noreply.github.com>
Docstrings generation was requested by @canstralian. The following files were modified: * `core/cli.py` * `core/logging.py` * `core/parser/engine.py` * `core/parser/tests/test_parser.py` * `core/server.py` * `plugins/anthropic_code_suggester.py` * `plugins/code_analyzer.py` * `plugins/openai_code_analyzer.py` * `scripts/cleanup_stale_prs.py` * `scripts/validate_workflows.py` * `tests/test_code_analyzer.py` * `tests/test_workflow_security.py` * `tests/test_workflow_simulation.py` * `tests/test_workflows.py` * `utils/__init__.py` * `utils/argilla_dataset.py` * `utils/documentation.py` These files were kept as they were: * `components/tokenizer_builder.py` * `scripts/validate_pyproject.py` * `tests/test_anthropic_code_suggester.py` * `tests/test_openai_code_analyzer.py` These file types are not supported: * `.github/workflows/pr-checklist-status.yml` * `.github/workflows/python-style-checks.yml` * `.github/workflows/security.yml`
Address the unresolved review findings on the 0.2.1 release-blocker stream (PR #356) and prepare release metadata. Security: - core/logging.redact_url: fully mask URL credentials as ***:*** so the database username is no longer emitted in clear text (Packet 1). - validate_workflows.py: wrap YAML loading in error handling and reject non-mapping documents; broaden hardcoded-secret detection (unquoted keys + api_key); make pull_request_target a hard error; flag actions not pinned to a full commit SHA (mutable tag pins -> warning). Reliability: - core/parser/engine: instantiate PyJsParser per call (thread-safe) and validate non-string code/filename inputs, returning structured ParseResult errors instead of raising (Packet 2). Tests updated + added. - scripts/cleanup_stale_prs.py: add timeouts to GitHub API calls. - plugins/code_analyzer: log swallowed parse/analysis exceptions while keeping generic user-facing error payloads. Quality: - Apply Black formatting and resolve Ruff ANN202/EM102 findings across touched modules. Release: - Bump version 0.2.0 -> 0.2.1 (core.__version__, test assertion) and add CHANGELOG 0.2.1 entry.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe PR updates parser behavior, plugin resilience, workflow validation, CI and security checks, release metadata, maintenance scripts, tooling configuration, and checked-in virtual-environment artifacts. Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
| ) | ||
|
|
||
| def _validate_best_practices(self, filename: str, content: dict[str, Any]) -> None: | ||
| """ |
There was a problem hiding this comment.
Code Review
This pull request updates CodeTune Studio to version 0.2.1, introducing credential redaction in logs, thread-safe JavaScript parsing, lazy-loaded utility modules, and hardened workflow validation. It also adds comprehensive docstrings and unit tests across several modules. The code review identified critical issues in the Argilla dataset loading logic, where incorrect handling of dictionary structures will cause data loss or crashes. Additionally, missing type checks in the workflow validator when processing malformed jobs sections could lead to AttributeError crashes, and a type hint in the documentation parser needs to be broadened to support asynchronous functions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Get the text from the first text field | ||
| text_fields = [f.value for f in record.fields if hasattr(f, 'value')] | ||
| text_fields = [f.value for f in record.fields if hasattr(f, "value")] | ||
| dataset_dict["text"].append(text_fields[0] if text_fields else "") | ||
|
|
||
| # Get responses/annotations if available | ||
| responses = record.responses if hasattr(record, 'responses') else [] | ||
| responses = record.responses if hasattr(record, "responses") else [] | ||
| # Safely extract values from first response | ||
| label_value = None | ||
| if responses and len(responses) > 0: | ||
| label_value = getattr(responses[0], 'values', None) | ||
| label_value = getattr(responses[0], "values", None) | ||
| dataset_dict["label"].append(label_value) |
There was a problem hiding this comment.
In Argilla 2.x, record.fields is a standard Python dictionary of field names to string values, and record.responses is a dictionary mapping question names to lists of response objects.
The current implementation treats record.fields as a list of objects with a .value attribute, which will result in text_fields always being empty and all text columns being populated with empty strings.
Furthermore, treating record.responses as a list and indexing it with responses[0] will raise a KeyError and crash the dataset loading process when responses are present.
Updating this logic to correctly handle Argilla 2.x dictionary structures resolves both the data loss and the crash.
| # Get the text from the first text field | |
| text_fields = [f.value for f in record.fields if hasattr(f, 'value')] | |
| text_fields = [f.value for f in record.fields if hasattr(f, "value")] | |
| dataset_dict["text"].append(text_fields[0] if text_fields else "") | |
| # Get responses/annotations if available | |
| responses = record.responses if hasattr(record, 'responses') else [] | |
| responses = record.responses if hasattr(record, "responses") else [] | |
| # Safely extract values from first response | |
| label_value = None | |
| if responses and len(responses) > 0: | |
| label_value = getattr(responses[0], 'values', None) | |
| label_value = getattr(responses[0], "values", None) | |
| dataset_dict["label"].append(label_value) | |
| # Get the text from the first text field | |
| text_fields = list(record.fields.values()) if isinstance(record.fields, dict) else [] | |
| dataset_dict["text"].append(text_fields[0] if text_fields else "") | |
| # Get responses/annotations if available | |
| responses = record.responses if hasattr(record, "responses") else {} | |
| # Safely extract values from first response | |
| label_value = None | |
| if isinstance(responses, dict) and responses: | |
| first_response_list = next(iter(responses.values())) | |
| if first_response_list and len(first_response_list) > 0: | |
| label_value = getattr(first_response_list[0], "value", None) | |
| dataset_dict["label"].append(label_value) |
| jobs = content.get("jobs", {}) | ||
| has_job_permissions = any( | ||
| isinstance(job, dict) and "permissions" in job for job in jobs.values() | ||
| ) |
There was a problem hiding this comment.
If the jobs section is not a dictionary (e.g., if it is parsed as a list or a string due to invalid YAML structure), calling jobs.values() will raise an AttributeError and crash the validator. Adding a type check prevents unexpected crashes when scanning malformed workflows.
jobs = content.get("jobs")
has_job_permissions = False
if isinstance(jobs, dict):
has_job_permissions = any(
isinstance(job, dict) and "permissions" in job for job in jobs.values()
)| jobs = content.get("jobs", {}) | ||
| for job_name, job_config in jobs.items(): |
There was a problem hiding this comment.
If the workflow's jobs section is not a dictionary, calling jobs.items() will raise an AttributeError and crash the validator script. Adding a type check ensures the validator handles malformed structures gracefully.
| jobs = content.get("jobs", {}) | |
| for job_name, job_config in jobs.items(): | |
| jobs = content.get("jobs") | |
| if not isinstance(jobs, dict): | |
| return | |
| for job_name, job_config in jobs.items(): |
| logger.exception("Failed to parse documentation source: %s", file_path) | ||
| return [] | ||
|
|
||
| def _get_function_signature(self, node: ast.FunctionDef) -> str: |
There was a problem hiding this comment.
The _get_function_signature method is called with both ast.FunctionDef and ast.AsyncFunctionDef nodes in parse_file. Updating the type hint to accept both types prevents static type checking errors (e.g., from mypy).
| def _get_function_signature(self, node: ast.FunctionDef) -> str: | |
| def _get_function_signature(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> str: |
…ned) CI pins black==25.11.0, which splits triple-quoted string arguments onto their own lines (closing paren on a separate line). Match that form in tokenizer_builder.py and core/server.py so the style-check Black gate passes.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
utils/argilla_dataset.py (1)
59-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix logging calls that currently fail lint gates
Line 59, Line 69, Line 146, and Line 183 use
logger.exceptionwith f-strings and
redundant exception interpolation; Line 140 and Line 178 also use f-strings in
logging. These are currently hard CI failures.As per coding guidelines, "Use PEP 8 compliance with max line length of 88 characters (Black formatter compatible)" and avoid quality regressions that break CI checks.
Suggested patch
- except Exception as e: - logger.exception(f"Failed to initialize Argilla client: {e!s}") + except Exception: + logger.exception("Failed to initialize Argilla client") raise @@ - except Exception as e: - logger.exception(f"Failed to list datasets: {e!s}") + except Exception: + logger.exception("Failed to list datasets") return [] # Return empty list on error instead of raising @@ - logger.info( - f"Successfully loaded dataset '{dataset_name}' with " - f"{len(hf_dataset)} records" - ) + logger.info( + "Successfully loaded dataset '%s' with %d records", + dataset_name, + len(hf_dataset), + ) return hf_dataset @@ - except Exception as e: - logger.exception(f"Failed to load dataset '{dataset_name}': {e!s}") + except Exception: + logger.exception("Failed to load dataset '%s'", dataset_name) raise @@ - logger.info( - f"Dataset prepared for training with {len(dataset)} valid examples" - ) + logger.info( + "Dataset prepared for training with %d valid examples", + len(dataset), + ) return dataset @@ - except Exception as e: - logger.exception(f"Failed to prepare dataset for training: {e!s}") + except Exception: + logger.exception("Failed to prepare dataset for training") raiseAlso applies to: 69-70, 139-142, 146-147, 177-179, 183-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/argilla_dataset.py` around lines 59 - 60, Replace all logging calls that use f-strings or interpolate exception objects directly with proper logger API usage: for exception cases (currently logger.exception(f"...{e}") at the sites around the former lines 59, 69, 146, 183) drop the f-string and call logger.exception("Failed to initialize Argilla client") (or the appropriate static message) so the traceback is automatically included; for non-exception logs that use f-strings (around former lines 140 and 178 and the grouped ranges 69-70, 139-142, 146-147, 177-179, 183-184) replace f-strings with parameterized logging like logger.error("...: %s", value) or logger.info("...: %s", value) and wrap long messages to respect the 88-character line limit; ensure no call passes the exception via string interpolation and do not add redundant exc_info when using logger.exception.Sources: Coding guidelines, Pipeline failures
tests/test_core_package.py (1)
77-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRuff blocker: remove or use the unused
__version__import at Line 77.
test_cli_version_flagimports__version__but never references it, and the style job fails with F401.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_core_package.py` at line 77, The test imports __version__ but never uses it, causing an F401; either remove the unused import statement or use it by updating test_cli_version_flag to assert the CLI version output equals __version__ (capture output from CliRunner.invoke and compare result.output.strip() or result.stdout to __version__); specifically edit the import line (remove "__version__") or modify test_cli_version_flag to reference the __version__ symbol in the assertion so the import is used.Source: Pipeline failures
tests/test_workflow_security.py (1)
101-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
_check_secrets_usageaccepts non-secret expressions as valid for sensitive vars.This check only requires
"${{"in the value. Expressions like${{ github.actor }}would pass forAPI_KEY/HF_TOKENeven though they are not secrets. Enforcesecrets.(and optionallygithub.tokenonly forGITHUB_TOKEN) to match the test’s intent.As per coding guidelines, "Never hardcode secrets, API keys, tokens, passwords, or credentials in code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_workflow_security.py` around lines 101 - 103, The _check_secrets_usage helper currently accepts any GitHub expression because it only checks for the substring "${{" in value_str; change it to require the expression reference the secrets namespace (i.e. ensure value_str contains an expression that includes "secrets.<NAME>") and only allow "github.token" when the key is exactly "GITHUB_TOKEN"; update the logic in _check_secrets_usage to validate value_str accordingly (e.g. match an expression like ${{ secrets.xxx }} or, if key == "GITHUB_TOKEN", allow ${{ github.token }}), and fail the test otherwise using the existing self.fail(f"{filename}: {key} should use GitHub secrets, found: {value}") so sensitive vars (key, value_str, filename) are correctly validated.Source: Coding guidelines
tests/test_workflow_simulation.py (1)
30-30:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSeveral updated docstrings/strings are breaking Ruff (E501 and W605).
The changed text introduces multiple line-length failures, and the regex example in a normal docstring triggers invalid escape warnings.
✂️ Suggested pattern for fixes
- Executes the command `ruff check . --ignore E501` in the repository root. Skips the test if Ruff is not installed and fails the test if the command times out. + Executes `ruff check . --ignore E501` in the repository root. + Skips the test if Ruff is not installed and fails if the command + times out. - Attempts to import core.__version__ from the repository root and, on success, asserts that the value matches the regular expression '^\d+\.\d+\.\d+$'. If the import fails, the test is skipped with the import error message. + Attempts to import core.__version__ from the repository root and + asserts it matches '^\\d+\\.\\d+\\.\\d+$'. If import fails, the test + is skipped with the import error message.As per coding guidelines, "Use PEP 8 compliance with max line length of 88 characters (Black formatter compatible)".
Also applies to: 50-50, 81-81, 124-124, 179-181, 221-223, 255-255, 263-265, 281-284
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_workflow_simulation.py` at line 30, Several updated docstrings/strings exceed the 88-char line limit and include unescaped backslashes that trigger Ruff E501 and W605 (e.g., the list entry string 'black --check --diff --line-length=88 --exclude "app\\.py|index\\.html" .'). Fix by splitting long literal strings so no source line exceeds 88 chars (use implicit concatenation inside parentheses or join short strings) and convert regex-containing literals to raw strings (r'...') or properly escape backslashes (double them) in docstrings and test strings; apply these changes to the modified docstrings/strings referenced in the review so all occurrences conform to PEP8/Black line length and remove invalid escape sequences.Sources: Coding guidelines, Pipeline failures
🧹 Nitpick comments (1)
core/parser/tests/test_parser.py (1)
5-8: ⚡ Quick winAdd return type hints to test function signatures.
These test functions (and
BrokenParser.parse) are missing type annotations.
Please add explicit return types (e.g.,-> None) and argument types where applicable.As per coding guidelines, "Use type hints for all Python function signatures."
Also applies to: 11-16, 19-25, 28-55, 57-61, 64-68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/parser/tests/test_parser.py` around lines 5 - 8, Add explicit type hints to the test functions and the parser method: for each test function (e.g., test_language_detection) add a return type -> None and annotate any parameters if present; for the BrokenParser.parse method add parameter and return type annotations (e.g., def parse(self, code: str) -> Any or the appropriate parsed-type) so the signature is fully typed. Ensure all function signatures mentioned in the review (including the other test functions referenced and BrokenParser.parse) have explicit type hints per the project's typing guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 8: The changelog header "[0.2.1] - 2026-06-13" uses reference-link syntax
but the corresponding reference definition is missing; open CHANGELOG.md and add
a bottom reference entry for [0.2.1] (matching the format used by other
releases, e.g. "[0.2.1]: <compare_or_release_URL>") so the link resolves
correctly, and ensure any other duplicate occurrences of the same heading are
satisfied by this single reference (no further code changes needed).
In `@core/logging.py`:
- Around line 87-90: In core.logging.redact_url, accessing parts.port can raise
ValueError for non-numeric ports; update redact_url to safely handle this by
wrapping retrieval of parts.port in a try/except (catch ValueError) or by using
a helper that converts parts.port access to None on error, then only append
f":{parts.port}" when the port value was successfully obtained; preserve the
existing username/password masking logic (the redacted_netloc and
"***:***@{redacted_netloc}" code) and treat invalid ports as "no port" (omit or
mask) so redaction never raises during startup.
In `@core/parser/engine.py`:
- Around line 43-44: The current shebang detection always returns Language.BASH
when code.startswith("#!"), which misclassifies Python/Node scripts; update the
shebang handling to parse the interpreter token after "#!" (e.g., check for
substrings like "python", "python3", "node", "nodejs", "bash", "sh") and return
the appropriate Language enum instead of always Language.BASH, and then ensure
parse_code() uses that returned language to route to the implemented Python/JS
parsers (adjust parse_code()'s routing for the language values if necessary so
Python/Node shebangs no longer go to the unimplemented parser path).
In `@plugins/anthropic_code_suggester.py`:
- Around line 23-25: The long docstring and string literals in the
AnthropicCodeSuggesterTool initializer exceed the 88-character line length
limit; wrap those long docstring/log-message lines so each physical line is <=88
characters (break sentences or clauses across lines), and adjust any
concatenated log strings used in __init__ (e.g., messages that set self.client
to None and warnings) and the self.metadata description entries so they conform
to PEP8/Black max line length; keep the same wording and variable names
(AnthropicCodeSuggesterTool, self.client, self.metadata) but split long strings
into multiple shorter literal lines or use implicit string literal
concatenation.
- Around line 50-60: The validate_inputs function currently only checks
isinstance(inputs.get("code"), str); update validate_inputs to (1) define a
MAX_CODE_LENGTH constant (e.g. 100_000) and optionally allowed_min_length (e.g.
1), (2) fetch raw = inputs.get("code") and return False if not isinstance(raw,
str), (3) sanitize by removing NULs and undesirable control characters (e.g. raw
= raw.replace("\x00", "") or use a regex to strip control chars) and collapse
excessive whitespace if desired, (4) compute trimmed = raw.strip() and return
False if len(trimmed) < allowed_min_length or len(trimmed) > MAX_CODE_LENGTH,
and (5) only return True when the sanitized trimmed string meets the length
bounds; otherwise return False. This keeps the function name validate_inputs and
ensures earlier rejection of whitespace-only or overly large code strings before
calling the external model API.
In `@plugins/code_analyzer.py`:
- Line 17: Long literal and constructor call for self.metadata exceeds the
88-character line limit; break the ToolMetadata instantiation across multiple
lines so each line is <=88 chars (for example, pass arguments on separate lines
or assign long string values to shorter-named variables before constructing
ToolMetadata). Update the code that sets self.metadata (the ToolMetadata(...)
call and its arguments: name, description, version, author, tags) to use
multi-line formatting or implicit string concatenation so every source line
conforms to PEP 8 max-length 88.
In `@plugins/openai_code_analyzer.py`:
- Line 29: Shorten and wrap any lines exceeding 88 chars in
plugins/openai_code_analyzer.py around the OpenAICodeAnalyzerTool initialization
and OpenAI client configuration (notably the lines that set the tool's metadata
and configure the client) so they conform to PEP8/Black width: break long string
literals using implicit concatenation or parentheses, split long argument lists
across multiple lines, or assign long expressions to intermediate variables;
ensure symbols referenced include OpenAICodeAnalyzerTool and the OpenAI
client/configuration call so all affected lines (near the tool initialization
and metadata setup) are reformatted to <=88 characters and pass Ruff E501.
- Around line 57-67: The validate_inputs function currently only checks type;
update validate_inputs to (1) retrieve code = inputs.get("code"), return False
if not isinstance(code, str), (2) sanitize with code_stripped = code.strip() and
reject if code_stripped == "" (to disallow all-whitespace input), and (3)
enforce a max length by returning False if len(code_stripped) > MAX_CODE_LENGTH
(define MAX_CODE_LENGTH as an appropriate constant, e.g. 10000). Keep the final
return value True only when the sanitized string passes these checks; reference
the function name validate_inputs and the MAX_CODE_LENGTH symbol when making the
change.
In `@scripts/cleanup_stale_prs.py`:
- Around line 193-205: The docstring for the function that closes pull requests
(the triple-quoted block describing "Close the pull requests listed for a given
category." and the Parameters: category, pr_numbers, dry_run, and Returns:
Dict[str, int]) has several lines longer than 88 characters; reflow the text so
every line is <=88 chars (wrap long sentences, break parameter descriptions
across lines as needed), preserve the docstring content and indentation, and
ensure the "Parameters" and "Returns" sections remain PEP 8/NumPy-style readable
while not exceeding the 88-char limit.
- Around line 190-205: The docstring for close_prs_by_category is missing
documentation for the token parameter; update the Parameters section to include
token (str): a GitHub API token used to authenticate requests (or a brief note
about its use/scope), following the existing Google/NumPy style and matching the
formatting of the other parameters so tools/readers parse it correctly.
In `@scripts/validate_pyproject.py`:
- Around line 36-37: The long compatibility comment immediately above the
tomli/tomllib import (the line starting "# Project uses Python 3.10, so tomli is
expected, but tomllib provides forward compatibility") exceeds 88 characters;
wrap it into two or more shorter comment lines (each starting with "#") so no
line exceeds 88 chars, preserving the same wording/meaning and placing the
wrapped lines directly above the tomli/tomllib import.
In `@scripts/validate_workflows.py`:
- Around line 203-205: The secret-detection regexes that currently require a
starting quote (the tuples matching r"password\s*[:=]\s*['\"](?!.*\$\{)",
r"token\s*[:=]\s*['\"](?!.*\$\{)", r"api[_-]?key\s*[:=]\s*['\"](?!.*\$\{)") miss
unquoted YAML values like `token: abc123`; update these patterns so they allow
an optional quote and also match unquoted non-whitespace values while still
excluding templated values (the existing (?!.*\$\{) check). Replace the three
regexes with equivalents that accept either quoted or unquoted values (e.g.,
make the quote optional and match \S+ for the value) and apply the same change
to the other instances noted in the comment (the similar tuples at lines
210-213).
- Around line 86-91: Prevent directory traversal by validating workflow_name
before joining: ensure workflow_name is not absolute and does not contain '..',
enforce allowed extensions (.yml/.yaml), then resolve the candidate path and
verify it is inside self.workflows_dir (e.g., compare workflow_file.resolve()
with self.workflows_dir.resolve() or use Path.is_relative_to); if any check
fails append the error and return []. Additionally, guard the later call to
relative_to(self.repo_root) (the code around the existing workflow_file and
self.repo_root usage) with a try/except ValueError and treat a ValueError as a
validation failure (append error and return []) so traversal or escape cannot
raise and abort validation.
- Around line 22-25: Several docstring lines in this module exceed the
88-character PEP 8 limit and must be wrapped; edit the long docstrings (starting
with the WorkflowValidator __init__ docstring and any other
function/module/method docstrings in this file) so that no line is longer than
88 characters (break sentences between words, keep indentation and triple-quote
formatting intact), then run the formatter/linter (black/flake8) to confirm the
E501 violations are resolved.
In `@tests/test_code_analyzer.py`:
- Line 73: Wrap the long test docstring lines in the test that "Verifies that
executing the analyzer with a non-string `code` input returns an error result"
so no line exceeds 88 characters; edit the triple-quoted docstring text (and the
other long docstrings referenced) to break sentences into shorter lines or
reflow them to follow PEP8/Black max line length rules while preserving the
original wording and meaning.
In `@tests/test_workflow_security.py`:
- Around line 76-78: Several lines in the test that validates sensitive env vars
(the docstring starting "Validate that sensitive environment variables in a
parsed GitHub Actions workflow..." and the surrounding assertions/strings used
to check job-level and step-level env keys like GITHUB_TOKEN, HF_TOKEN,
PYPI_API_TOKEN, API_KEY) exceed the 88-character limit; wrap long string
literals and long assertion/format expressions to <=88 columns by breaking
strings into implicit concatenation inside parentheses or using multiline
f-strings, and split long boolean/regex expressions across lines with proper
indentation so the test message strings and conditionals (the docstring,
assertion messages, and any long regex/format expressions used to detect `${{`)
all conform to PEP8 line-length limits. Ensure no logic changes, only reflowing
lines and adjusting indentation.
In `@tests/test_workflows.py`:
- Around line 28-32: The module/test docstring that starts with "Verify that
every workflow file under the workflows directory contains valid, non-empty
YAML." is exceeding the 88-character line length; reformat the triple-quoted
docstring text to wrap at 88 characters (preserve sentences and indentation
within the triple quotes), splitting long lines into multiple shorter lines so
the docstring remains identical in content but conforms to the 88-char limit.
In `@utils/argilla_dataset.py`:
- Around line 37-40: Docstrings and unused compatibility arguments in
utils/argilla_dataset.py violate PEP8 (max 88 chars) and trigger unused-argument
errors: shorten/wrap any long docstring lines (the descriptions referenced
around the class or function docstrings) so no line exceeds 88 characters, and
silence the unused parameters `query` and `filter_by` by renaming them to
`_query` and `_filter_by` (or prefixing them with an underscore in the function
signatures where they appear) so linters accept them; keep behavior identical
and do not remove the compatibility parameters. Update the docstrings for
initialization / logging messages to wrap text under 88 characters and ensure
the docstrings still mention `api_url`/`api_key` without exceeding the line
limit.
In `@utils/documentation.py`:
- Line 33: Shorten long docstring/comment and any overlong code lines in the
parse_file function and the signature-building code to comply with 88-character
line length; replace use of open(path, ...) with Path.open() when opening a Path
object (e.g., in parse_file where the file is read) and simplify the block that
builds the signature list (currently four lines) into a single list
comprehension to produce the same list of signatures. Reference the parse_file
function and the signature builder helper (the function that creates DocItem
signatures) when making these edits so you only reformat lines, swap open(...)
for Path.open(), and collapse the multi-line append loop into a concise
comprehension.
---
Outside diff comments:
In `@tests/test_core_package.py`:
- Line 77: The test imports __version__ but never uses it, causing an F401;
either remove the unused import statement or use it by updating
test_cli_version_flag to assert the CLI version output equals __version__
(capture output from CliRunner.invoke and compare result.output.strip() or
result.stdout to __version__); specifically edit the import line (remove
"__version__") or modify test_cli_version_flag to reference the __version__
symbol in the assertion so the import is used.
In `@tests/test_workflow_security.py`:
- Around line 101-103: The _check_secrets_usage helper currently accepts any
GitHub expression because it only checks for the substring "${{" in value_str;
change it to require the expression reference the secrets namespace (i.e. ensure
value_str contains an expression that includes "secrets.<NAME>") and only allow
"github.token" when the key is exactly "GITHUB_TOKEN"; update the logic in
_check_secrets_usage to validate value_str accordingly (e.g. match an expression
like ${{ secrets.xxx }} or, if key == "GITHUB_TOKEN", allow ${{ github.token
}}), and fail the test otherwise using the existing self.fail(f"{filename}:
{key} should use GitHub secrets, found: {value}") so sensitive vars (key,
value_str, filename) are correctly validated.
In `@tests/test_workflow_simulation.py`:
- Line 30: Several updated docstrings/strings exceed the 88-char line limit and
include unescaped backslashes that trigger Ruff E501 and W605 (e.g., the list
entry string 'black --check --diff --line-length=88 --exclude
"app\\.py|index\\.html" .'). Fix by splitting long literal strings so no source
line exceeds 88 chars (use implicit concatenation inside parentheses or join
short strings) and convert regex-containing literals to raw strings (r'...') or
properly escape backslashes (double them) in docstrings and test strings; apply
these changes to the modified docstrings/strings referenced in the review so all
occurrences conform to PEP8/Black line length and remove invalid escape
sequences.
In `@utils/argilla_dataset.py`:
- Around line 59-60: Replace all logging calls that use f-strings or interpolate
exception objects directly with proper logger API usage: for exception cases
(currently logger.exception(f"...{e}") at the sites around the former lines 59,
69, 146, 183) drop the f-string and call logger.exception("Failed to initialize
Argilla client") (or the appropriate static message) so the traceback is
automatically included; for non-exception logs that use f-strings (around former
lines 140 and 178 and the grouped ranges 69-70, 139-142, 146-147, 177-179,
183-184) replace f-strings with parameterized logging like logger.error("...:
%s", value) or logger.info("...: %s", value) and wrap long messages to respect
the 88-character line limit; ensure no call passes the exception via string
interpolation and do not add redundant exc_info when using logger.exception.
---
Nitpick comments:
In `@core/parser/tests/test_parser.py`:
- Around line 5-8: Add explicit type hints to the test functions and the parser
method: for each test function (e.g., test_language_detection) add a return type
-> None and annotate any parameters if present; for the BrokenParser.parse
method add parameter and return type annotations (e.g., def parse(self, code:
str) -> Any or the appropriate parsed-type) so the signature is fully typed.
Ensure all function signatures mentioned in the review (including the other test
functions referenced and BrokenParser.parse) have explicit type hints per the
project's typing guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b94444d-a840-40a7-9d54-75cacc46448b
📒 Files selected for processing (28)
.github/workflows/pr-checklist-status.yml.github/workflows/python-style-checks.yml.github/workflows/security.ymlCHANGELOG.mdcore/__init__.pycore/cli.pycore/logging.pycore/parser/__init__.pycore/parser/engine.pycore/parser/tests/test_parser.pycore/parser/types.pycore/server.pyplugins/anthropic_code_suggester.pyplugins/code_analyzer.pyplugins/openai_code_analyzer.pyscripts/cleanup_stale_prs.pyscripts/validate_pyproject.pyscripts/validate_workflows.pytests/test_anthropic_code_suggester.pytests/test_code_analyzer.pytests/test_core_package.pytests/test_openai_code_analyzer.pytests/test_workflow_security.pytests/test_workflow_simulation.pytests/test_workflows.pyutils/__init__.pyutils/argilla_dataset.pyutils/documentation.py
| def validate_inputs(self, inputs: dict[str, Any]) -> bool: | ||
| """ | ||
| Check that inputs include a 'code' field containing the source code as a string. | ||
|
|
||
| Parameters: | ||
| inputs (dict[str, Any]): Input mapping; must contain a 'code' key with the source code. | ||
|
|
||
| Returns: | ||
| bool: True if 'code' exists in inputs and is a `str`, False otherwise. | ||
| """ | ||
| return isinstance(inputs.get("code"), str) |
There was a problem hiding this comment.
Harden code input validation with sanitization and bounds.
validate_inputs currently accepts whitespace-only and unbounded strings. Add
sanitization and a maximum length guard before invoking the external model API.
Suggested patch
class AnthropicCodeSuggesterTool(AgentTool):
@@
+ MAX_CODE_LENGTH = 100_000
+
def validate_inputs(self, inputs: dict[str, Any]) -> bool:
@@
- return isinstance(inputs.get("code"), str)
+ code = inputs.get("code")
+ return (
+ isinstance(code, str)
+ and bool(code.strip())
+ and len(code) <= self.MAX_CODE_LENGTH
+ )As per coding guidelines, Always validate data types (str, int, float, bool), check value ranges, and sanitize string inputs and Never trust user input without validation and sanitization.
🧰 Tools
🪛 GitHub Actions: Python Style Checks / 0_style-check.txt
[error] 55-55: Ruff E501 line too long (99 > 88 characters)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/anthropic_code_suggester.py` around lines 50 - 60, The
validate_inputs function currently only checks isinstance(inputs.get("code"),
str); update validate_inputs to (1) define a MAX_CODE_LENGTH constant (e.g.
100_000) and optionally allowed_min_length (e.g. 1), (2) fetch raw =
inputs.get("code") and return False if not isinstance(raw, str), (3) sanitize by
removing NULs and undesirable control characters (e.g. raw = raw.replace("\x00",
"") or use a regex to strip control chars) and collapse excessive whitespace if
desired, (4) compute trimmed = raw.strip() and return False if len(trimmed) <
allowed_min_length or len(trimmed) > MAX_CODE_LENGTH, and (5) only return True
when the sanitized trimmed string meets the length bounds; otherwise return
False. This keeps the function name validate_inputs and ensures earlier
rejection of whitespace-only or overly large code strings before calling the
external model API.
Source: Coding guidelines
| Uses the provided `api_url` and `api_key` when given; otherwise falls back to the | ||
| ARGILLA_API_URL and ARGILLA_API_KEY environment variables (ARGILLA_API_URL | ||
| defaults to "http://localhost:6900" if not set). Logs initialization success, | ||
| and on failure logs the error without exposing the `api_key` value and re-raises |
There was a problem hiding this comment.
Resolve remaining lint blockers in docstrings and unused compatibility args
Line 37/45/83/84/87/90/93 exceed the 88-char limit, and Line 75-76 trigger
unused-argument errors (query, filter_by). These are active CI blockers.
As per coding guidelines, "Use PEP 8 compliance with max line length of 88 characters (Black formatter compatible)."
Suggested patch
- Uses the provided `api_url` and `api_key` when given; otherwise falls back to the
- ARGILLA_API_URL and ARGILLA_API_KEY environment variables (ARGILLA_API_URL
+ Uses the provided `api_url` and `api_key` when given; otherwise falls
+ back to ARGILLA_API_URL and ARGILLA_API_KEY environment variables.
+ ARGILLA_API_URL
defaults to "http://localhost:6900" if not set). Logs initialization success,
@@
- api_key (str | None): Optional API key for authentication (sensitive; never logged).
+ api_key (str | None): Optional API key for authentication
+ (sensitive; never logged).
@@
- query (str | None): Optional query parameter accepted for compatibility but not used by this implementation.
- filter_by (dict[str, Any] | None): Optional filters accepted for compatibility but not used by this implementation.
+ query (str | None): Optional compatibility parameter;
+ currently not used.
+ filter_by (dict[str, Any] | None): Optional compatibility
+ filters; currently not used.
@@
- Dataset: A Hugging Face Dataset containing three columns: `text`, `label`, and `metadata`.
+ Dataset: A Hugging Face Dataset with `text`, `label`,
+ and `metadata` columns.
@@
- ValueError: If a dataset with `dataset_name` is not found in the Argilla client.
+ ValueError: If `dataset_name` is not found in the Argilla client.
@@
- This method loads all dataset records into memory; for very large datasets consider using pagination or streaming.
+ This method loads all records in memory; use pagination or
+ streaming for very large datasets.
@@
def load_dataset(
self,
dataset_name: str,
query: str | None = None,
filter_by: dict[str, Any] | None = None,
) -> Dataset:
@@
+ _ = (query, filter_by)
try:Also applies to: 45-46, 83-84, 87-87, 90-90, 93-93, 75-76
🧰 Tools
🪛 GitHub Actions: Python Style Checks / 0_style-check.txt
[error] 37-37: Ruff E501 line too long (89 > 88 characters)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@utils/argilla_dataset.py` around lines 37 - 40, Docstrings and unused
compatibility arguments in utils/argilla_dataset.py violate PEP8 (max 88 chars)
and trigger unused-argument errors: shorten/wrap any long docstring lines (the
descriptions referenced around the class or function docstrings) so no line
exceeds 88 characters, and silence the unused parameters `query` and `filter_by`
by renaming them to `_query` and `_filter_by` (or prefixing them with an
underscore in the function signatures where they appear) so linters accept them;
keep behavior identical and do not remove the compatibility parameters. Update
the docstrings for initialization / logging messages to wrap text under 88
characters and ensure the docstrings still mention `api_url`/`api_key` without
exceeding the line limit.
Sources: Coding guidelines, Pipeline failures
|
Note Docstrings generation - SUCCESS |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Docstrings generation was requested by @canstralian. The following files were modified: * `core/cli.py` * `core/parser/engine.py` * `core/server.py` * `plugins/anthropic_code_suggester.py` * `plugins/code_analyzer.py` * `plugins/openai_code_analyzer.py` * `scripts/cleanup_stale_prs.py` * `scripts/validate_pyproject.py` * `scripts/validate_workflows.py` * `tests/test_code_analyzer.py` * `tests/test_openai_code_analyzer.py` * `tests/test_workflow_simulation.py` * `tests/test_workflows.py` * `utils/argilla_dataset.py` These files were kept as they were: * `core/logging.py` * `core/parser/tests/test_parser.py` * `tests/test_anthropic_code_suggester.py` * `tests/test_core_package.py` * `tests/test_workflow_security.py` * `utils/__init__.py` * `utils/documentation.py` These file types are not supported: * `.github/workflows/pr-checklist-status.yml` * `.github/workflows/python-style-checks.yml` * `.github/workflows/security.yml` * `CHANGELOG.md`
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
✅ Unit tests committed locally. Commit: |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Docstrings generation was requested by @coderabbitai[bot]. The following files were modified: * `core/logging.py` * `core/parser/engine.py` * `core/parser/tests/test_parser.py` * `core/server.py` * `plugins/code_analyzer.py` * `plugins/openai_code_analyzer.py` * `scripts/cleanup_stale_prs.py` * `scripts/validate_pyproject.py` * `scripts/validate_workflows.py` * `tests/test_core_package.py` * `tests/test_validate_workflows_unit.py` * `tests/test_workflow_security.py` * `tests/test_workflow_simulation.py` * `tests/test_workflows.py` * `utils/__init__.py` * `utils/documentation.py` These files were kept as they were: * `core/cli.py` * `plugins/anthropic_code_suggester.py` * `tests/test_anthropic_code_suggester.py` * `tests/test_code_analyzer.py` * `tests/test_openai_code_analyzer.py` * `utils/argilla_dataset.py` These file types are not supported: * `.github/workflows/pr-checklist-status.yml` * `.github/workflows/python-style-checks.yml` * `.github/workflows/security.yml` * `CHANGELOG.md`
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_eeda8d52-6f07-4492-bbd8-352f073521e4) |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
There was a problem hiding this comment.
Stale comment
Risk: high. Left a non-blocking comment (not approving): Cursor Bugbot and Security Agent checks completed as skipped with unresolved high/medium findings, CodeQL failed, and custom policy requires low risk with clean signals. Human review is needed; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-checklist-status.yml:
- Line 10: Move checks: write from the workflow-level permissions block into the
checklist-validation job’s permissions, and place any pull-requests: write
permission required by that job there as well. Keep lint-and-format and other
jobs limited to read-only permissions by retaining only the necessary
workflow-level defaults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0334f49d-8230-4a93-b25c-e30959014870
📒 Files selected for processing (5)
.github/workflows/pr-checklist-status.yml.github/workflows/security.ymlpyproject.tomlscripts/validate_workflows.pytests/test_validate_workflows_unit.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/security.yml
- tests/test_validate_workflows_unit.py
- scripts/validate_workflows.py
… them Move `checks: write` and `pull-requests: write` from the workflow-level permissions block into the `checklist-validation` job, keeping the workflow default read-only. The `lint-and-format` job no longer receives unnecessary write access (zizmor excessive-permissions). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M11yDjPkpkkG7C9zU97yjf
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5c2989d1-095b-44c6-b925-4a3e3cdcb440) |
There was a problem hiding this comment.
Stale comment
Risk: high. Left a non-blocking comment (not approving): Cursor Bugbot and Security Agent checks completed as skipped with unresolved high/medium findings, and custom policy requires low risk with clean automated signals. Human review is needed; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-checklist-status.yml:
- Around line 71-77: Pin the task-completed-checker-action reference in the
affected workflow job to the reviewed 40-character commit SHA
2ddb65fdd5577bae4a8e82e0564e459677aec893 instead of the mutable v0.1.2 tag,
while preserving the existing permissions and job behavior.
In `@scripts/validate_workflows.py`:
- Around line 288-289: Update the job validation logic after the jobs type guard
to validate nested steps and uses values before consuming them: skip or report
jobs whose steps is not a collection suitable for iteration, and only call
startswith when uses is a string. Preserve normal validation for well-formed
jobs and prevent malformed values from raising runtime errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: afadf5dd-a4de-49b7-96ba-23c2eb3b3736
📒 Files selected for processing (5)
.github/workflows/pr-checklist-status.yml.github/workflows/security.ymlpyproject.tomlscripts/validate_workflows.pytests/test_validate_workflows_unit.py
🚧 Files skipped from review as they are similar to previous changes (3)
- pyproject.toml
- .github/workflows/security.yml
- tests/test_validate_workflows_unit.py
…ator - pr-checklist-status.yml: pin kentaro-m/task-completed-checker-action to the immutable v0.1.2 commit SHA (2ddb65f…) now that the job grants it checks:write / pull-requests:write, closing the mutable-tag supply-chain gap. - validate_workflows.py: guard `_validate_best_practices` against malformed job content — skip jobs whose `steps` is not a list and steps whose `uses` is not a string, instead of raising TypeError/AttributeError. - Add a regression test for the malformed steps/uses paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M11yDjPkpkkG7C9zU97yjf
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8b32ebbb-6e29-4477-9947-5e0d2e531cfc) |
There was a problem hiding this comment.
Stale comment
Risk: high. Left a non-blocking comment (not approving): Cursor Bugbot and Security Agent checks completed as skipped with unresolved high/medium findings still open, and custom policy requires low risk with clean automated signals. Human review is needed; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver
|
This pull request has been automatically marked as stale because it has not had To keep this PR open:
If you believe this was marked in error, please comment and we'll review it. |
|
This pull request has been automatically closed due to inactivity (45+ days with no updates). If you still want to merge these changes:
Thank you for your contribution! |
ECC Tools / Security EvidenceCommit: Security evidence gate passed (success) No security-sensitive scanner-evidence gap detected. Mode: enforce Scanned 1054 changed file(s). No missing scanner-evidence signal was detected. Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / PR Risk TaxonomyCommit: PR taxonomy review recommended (neutral) Detected 3 PR taxonomy bucket(s): Security Evidence, CI/CD Recommendation, Cost/Token Risk. Scanned 1054 changed file(s). Roadmap taxonomy buckets: Security EvidenceSecurity-sensitive changes should carry explicit scanner, code-scanning, or focused regression evidence. Signals:
Paths:
CI/CD RecommendationCI, dependency, coverage, and contract signals should be routed into follow-up checks or verification work. Signals:
Paths:
Cost/Token RiskAI routing, usage, and token-budget changes should include budget or usage-limit evidence. Signals:
Paths:
Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / Reference Set ReadinessCommit: Reference set readiness gaps detected (neutral) Reference evidence present for 2/7 areas (29%) across 1054 changed file(s). This check is based on files changed in this PR. Repository-level readiness is still reported by
Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / Hosted Promotion ReadinessCommit: Hosted promotion readiness passed (success) No hosted promotion evidence gaps detected across 1054 changed file(s); 0 corpus scenarios had matching evidence. This check compares PR file changes against the evaluator/RAG promotion corpus in No evaluator corpus scenarios matched this PR. Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / PR Config AuditCommit: No changed-config issues detected (success) Scanned 4 config file(s) present at this commit across 4 changed config path(s) and found no issues in the supported security rules. Changed config files:
Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
ECC Tools / PR Harness AuditCommit: No harness issues detected (success) Scanned 4 changed config file(s) and found no harness issues. Changed config files:
Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission. |
There was a problem hiding this comment.
Risk: high. Left a non-blocking comment (not approving): Cursor Security Agent completed with an unresolved medium finding, required checks failed (CodeQL), and custom policy requires low risk with clean signals. Human review is needed; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver


Description
Consolidates the 0.2.1 release-blocker work: security/CI fixes plus the follow-up
commits that close the open review threads from #356, and prepares release metadata.
Security
core/logging.redact_url: fully masks URL credentials as***:***(the DBusername was previously emitted in clear text). Verified for
user:pass@,user@, password-only, and no-credential URLs.scripts/validate_workflows.py: YAML loading wrapped in error handling(unreadable / invalid / non-mapping documents → structured errors instead of a
crash); malformed non-mapping
jobs:are handled defensively; broadenedhardcoded-secret detection (unquoted keys +
api_key, without false-positives onempty values, and no longer fail-open on a trailing
${{ }}expression);pull_request_targetis a blocking error; actions not pinned to a full 40-charcommit SHA are reported as warnings.
.github/workflows/security.yml: gitleaks image pinned tov8.28.0by immutabledigest (removes the mutable
:latesttag; stays compatible with the newer.gitleaks.tomlallowlist schema).persist-credentials: falseon checkouts..github/workflows/pr-checklist-status.yml: grantchecks: writeso thetask-completed-checker action can publish its check run instead of failing with
"Resource not accessible by integration".
Reliability
core/parser/engine:PyJsParserinstantiated per invocation (thread-safe);parse_codevalidates non-stringcode/filenameand returns a structured failedParseResultinstead of raising.scripts/cleanup_stale_prs.py: request timeouts on GitHub API calls.plugins/code_analyzer+ OpenAI/Anthropic plugins: degrade gracefully withoutSDKs/keys and log swallowed exceptions while keeping generic user-facing payloads.
Quality / Release
pyproject.toml: explicit PEP 517[build-system]table so the CIpython -m buildrelease gate is deterministic (setuptools backend).
black==25.11.0; Ruff config scoped so thestyle gates are deterministic. Version
0.2.0→0.2.1; CHANGELOG0.2.1entry.venv/and repaired the pre-commit config.Type of Change
Not applicable: new feature, breaking change, documentation-only update, dependency update.
Checklist
Related Issues
Follows up on the review findings from #356. No separate linked issue.
Additional Notes
Decisions taken on the dashboard "decide" items:
validator on all repo workflows and block 0.2.1. Full SHA migration + mandatory
enforcement is deferred to the next minor.
pull_request_target→ hard error: zero blast radius today (unused);forward-looking hardening.
Known items left out of scope (maintainer follow-up): the
submit-pypianthropicpin mismatch inrequirements.txt, the repo-wide flake8/ruff debt inuntouched modules, the CodeQL default-vs-advanced code-scanning configuration conflict
(the "JS/TS default setup not found" status), and the non-blocking docstring-coverage
threshold — all repo-settings / broad-refactor items outside these findings.
Note
Medium Risk
Touches credential logging, workflow permissions, and secret-scanning pipelines—important for security posture but largely hardening rather than new exposed surfaces. Parser and plugin changes alter concurrency and error handling in shared analysis paths; CI/build changes affect release gates without changing production runtime logic directly.
Overview
Prepares 0.2.1 with security fixes, expanded CI gates, and reliability work across core, plugins, and tooling.
Security & logging: New
redact_urlmasks database URL credentials as***:***in CLI and server startup logs (username was previously visible).validate_workflows.pyis rewritten with safer YAML handling, broader secret-pattern detection,pull_request_targetas a blocking error, and warnings for actions not pinned to a full 40-char SHA.security.ymldrops CodeQL and leans on Semgrep plus Gitleaks (digest-pinned image) with a small.gitleaks.tomlallowlist; checkouts usepersist-credentials: false. PR checklist workflow scopes elevatedchecks/pull-requestswrite to the checklist job and pins the checker action to a commit SHA.CI & tooling:
ci.ymlbecomes a multi-job pipeline (fatal Ruff,compileall, release-contract pytest,python -m build+twine check). Pre-commit aligns with CI (Black 25.11.0, Ruff 0.14.6), dropsruff-format, and tightens excludes..flake8ignores E501/E402 to match Black/Ruff.pyproject.tomladds an explicit[build-system]table and a documented Ruff ignore set for low-signal rules.Runtime behavior: The parser package is restored as plain source: per-call
PyJsParserfor thread safety, structured failures for badcode/filename, and sanitized error messages. OpenAI/Anthropic/code-analyzer plugins returnstatus: errordicts instead of raising when inputs, SDKs, or keys are missing. GitHub scripts gain request timeouts;anthropicminimum version bumps inrequirements.txt.Tests: Large additions for
redact_url, workflow validator units, parser edge cases, and plugin graceful-failure paths; version expectations move to 0.2.1.Reviewed by Cursor Bugbot for commit 93123cc. Bugbot is set up for automated code reviews on this repo. Configure here.