feat(skills): add /nextwork readiness queue - #5641
Conversation
Introduce a readiness-oriented queue of assigned open issues/PRs with blocker BFS, status classification, and --apply/--take-over/--link-blocker actions. Fix GraphQL Int variables via -F, raise list --limit to 1000, and restrict link-blocker ID lookup to issues. Detect in-flight agent-status comments so stale ready-for-merge is not treated as mergeable. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Stop trusting ready-for-merge alone — fetch mergeable and reviewThreads, treat CONFLICTING/DIRTY as fix_conflicts, and require CLEAN/UNSTABLE with zero unresolved conversations before ready_to_merge. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Enqueue open children via BFS and surface close_or_plan when all sub-issues are already closed, so epics are not mis-routed as promote_code. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Drive re-triggers from launch signals and stuck agent starts via --stale-hours, treat failed CI and human threads as decisions, and only auto-fix when all unresolved threads are from the review bot. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Only open structured blockedBy links classify as blocked_by; an orphaned blocked label is ignored for status and removed as a trivial --apply action. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
A terminal agent status at/after /fs-* no longer leaves items stuck in waiting_*. Actionable unassigned items get assign:self first so --apply can claim them before slash commands. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Prefer dependency edges over unrelated seeds and stop charging dropped fetches against the cap so actionable roots are not truncated. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Do not treat fullsend:*-agent result comments as post-triage discussion that forces a re-triage. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Older triage runs only left <!-- fullsend:triage-agent --> without a terminal agent-status, so completed-triage staleness never fired and issues like fullsend-ai#1160 stayed on stale ready-to-code instead of needs_triage. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
PR Summary by Qodofeat(skills): add /nextwork readiness queue
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
|
🤖 Review · ❌ Terminated · Started 12:09 PM UTC · Ended 12:25 PM UTC |
Code Review by Qodo
1.
|
ReviewFindingsHigh
Low
Next steps:
Previous runReviewFindingsHigh
Low
Next steps:
Previous run (2)ReviewFindingsHigh
Low
Next steps:
Previous run (3)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (4)ReviewFindingsHigh
Low
Next steps:
Previous run (5)ReviewFindingsHigh
Low
Next steps:
Previous run (6)ReviewFindingsHigh
Low
Next steps:
Previous run (7)ReviewFindingsHigh
Low
Next steps:
Previous run (8)ReviewFindingsHigh
Low
Next steps:
Previous run (9)ReviewFindingsHigh
Low
Next steps:
Previous run (10)ReviewFindingsHigh
Low
Next steps:
Previous run (11)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (12)ReviewFindingsHigh
Medium
Low
Previous run (13)ReviewFindingsHigh
Medium
Low
Previous run (14)ReviewFindingsHigh
Medium
Low
Labels: PR adds a new skill under skills/nextwork/ and introduces a new user-facing feature |
|
🤖 Finished Review · ✅ Success · Started 12:09 PM UTC · Completed 12:25 PM UTC |
Signed-off-by: Barak Korren <bkorren@redhat.com>
Correct resolve_repo/seed exit behavior, require open dependents for --link-blocker, defer and cap linked-PR scans, keep full comment bodies for classification, and document vs /topissues priority semantics. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Babysit updateMerged CI: prior Review threads: replied and resolved all 9 inline threads (qodo + fullsend-ai-review). Shared-helper extraction across Protected-path ( |
|
🤖 Review · |
Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review · ❌ Terminated · Started 8:37 PM UTC · Ended 8:54 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 Finished Review · ✅ Success · Started 8:37 PM UTC · Completed 8:54 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Automated review pass: 8 findings (2 high, 6 medium) on skills/nextwork/scripts/nextwork.py, posted inline. These are additional to prior review rounds and were checked against existing PR comments to avoid duplicates.
waynesun09
left a comment
There was a problem hiding this comment.
Review-only sweep: 3 findings (all MEDIUM), verified against the current head and checked against the existing review comments/fix threads on this PR — none of these three are covered by prior fixes.
waynesun09
left a comment
There was a problem hiding this comment.
Approving — no blocking issues. The confirmation-gate bypass for --take-over/--link-blocker (flagged last round) is confirmed fixed across the last 3 commits. Remaining 3 findings are non-blocking MEDIUM fast-follows: prose-only (not code-enforced) mutation gating as defense-in-depth, markdown output missing the truncation notice that JSON output already has, and a misleading suggested-action string for fullsend-no-fix. CI green, all other threads resolved.
Signed-off-by: Barak Korren <bkorren@redhat.com>
Require --confirmed with mutating flags, surface queue truncation in markdown, and stop suggesting removal of fullsend-no-fix before /fs-fix. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 11:55 AM UTC · Completed 12:09 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Automated review sweep: 2 findings posted inline (1 high, 1 medium).
Signed-off-by: Barak Korren <bkorren@redhat.com>
Treat unresolved review threads with any human reply as needing a decision, and fetch issue blockedBy/sub-issues separately so schema gaps degrade one axis. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 5:24 PM UTC · Completed 5:40 PM UTC |
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
skills/nextwork/scripts/nextwork.py:341: [low] edge-case
agent_terminal_succeeded returns True as its default fallback when no explicit success or failure markers are found in the body text. A terminal agent-status comment with a malformed or unanticipated body format would be silently classified as success, potentially clearing a launch wait prematurely. The function docstring documents this as intentional for legacy sticky triage posts, and the function is only called after AGENT_TERMINAL_MARKER in body is confirmed, bounding the surface. The test test_ambiguous_body_defaults_to_success exercises this path.
skills/nextwork/scripts/nextwork.py:1171: [low] naming-inconsistency
The new run_gh_soft function name diverges from the established try_run_gh name used for the same pattern in topissues.py (a gh wrapper that returns None on failure). nextwork.py defines both names - try_run_gh as a thin delegate to run_gh_soft - introducing a second name for the same concept. Additionally, try_run_gh is defined but never called within nextwork.py - all call sites use run_gh_soft directly.
Suggested fix: Consider renaming run_gh_soft to try_run_gh (adding the quiet parameter there) and removing the wrapper.
skills/nextwork/scripts/nextwork.py:28: [low] code-duplication
Substantial code is explicitly copied from skills/topissues/scripts/topissues.py (comment at line 28). Duplicated helpers include regex patterns, link-parsing functions, and gh CLI wrappers. The PR also updates the topissues.py regex to stay in sync, demonstrating the maintenance cost. Both files now carry cross-reference comments, which partially mitigates the risk.
Suggested fix: Consider extracting shared helpers into a common module, or accept the duplication as a trade-off for stdlib-only scripts.
skills/topissues/scripts/topissues.py(file-level): Line 296 · [low] type-hint-inconsistency
_gh_not_found() is declared with return type None in topissues.py but NoReturn in nextwork.py. Both functions always call sys.exit(1) and never return, so NoReturn is the more accurate type hint.
Suggested fix: Update topissues.py to use -> NoReturn for _gh_not_found().
|
Triaging the latest [high] protected-path ( [medium] shared-code-duplication — Ignored for this PR (accepted with cross-reference comments). Shared module is follow-up. [medium] architectural-coherence ( [low] comment truncation before classification — Incorrect on current head: [low] assignment vs waiting catalog order — Already documented under Classification priority in the skill. [low] APPROVED + leftover No new code changes for this round — PR stays merge-ready. |
Findings triaged in #5641 (comment) — already fixed, docs-covered, or deferred; no actionable code changes remain.
waynesun09
left a comment
There was a problem hiding this comment.
Review-only sweep: 3 findings (all MEDIUM), verified against current head (e6247ca) and checked against the ~170 existing review comments/fix threads on this PR — none of these three are covered by prior fixes.
| return assoc in TRUSTED_FS_ASSOCIATIONS | ||
|
|
||
|
|
||
| def agent_terminal_succeeded(body: str) -> bool: |
There was a problem hiding this comment.
[MEDIUM] Skipped terminal runs behave like failures, risking a stale-hours re-trigger loop for skip reasons that won't change
agent_terminal_succeeded() (line 340) correctly returns False for a ⏭️ Skipped terminal body, so latest_terminal_agent() never counts it as completed. In classify_launch_wait() (line 541+), when completed is None and the original launch signal goes stale (>= --stale-hours, default 6h), the function falls into the branch that recommends (and under --apply, re-posts) the exact same /fs-* command that just got skipped.
That's consistent with intent for genuine failures (per the earlier fix: "Failed/cancelled/terminated runs leave the launch wait in place so we can re-trigger after stale hours"), but a skip is different: skip reasons like ⏭️ Skipped (change is already in review on #123) describe a condition that a mere retry after N hours doesn't change. There's no test exercising the compounding scenario (skip → stale → re-trigger → skip again), and this is a distinct interaction from the already-fixed "skip treated as success" bug — that fix changed skip from clearing the wait to not clearing it, which is what now feeds it into the blind-retry path.
Suggestion: consider distinguishing skip from failure for retry purposes (e.g., only re-trigger a skip after a longer/backoff window, or surface it as human_work asking a person to confirm the skip reason no longer applies) instead of treating it identically to a hard failure that's likely to succeed on blind retry.
| return json.dumps(payload, indent=2) | ||
|
|
||
|
|
||
| def _format_item_line(item: dict[str, Any]) -> str: |
There was a problem hiding this comment.
[MEDIUM] Default markdown output never surfaces suggested_actions, so it doesn't stand alone as the documented "recommend the next action" tool
_format_item_line() renders only - {link} {title} — _{status}_: {reason} — no suggested_actions. format_markdown_output() calls this for every Do now / Waiting / Blocked / Assigned elsewhere item and never threads suggested_actions through at all, even though every Classification populates it and format_json_output emits it in full.
SKILL.md's own description states the skill's job is to "...classify every item into a status catalog, and recommend the next action", and --format markdown is the documented default. This isn't called out anywhere as a known limitation. Anyone running python3 skills/nextwork/scripts/nextwork.py directly (i.e., not through the /nextwork skill, which always forces --format json) sees only a status label and a reason, not the concrete recommended action.
Suggestion: render suggested_actions (or a short human-readable summary of it) per item in _format_item_line, at least in the "Do now" section, so direct markdown usage matches the tool's stated purpose.
| continue | ||
| # Launch/promote slash commands are handled by classify_launch_wait / | ||
| # waiting_code — they must not themselves flip completed triage stale. | ||
| cmd = comment_command(body) |
There was a problem hiding this comment.
[MEDIUM] Post-triage staleness exemption for /fs-* commands has no author-trust check, unlike the equivalent launch-signal path
is_completed_triage_stale() walks comments after a completed triage and, for each non-agent-marker comment, does cmd = comment_command(body); if cmd in ("/fs-code", "/fs-triage", "/fs-review", "/fs-fix"): continue — exempting it from counting as "non-exempt conversation" that would otherwise flip a fresh triage to needs_triage. This exemption calls comment_command(body) directly with no author/trust filtering.
By contrast, the equivalent launch-signal path was already hardened in a prior fix round: latest_fs_command_at() now requires _is_trusted_fs_commenter() (OWNER/MEMBER/COLLABORATOR authorAssociation or a fullsend bot) before trusting a /fs-* comment. is_completed_triage_stale() wasn't updated to use the same gate — any commenter, including an untrusted external user, can post a line starting with one of those four tokens to keep a triage looking "fresh" indefinitely, suppressing the needs_triage reclassification. This is a distinct code path from the already-fixed latest_fs_command_at/launch-signal trust gate.
Suggestion: filter the exemption check through _is_trusted_fs_commenter(c) (or equivalent) before treating a slash-command-prefixed comment as exempt from the post-triage staleness scan, matching the trust gate already applied to launch-signal detection.
|
🤖 Finished Retro · ✅ Success · Started 6:44 PM UTC · Completed 7:05 PM UTC |
Retro: PR #5641 — feat(skills): add /nextwork readiness queueTimeline. PR #5641 was a human-authored PR by ifireball adding a Review quality gap. The review bot found 0 HIGH-severity findings across all 14 rounds. Its only HIGH was
The bot's findings were entirely function-scoped edge cases, naming issues, and documentation gaps. It never performed cross-function or system-level analysis. Evidence for existing issues:
What went well. The human review process was exceptionally thorough — 49 distinct findings across 7 passes, all substantive, with zero false positives. The author resolved every finding promptly. The PR was well-tested (2300-line test file, codecov confirmed full coverage). The bot's Proposals filed |
Summary
Adds a
/nextworkskill and script that builds a readiness-oriented queue of assigned open issues/PRs, follows open GitHub blockers and sub-issues, classifies each item, and recommends the next action (with optional--apply/--take-over/--link-blocker).Unlike
/topissues, this has no RICE/project dependency — it answers “what can I work on next?” rather than “what is highest priority?”Related Issue
N/A
Changes
skills/nextwork/skill, Python classifier script, and unit testscommands/nextwork.md--apply(assign:self,/fs-*re-triggers, remove orphanedblockedlabel)Testing
skills/nextwork/scripts/nextwork_test.pymake lintpasses (stage changes first, then run)Checklist
!for breaking changes)Made with Cursor