Skip to content

fix(agent-actions): distinguish an executor error from a clean accept - #2428

Merged
JSONbored merged 1 commit into
mainfrom
fix-2423-pending-action-status
Jul 1, 2026
Merged

fix(agent-actions): distinguish an executor error from a clean accept#2428
JSONbored merged 1 commit into
mainfrom
fix-2423-pending-action-status

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • decidePendingAgentAction's accept path unconditionally set status: "accepted" on the persisted agentPendingActions row regardless of the executor's actual outcome (completed / queued / denied / error / dry_run), so a thrown exception during the live mutation was invisible next to a clean success without cross-referencing audit metadata.
  • Adds an "errored" status, scoped specifically to a genuine executor exception (execOutcome === "error"). A clean "denied" outcome (autonomy no longer authorizes, dry-run active, or a live pre-condition failed cleanly) still persists "accepted" — the maintainer's accept WAS honored, the executor just correctly chose not to act, which is not a failure and must not be conflated with one (matches existing, intentional test coverage for that case).
  • Threads the new status through AgentPendingActionStatus (types.ts), ApprovalDecisionResult.status, and the MCP gittensory_list_pending_actions filter enum + gittensory_decide_pending_action summary text.

Closes #2423

Test plan

  • npm run typecheck clean
  • Full unsharded npm run test:coverage: 312 files / 5823 tests passing, no threshold failures
  • npm audit --audit-level=moderate: 0 vulnerabilities
  • New regression tests (unit + MCP-surface) assert a thrown executor mutation persists "errored", not "accepted"
  • Existing "denied stays accepted" test coverage unchanged and still green

decidePendingAgentAction unconditionally persisted status: "accepted" on
the approval-queue row regardless of what the executor actually did, so a
thrown exception during the live mutation was indistinguishable from a
clean success or an intentional policy denial. Add an "errored" status,
scoped to a genuine executor exception -- a clean "denied" outcome (the
gates ran and correctly declined to act) still reads as accepted, matching
existing behavior.

Closes #2423
@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 22:46:14 UTC

5 files · 1 AI reviewer · no blockers · readiness 75/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change correctly separates a maintainer accept whose executor threw from a clean accept path by persisting and returning `errored` only when the executor outcome is `error`. The core service mapping in `src/services/agent-approval-queue.ts` is narrow enough to preserve existing denied/dry-run semantics, and the new unit plus MCP regression tests exercise the real accept-time mutation path. The main remaining gap is MCP list-surface coverage for the newly accepted `errored` status filter/output path.

Nits — 7 non-blocking
  • nit: `src/mcp/server.ts:404` adds `errored` to the list filter enum, but the tests only cover `gittensory_decide_pending_action`; add a focused `gittensory_list_pending_actions` assertion that an errored row can be filtered and returned.
  • nit: `src/services/agent-approval-queue.ts:159` and `src/types.ts:696` duplicate a long explanation of the same status semantics; keep the detailed rationale in one place and make the other comment shorter to reduce future drift.
  • nit: `test/unit/mcp-automation-state.test.ts:13` installs broad `pr-actions` and freshness mocks for the whole file even though the new error regression is the only test that needs the mutation throw path; consider scoping/resetting the mocked rejection more explicitly so later tests cannot inherit surprising behavior.
  • Add an MCP list regression near `test/unit/mcp-automation-state.test.ts:413` that creates an errored pending action, calls `gittensory_list_pending_actions` with `status: "errored"`, and asserts the returned entry carries `status: "errored"`.
  • Consider narrowing `ApprovalDecisionResult.executionOutcome` in `src/services/agent-approval-queue.ts:13` to the executor outcome union instead of `string`, so future status mappings are exhaustively checked.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • 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 ✅ Linked #2423
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:S; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 572 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 572 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Check active issues and PRs before submitting.
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

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.88%. Comparing base (cbf1909) to head (0cfcad3).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2428      +/-   ##
==========================================
+ Coverage   95.85%   95.88%   +0.02%     
==========================================
  Files         224      224              
  Lines       25104    25106       +2     
  Branches     9129     9131       +2     
==========================================
+ Hits        24064    24072       +8     
+ Misses        427      421       -6     
  Partials      613      613              
Files with missing lines Coverage Δ
src/mcp/server.ts 95.77% <100.00%> (+<0.01%) ⬆️
src/services/agent-approval-queue.ts 100.00% <100.00%> (ø)

... and 2 files 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.

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 1, 2026
@JSONbored
JSONbored merged commit 76e9baa into main Jul 1, 2026
12 checks passed
@JSONbored
JSONbored deleted the fix-2423-pending-action-status branch July 1, 2026 22:49
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
@github-actions github-actions Bot mentioned this pull request Jul 2, 2026
12 tasks
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.

chore(agent-actions): approval-queue accept always persists status=accepted even when the executor errors

1 participant