Skip to content

fix(agent-actions): scope live CI close recheck - #2478

Merged
JSONbored merged 2 commits into
mainfrom
codex/fix-vulnerability-in-heuristic-close-logic
Jul 2, 2026
Merged

fix(agent-actions): scope live CI close recheck#2478
JSONbored merged 2 commits into
mainfrom
codex/fix-vulnerability-in-heuristic-close-logic

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • A recent executor guard re-fetched live CI for any close action tagged closeKind: "heuristic", which incorrectly required live CI to still be failed for all heuristic closes and thus blocked non-CI-driven closes (gate failures, slop/duplicate findings, conflicts) when CI had become passing.
  • The intent is to re-check CI only for merges and for closes that were specifically driven by red CI, not for every heuristic close the planner emits.

Description

  • Add a narrow discriminator closeRequiresCiState?: "failed" to the planned action payload (PlannedAgentAction) and the persisted approval params (AgentPendingActionParams) so CI-driven closes can carry an explicit CI dependency.
  • Populate closeRequiresCiState: "failed" in the planner only when the computed close reason is actually a CI failure (ciFailed).
  • Scope the executor's actuation-time live-CI recheck to merge actions and only to close actions that carry closeRequiresCiState === "failed", while keeping the broader closeKind: "heuristic" value for the close-precision breaker.
  • Persist and round-trip the new closeRequiresCiState through actionParams() and pendingActionToPlanned() so queued/accepted closes retain the CI dependency; update comments and type docs accordingly.
  • Update and extend unit tests to cover CI-driven close denial/proceed paths, queued replay round-trip, non-CI heuristic closes, and the planner's behavior for CI vs non-CI closes.

Testing

  • Ran the updated unit tests via vitest for the changed suites: test/unit/agent-action-executor.test.ts, test/unit/agent-actions.test.ts, and test/unit/agent-approval-queue.test.ts, and they all passed locally.
  • Type checking with tsc --noEmit completed successfully.
  • Ran git diff --check with no issues reported.
  • npm run test:ci could not complete in this environment due to network/setup limits affecting actionlint and the repository's self-hosted runner label, and npm audit --audit-level=moderate could not complete because the audit endpoint returned 403 Forbidden.

Codex Task

@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-02 07:37:35 UTC

6 files · 1 AI reviewer · no blockers · readiness 95/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change cleanly separates broad heuristic-close classification from the narrower red-CI dependency, and the executor now preserves the old fail-closed behavior only for legacy staged heuristic closes where the new discriminator is absent. The planner writes an explicit `failed` versus `not_required` value for fresh heuristic closes, and `actionParams()` persists the discriminator so queued CI-driven closes still re-check live CI before mutation. The visible tests cover the important live, replayed, non-CI, and legacy paths, so the diff is safe enough to proceed.

Nits — 5 non-blocking
  • nit: `src/settings/agent-actions.ts:67` documents `closeRequiresCiState` as always set for heuristic closes, but the optional field leaves that convention unenforced outside `planAgentMaintenanceActions`; consider a discriminated helper/type if more call sites construct `PlannedAgentAction` directly.
  • nit: `test/unit/agent-approval-queue.test.ts:889` only asserts `actionParams()` persistence for `closeRequiresCiState: "failed"`; adding the `"not_required"` case would pin the non-CI queued replay contract too.
  • In `test/unit/agent-approval-queue.test.ts:889`, add a companion assertion for `{ closeKind: "heuristic", closeRequiresCiState: "not_required" }` so the persisted non-CI discriminator cannot regress.
  • In `src/settings/agent-actions.ts:67`, either enforce heuristic close construction through the planner/helper or tighten the type shape so fresh heuristic closes cannot accidentally omit `closeRequiresCiState` while legacy replay remains explicitly represented.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:M; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 548 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 548 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 65 PR(s), 548 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui ab246a0 Commit Preview URL

Branch Preview URL
Jul 02 2026, 05:43 AM

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.96%. Comparing base (4101ca6) to head (82b7bbf).
⚠️ Report is 39 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2478      +/-   ##
==========================================
+ Coverage   95.94%   95.96%   +0.01%     
==========================================
  Files         226      226              
  Lines       25361    25473     +112     
  Branches     9229     9266      +37     
==========================================
+ Hits        24333    24445     +112     
  Misses        417      417              
  Partials      611      611              
Files with missing lines Coverage Δ
src/services/agent-action-executor.ts 93.44% <100.00%> (+0.64%) ⬆️
src/settings/agent-actions.ts 93.04% <100.00%> (+0.25%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added 2 commits July 2, 2026 00:30
…oses

The narrowed live-CI recheck only fired for closeRequiresCiState ===
"failed", but a heuristic close staged before this field existed has no
closeRequiresCiState at all -- indistinguishable in storage from a fresh,
genuinely non-CI-driven close. Accepting such a legacy row skipped the
recheck entirely, so a stale CI-driven close could still execute after
CI recovered (flagged by the gate's own review).

Make the planner always tag a heuristic close explicitly ("failed" or
"not_required", never omitted) so an absent value can only mean a
legacy row. The executor now re-checks CI for that ambiguous case too,
falling back to the old pre-fix requirement (CI must still be failed).
@JSONbored
JSONbored force-pushed the codex/fix-vulnerability-in-heuristic-close-logic branch from 5d8f2fb to 82b7bbf Compare July 2, 2026 07:33
@JSONbored
JSONbored merged commit a08b959 into main Jul 2, 2026
9 checks passed
@JSONbored
JSONbored deleted the codex/fix-vulnerability-in-heuristic-close-logic branch July 2, 2026 07:43
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 2, 2026
JSONbored added a commit that referenced this pull request Jul 2, 2026
…cap enforcement

Gate findings on this PR:
- .gittensory.yml.example still documented contributorOpenIssueCap as
  inert, but the issue-cap enforcement from #2493 already landed on
  this branch — the doc was stale. Both caps are now documented as
  enforced.
- maybeCloseIssueOverContributorCap counted siblings from
  listOpenIssues (a DB read), with no live verification. A stale
  "open" row for a sibling already closed on GitHub could inflate the
  count and wrongly close a newly opened issue within the real cap.
  Live-verify each counted sibling before trusting it, mirroring
  reconcileLiveDuplicateSiblings' fail-open contract (only drop a
  sibling on a positive "not open" confirmation).

Also resolves a base conflict from a rebase onto main (closeKind's
"contributor_cap" variant combined with the closeRequiresCiState
tri-state added by #2478, both touching the same close-action types).
JSONbored added a commit that referenced this pull request Jul 2, 2026
…ap (#2270) (#2479)

* feat(settings): add contributorOpenPrCap/contributorOpenIssueCap config (#2270)

Adds the config-as-code contract for per-contributor open PR/issue caps: DB
migration, RepositorySettings type, .gittensory.yml settings: parsing with
precedence over the DB, OpenAPI response fields, and docs. No enforcement
behavior yet (a repo with no cap configured sees zero change) — auto-close
over the cap lands in a follow-up PR.

* feat(agent-actions): auto-close a contributor's PR over the open-PR cap (#2270)

Adds the enforcement half of #2270: a new closeKind: "contributor_cap"
short-circuit in planAgentMaintenanceActions, mirroring the existing
contributor-blacklist block exactly (fires ahead of merit/CI/AI analysis,
exempt from the close-precision breaker, owner/admin/bot never touched).

The webhook-time check counts the author's other open PRs on the repo
(already-fetched, live-DB-backed data — no new query) plus the incoming
one, ranked by PR number so a burst of near-simultaneous opens still
ranks deterministically; only the PR(s) pushing the count over the cap
close, oldest ones stay open. Adds the paired contributorCapLabel setting
(default "over-contributor-limit") through the full config-as-code chain.

Known scope limit: this covers live webhook-time enforcement only. A
maintainer lowering the cap after a contributor already has more PRs open
than the new cap allows is not retroactively enforced by a sweep in this
PR — tracked as follow-up work on #2270, not required for the core ask
(closing a NEW PR that pushes someone over the limit).

* fix(docs): correct .gittensory.yml.example — the PR cap is now enforced (#2479)

Gate finding on this PR: the example file still said contributorOpenPrCap
was inert with enforcement "landing in a follow-up PR" — but this very PR
IS that follow-up, so the doc was stale the moment it merged. Splits the
two fields' status: contributorOpenPrCap is enforced; contributorOpenIssueCap
remains inert until its own follow-up. Also adds a webhook-level (not just
planner-level) owner-exemption test, per the gate's non-blocking suggestion.

* fix(queue): wake missed siblings on out-of-order contributor-cap delivery (#2270)

Webhook delivery order is not guaranteed to match PR creation order, so a
sibling PR's webhook can process before a later-created PR exists in the DB
and wrongly conclude the author is within the open-PR cap. Nothing else
would ever re-evaluate that sibling, permanently bypassing the cap. Now that
a later delivery has the complete picture, wake any other still-open
over-cap sibling so its own next pass self-corrects.

* feat(agent-actions): auto-close a contributor's issue over the open-issue cap (#2270) (#2493)

* feat(settings): add contributorOpenPrCap/contributorOpenIssueCap config (#2270)

Adds the config-as-code contract for per-contributor open PR/issue caps: DB
migration, RepositorySettings type, .gittensory.yml settings: parsing with
precedence over the DB, OpenAPI response fields, and docs. No enforcement
behavior yet (a repo with no cap configured sees zero change) — auto-close
over the cap lands in a follow-up PR.

* fix(settings): preserve an explicit yml null clearing a contributor cap (#2467)

parseSettingsOverride collapsed an explicit `settings.contributorOpenPrCap:
null` in .gittensory.yml to the same outcome as omitting the key, so a
maintainer had no way to force a DB-configured cap back to disabled via
config-as-code, contradicting the documented yml > DB > null precedence.
Distinguish the three states (omitted / explicit null / invalid) before
normalizing. Also corrects .gittensory.yml.example, which described these
fields as already closing over-cap PRs; enforcement lands in a follow-up PR.

* feat(agent-actions): auto-close a contributor's issue over the open-issue cap (#2270)

Adds the first eventName === "issues" actuation branch in
processGitHubWebhook — issues previously had zero auto-close path.
maybeCloseIssueOverContributorCap mirrors the PR-path cap exactly (same
oldest-stays/newest-closes ranking by item number, same owner/admin/bot
exemption) and reuses planAgentMaintenanceActions's contributor_cap
short-circuit to build the label+close plan, so the label/message/closeKind
construction is identical between the PR and issue paths.

Fixes a bug introduced by that reuse: the close-comment message was
hardcoded to say "pull requests" even when closing an issue. contributorCapMatch
now carries an itemKind ("pull requests" | "issues") so each caller states
its own noun explicitly.

New executeIssueMaintenanceActions is deliberately narrower than the PR
executor: no freshness/live-CI/pull_requests:write gates (none of those PR
concepts apply to a plain issue), and auto_with_approval is denied rather
than staged — the pending-action queue is PR-shaped (pullNumber-typed
staging + a /pull/{n} deeplink); extending it to issues is a documented
follow-up, not silently bypassed here. closeIssue is a new GitHub REST
primitive (PATCH /issues/{n}), since closePullRequest hits the Pulls API,
which a plain issue number isn't valid against.

* fix(agent-actions): close every over-cap sibling issue, not just the incoming one (#2493)

Same delivery-order gap as #2479, mirrored for issues: closing only "the
incoming issue, if it's over cap" let an older sibling's stale verdict
(computed before a newer sibling existed in the DB) stand forever, since
nothing else ever re-evaluates an issue. Now closes every number in the
over-cap set discovered by the current delivery. Unlike the PR path,
issues have no live-head/CI staleness risk to guard against and no
issue-side "regate" job type to reuse, so acting directly on the
already-fetched snapshot is safe.

Also fixes the codecov/patch gap the gate flagged on this PR: adds
coverage for the label ?? "" fallback, the no-slash repoFullName guard,
and an author-less (ghost) sibling issue.

* test(queue): cover the sibling-wake enqueue-failure path (#2270)

env.JOBS.send can reject (queue backpressure/outage) when waking a missed
over-cap sibling; pin the fail-safe behavior — swallow the error and skip
claiming the coalescing key so a later discovery can retry the enqueue.

* fix(queue): resolve base conflict and 2 gate findings on contributor-cap enforcement

Gate findings on this PR:
- .gittensory.yml.example still documented contributorOpenIssueCap as
  inert, but the issue-cap enforcement from #2493 already landed on
  this branch — the doc was stale. Both caps are now documented as
  enforced.
- maybeCloseIssueOverContributorCap counted siblings from
  listOpenIssues (a DB read), with no live verification. A stale
  "open" row for a sibling already closed on GitHub could inflate the
  count and wrongly close a newly opened issue within the real cap.
  Live-verify each counted sibling before trusting it, mirroring
  reconcileLiveDuplicateSiblings' fail-open contract (only drop a
  sibling on a positive "not open" confirmation).

Also resolves a base conflict from a rebase onto main (closeKind's
"contributor_cap" variant combined with the closeRequiresCiState
tri-state added by #2478, both touching the same close-action types).

* fix(queue): fail SAFE, not fail-open, on an unverifiable issue-cap sibling

The live-verify guard added for the first gate finding on this PR still
counted an unreadable sibling toward the cap by default (mirroring
reconcileLiveDuplicateSiblings' fail-open-to-stored contract). The gate
flagged that this compounds with a stale "open" DB row: a transient
fetch failure could still let a stale-closed sibling inflate the count
and wrongly close a newly opened issue within the real cap.

Unlike reconcileLiveDuplicateSiblings (which only re-ranks a non-final
duplicate-cluster winner), this count gates an irreversible close, so
the safe default flips: only a POSITIVE "open" confirmation counts a
sibling toward the cap. An unreadable check now excludes it instead of
including it.
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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant