Skip to content

fix(gate): route draft-dodge close through agent executor - #1223

Closed
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-draft-dodge-close-vulnerability
Closed

fix(gate): route draft-dodge close through agent executor#1223
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-draft-dodge-close-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The converted-to-draft "draft-dodge" handler previously posted a comment and directly closed PRs, which bypassed the agent action executor's safety stack (global pause, dry-run, per-action autonomy, approval staging, and permission readiness), creating an authorization/behavior mismatch.
  • The change ensures draft-conversion close attempts obey the same safety and audit semantics as other automated maintainer actions.

Description

  • Replace the direct createIssueComment + closePullRequest path with a planned close action routed through executeAgentMaintenanceActions so close-specific autonomy, approval staging, dry-run, global/per-repo pause, and permission checks run before any GitHub mutation (src/queue/processors.ts).
  • Use getInstallation to supply installationPermissions to the executor and build a close PlannedAgentAction including requiresApproval, closeKind, and closeComment so the existing approval/flagging logic applies.
  • Only record the github_app.draft_dodge_closed audit event when the executor reports a completed close, avoiding misleading audit records when the executor denies, queues, dry-runs, or errors the action.
  • Add and update unit tests to cover executor error recording, label-only autonomy (close denied when only label is enabled), dry-run/global pause behavior, and staging when close requires approval (test/unit/queue.test.ts).

Testing

  • Ran the focused unit suite for the converted-to-draft handler: npx vitest run test/unit/queue.test.ts -t "converted_to_draft gate-close", which passed (tests for the block/close branches and new regression cases succeeded).
  • Type-check verified with npm run typecheck and succeeded.
  • Lint/checks verified locally with git diff --check (no whitespace/conflict marker issues).

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.82%. Comparing base (2ea7ed1) to head (1f7d813).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 77.77% 1 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (77.77%) is below the target coverage (97.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1223   +/-   ##
=======================================
  Coverage   94.82%   94.82%           
=======================================
  Files         158      158           
  Lines       19157    19157           
  Branches     6939     6940    +1     
=======================================
  Hits        18165    18165           
+ Misses        396      395    -1     
- Partials      596      597    +1     
Files with missing lines Coverage Δ
src/queue/processors.ts 86.51% <77.77%> (-0.18%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jun 24, 2026
@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 24, 2026
@JSONbored

Copy link
Copy Markdown
Owner Author

Superseded by #1247, which closes the same gap (the draft-dodge close now respects per-repo pause, the global freeze, and agent_dry_run) and also covers the maybeRecloseDisallowedReopen close path, which this PR did not. Your executor-routing approach is cleaner architecturally — routing both direct-close paths through executeAgentMaintenanceActions so they inherit close-autonomy + approval enforcement too is a worthwhile follow-up refactor; filing that separately. Thanks for catching the gap.

@JSONbored JSONbored closed this Jun 24, 2026
@JSONbored
JSONbored deleted the codex/fix-draft-dodge-close-vulnerability branch June 24, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant