[FEATURE] GraphQL Escape Sequence Lint Gap and Untested Mutation Syntax#4073
Merged
Trecek merged 10 commits intoJun 11, 2026
Merged
Conversation
Replace `\$` with `$` at two sites in _cmd_rpc_issues.py. Python silently drops the backslash at runtime (producing valid GraphQL accidentally), but the source emits SyntaxWarning on 3.13+ and will become SyntaxError in a future release. `$` needs no escaping in Python f-strings. Refs #4062
Three complementary defenses against GraphQL construction defects in batch_create_issues and refetch_issues: 1. Ruff W category (W605 invalid-escape-sequence) — source-text gate that fires in pre-commit and CI for any new invalid escape sequence. 2. Pytest filterwarnings: error::SyntaxWarning — compile-time gate that fails the test suite at import time if any module has invalid escapes, even those not covered by specific GraphQL tests. 3. _validate_mutation_variables runtime helper — asserts that every key in the variables dict is both declared and referenced in the mutation string. Guards against variable-name drift and the ruff W605 raw-string auto-fix trap (if ruff converts $ to rf"\$", the runtime string would contain literal backslash-dollar and this check would fail). Also adds source-text lint test and strengthens existing GraphQL test assertions with structural form checks (starts-with-mutation, variable declaration/reference assertions). Refs #4062
…g headless guards The error::DeprecationWarning:autoskillit filter promotes the operational DeprecationWarning from session_type() into an exception when HEADLESS=1 is set without SESSION_TYPE. The guards only caught ValueError, causing headless_error responses to become unhandled tool_exception responses. - Add filterwarnings exclusion for the session_type() DeprecationWarning - Broaden guard except clauses to catch (ValueError, DeprecationWarning) as defense-in-depth for fail-closed behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s GraphQL query
This adds a structural guard beyond the existing weak substring checks
('org' in query_arg). The new assertion validates that the GraphQL query
produced by refetch_issues contains the 'repository(owner:' field syntax,
providing coverage that would fail if the query structure broke while
the weaker 'org' substring still passed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…uards session_type() uses warnings.warn() — DeprecationWarning is never raised as an exception in production. The filterwarnings suppression in pyproject.toml (added in 70582b9) handles the test-suite promotion path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use Path(__file__)-relative resolution instead of hardcoded relative path to prevent silent pass under xdist CWD variance. Assert ruff exits 0 or 1 to catch tool failures that would otherwise produce empty findings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tput
Remove the dead `or query.startswith("{")` branch from the assertion in
test_batch_create_issues_constructs_graphql_mutation. The production code
always emits `mutation(...)` form — the `{` branch was unreachable and
weakened the structural guard.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…utput
Remove the dead `or query.startswith("{")` branch from the assertion in
test_batch_create_issues_chunks_large_batches. Same fix as the prior
commit — production always emits `mutation(...)` form.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If ruff exits 1 (violations found) but stdout is empty, the prior logic set findings=[] and the test passed falsely. Add a post-check assertion that returncode must be 0 when no findings were parsed, catching the empty-stdout-on-exit-1 edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test called bare "ruff" which isn't on PATH in CI's subprocess environment. Resolve the ruff binary from sys.executable's parent directory (the venv bin/) and skip if unavailable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Trecek
deleted the
batched-createissue-graphql-path-broken-escape-emits-literal/4062-2
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two invalid Python escape sequences (
\$) in_cmd_rpc_issues.pyemitSyntaxWarningon Python 3.13 and will becomeSyntaxErrorin a future Python release. The fix corrects the escape sequences, adds ruffW605(invalid-escape-sequence) and pytestfilterwarningsgates, introduces runtime structural validation for GraphQL mutation variable declarations, and strengthens existing test assertions with forward-looking structural checks.This is a four-step immunity approach: (1) fix the
\$escapes at lines 293 and 303, (2) add lint and pytest warning gates to pyproject.toml and guard infrastructure, (3) add runtime structural validation for mutation variable declarations, and (4) strengthen existing test assertions to validate variable declaration syntax and absence of backslash artifacts.Requirements
autoskillit ordersessions print compile-time warnings to the user's terminal on first import:\$is not a valid Python escape sequence. The backslash is silently dropped on current Python so the GraphQL is accidentally valid at runtime, but the code is formally incorrect and on a deprecation path to hard failure.Scope of fix:
\$with$at both sites (lines 293 and 303).W605to ruffselectin pyproject.toml to catch invalid escape sequences.filterwarnings = ["error"]for headless test gates.test_batch_create_issues_constructs_graphql_mutationwith structural assertions for variable declaration/reference syntax.Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260611-105557-432256/.autoskillit/temp/rectify/rectify_graphql_escape_lint_gap_2026-06-11_110500.mdCloses #4062
🤖 Generated with Claude Code via AutoSkillit