Skip to content

feat(github): fix security/quality findings and ship PRs with auto-detected coding agents - #4597

Merged
VaibhavUpreti merged 10 commits into
mainfrom
feat/security_issue_fix_skill
Aug 3, 2026
Merged

feat(github): fix security/quality findings and ship PRs with auto-detected coding agents#4597
VaibhavUpreti merged 10 commits into
mainfrom
feat/security_issue_fix_skill

Conversation

@VaibhavUpreti

Copy link
Copy Markdown
Member

Fixes #

Describe the changes you have made in this PR -

This PR adds the GitHub security/quality fix + PR capability to the interactive shell, and makes the fixer actually work on machines without the Pi CLI by turning the coding-agent seam into a multi-backend, auto-detected one.

Feature (fix + ship a PR from a GitHub finding):

  • New fix_github_security_alert action tool (integrations/github/tools/security_fix/): resolves a Dependabot / code-scanning / Code Quality finding (exact alert, URL, or highest-severity auto-selection), fixes it in the local checkout, and with open_pr=true commits only the changed files to a fresh opensre/github-security-fix-* branch, pushes, and opens a PR. Secret-scanning alerts are refused (rotation first).
  • Built-in deterministic local fixers (ruff-backed) for unused-import / unused-local Code Quality findings — no LLM needed for those.
  • Shared PR-creation helper integrations/github/pull_requests.py (also adopted by fix_sentry_issue, whose local pr.py is removed).
  • Action-agent skill (github_security_fix) + docs page (docs/github-security-fix.mdx), wired into docs.json.
  • Action driver now prefers a tool's response_text for user-facing output, so the reply is the tool's one-line result instead of model-invented "options" lists.

Coding-agent seam: auto-detected multi-backend (the "shitty response" fix):
Previously CODING_AGENT defaulted to pi and Pi was the only backend, so any finding without a built-in fixer dead-ended in "the built-in fixer couldn't produce a patch… configure a coding agent" — even on machines with Claude Code or Codex installed.

  • integrations/coding_agent/ now has three backends: Pi, Claude Code (claude -p --permission-mode acceptEdits --allowedTools …), and Codex (codex exec -s workspace-write).
  • CODING_AGENT defaults to auto: the runner picks the first installed-and-authenticated backend (pi → claude-code → codex). A concrete name (or alias claude) pins one.
  • Shared machinery extracted to leaf modules so backends stay thin and cycle-free:
    • integrations/llm_cli/agent_exec.py — injection-guarded task prompt, deadline-polled subprocess with pipe draining, outcome classification (provider limit markers).
    • integrations/git/worktree_capture.py — tolerant changed-files + diff capture (tracked edits + untracked files).
    • integrations/pi/client.py and the security-fix local fixer are refactored onto these (no behavior change; tests migrated in the same change).
  • When no agent is ready, the tool error is now one actionable line naming what to install, instead of a vague "optional configured coding-agent fallback".

Demo/Screenshot for feature changes and bug fixes -

Auto-detection on a machine without Pi (previously this configuration dead-ended):

$ uv run python -c "
from integrations.coding_agent import verify_coding_agent, coding_agent_provider
print('CODING_AGENT default:', coding_agent_provider({}))
print(verify_coding_agent())"
CODING_AGENT default: auto
(True, 'claude-code: Authenticated via Claude subscription (…)')

Test run:

===== 13076 passed, 14 skipped in 0:11:15 =====   (make test-cov)
make lint / make format-check / make typecheck    all green
make verify-integrations-smoke                    31 passed

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

  • Problem: asking OpenSRE to "fix the security issues and raise a PR" either did nothing useful (no Pi installed → dead-end message with option lists) or wasn't possible at all (no fix-and-ship tool).
  • Alternatives considered: (a) building an in-process LLM edit loop in core/llm — rejected as a large new surface duplicating what installed coding CLIs already do well; (b) keeping Pi-only and just improving the error — rejected because it still can't fix anything on most machines.
  • Chosen approach: keep the existing agent-neutral seam (run_coding_task / verify_coding_agent) and register additional backends behind it, exactly as its module docstring anticipated. Auto-detection makes it zero-config; the seam's callers are unchanged.
  • Key components: agent_exec.py (generic "hands"-role subprocess machinery — moved from the Pi client so all backends share the prompt-injection guard, deadline polling, and limit-marker classification), worktree_capture.py (what-changed capture), claude_code_backend.py / codex_backend.py (vendor argv/env + verify via the existing llm_cli adapters' detect()), runner.py (auto resolution order pi → claude-code → codex), and the security_fix tool package (alert context → fix → confirm → branch/commit/push/PR).

Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

🤖 Generated with Claude Code

…tected coding agents

Add the fix_github_security_alert action tool: resolve a Dependabot,
code-scanning, or Code Quality finding, fix it in the local checkout
(built-in ruff fixers first), and optionally commit/push a fresh
opensre/github-security-fix-* branch and open a PR.

Make the coding-agent seam multi-backend and zero-config: CODING_AGENT
now defaults to auto, which picks the first ready backend among Pi,
Claude Code (claude -p acceptEdits), and Codex (codex exec
workspace-write). Shared machinery moves to leaf modules
(integrations/llm_cli/agent_exec.py, integrations/git/worktree_capture.py)
and the Pi client is refactored onto them; tests migrated in the same
change. When no agent is ready the tool now returns one actionable line
instead of a vague coding-agent-fallback message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

Comment thread integrations/github/tools/security_fix/local_fix.py
Comment thread integrations/llm_cli/agent_exec.py Outdated
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds GitHub security and quality finding remediation with optional pull-request shipping.

  • Introduces built-in and coding-agent-backed fix execution for supported GitHub findings.
  • Adds automatic selection among Pi, Claude Code, and Codex coding-agent backends.
  • Extracts shared subprocess, worktree-capture, and pull-request creation helpers.
  • Updates interactive-shell response handling, skills, documentation, and tests.

Confidence Score: 4/5

The PR should not be merged until timeout cleanup reliably terminates coding-agent descendants on every supported platform.

The stale-line import deletion has been corrected, but the timeout path still falls back to terminating only the immediate CLI process when POSIX process-group APIs are unavailable, so spawned tests, linters, or other descendants can continue consuming resources or editing the checkout after a timeout is reported.

Files Needing Attention: integrations/llm_cli/agent_exec.py

Important Files Changed

Filename Overview
integrations/github/tools/security_fix/local_fix.py Adds Ruff-backed deterministic fixes and now avoids applying original finding coordinates after Ruff has rewritten the file.
integrations/llm_cli/agent_exec.py Centralizes guarded CLI execution and adds POSIX process-group timeout cleanup, while non-POSIX fallback cleanup remains limited to the direct child.
integrations/github/tools/security_fix/runner.py Coordinates alert resolution, baseline capture, remediation, and optional PR shipping.
integrations/github/tools/security_fix/ship.py Implements branch creation, scoped commits, pushing, and pull-request creation for completed fixes.
integrations/coding_agent/runner.py Adds automatic readiness-based selection among Pi, Claude Code, and Codex backends.
integrations/github/pull_requests.py Extracts reusable GitHub pull-request creation and stable error mapping from the Sentry-specific implementation.

Sequence Diagram

sequenceDiagram
    participant User
    participant Tool as GitHub security-fix tool
    participant GitHub
    participant Fixer as Built-in fixer / coding agent
    participant Git as Local Git checkout
    User->>Tool: Fix finding, optionally open PR
    Tool->>GitHub: Resolve alert details
    Tool->>Git: Validate repository and capture baseline
    Tool->>Fixer: Apply remediation
    Fixer->>Git: Edit and verify files
    Tool->>Git: Capture changed paths
    alt PR requested and approved
        Tool->>Git: Create branch and commit fix paths
        Tool->>GitHub: Push branch and open PR
        GitHub-->>Tool: PR URL
    end
    Tool-->>User: Result or actionable error
Loading

Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/security_i..." | Re-trigger Greptile

Drop stale fix_sentry_issue.pr ignore_imports (module removed) and allow
ship → pull_requests. Skip the F401 line-delete fallback after Ruff rewrites
the file, and terminate coding-agent process groups on timeout.
The findings API returns only an authenticated api.github.com url
(html_url is null). Synthesize github.com/security/quality/findings/{n}
so tasks, PR bodies, and tool responses never link to a 401 API endpoint.
## Summary

**Root cause:** `test_run_pi_coding_task_timeout` drives the timeout path in `poll_agent_process`, which calls `_signal_process_group` (integrations/llm_cli/agent_exec.py:121). That function reads `proc.pid`, but the test's `_FakePopen` stand-in never defined a `pid` attribute → `AttributeError`.

**Change (1 file):**
- `tests/integrations/test_pi.py` — added `self.pid: int | None = None` to `_FakePopen.__init__`. With `pid=None`, `_signal_process_group` skips the `os.killpg` branch and uses its documented `terminate()`/`kill()` fallback, which the fake implements. This also guarantees tests never signal a real process group.

No production code was touched — the `pid is not None` guard in `agent_exec.py` already handles this case correctly; the fake was just incomplete.

**Verification:**
- `uv run python -m pytest tests/integrations/test_pi.py -q` → 11 passed, 1 skipped (opt-in live test)
- `ruff check` + `ruff format --check` on the edited file → clean

Per instructions, I did not commit or push; the fix is in the working tree alongside your other uncommitted changes.

Generated by OpenSRE from #4597.
@VaibhavUpreti

Copy link
Copy Markdown
Member Author

@greptile review

Add fix_github_pr_ci to inspect failing Actions checks, run an
auto-detected coding agent, and push fixes to the existing PR branch.
Skip cancelled sibling checks, sanitize markdown headings out of commit
subjects, and make process-group cleanup tolerate test doubles without pid.
Resolve action_driver conflict by keeping main's turn helpers while
preserving preferred tool response_text over chatty model closings.
Drop stale FLEET_CDK exports and update the harness test to ActionTurnRunner.
Ensure local commits and CI formula/readme bump commits include the
OpenSRE Agent trailer so agent-authored changes stay attributable.
The git helpers import these trailers; keep the constants module in the
repo so the branch stays importable after the co-author stamp landed.
Compress github_cli skill guidance under the registry char budget,
classify fix_github_pr_ci for Sentry telemetry coverage, and refresh
the action-system prompt characterization snapshot.
@VaibhavUpreti
VaibhavUpreti merged commit 23d5dea into main Aug 3, 2026
28 checks passed
@VaibhavUpreti
VaibhavUpreti deleted the feat/security_issue_fix_skill branch August 3, 2026 19:56
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🤖 CI passed. Linter didn't scream. Reviewer typed LGTM. @VaibhavUpreti, every machine in this pipeline just slow-clapped. 🖥️✨


👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant