Skip to content

feat(skills): add /topissues slash command for RICE backlog - #1232

Merged
ifireball merged 1 commit into
fullsend-ai:mainfrom
ifireball:cursor/63c173df
May 27, 2026
Merged

feat(skills): add /topissues slash command for RICE backlog#1232
ifireball merged 1 commit into
fullsend-ai:mainfrom
ifireball:cursor/63c173df

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

  • Add /topissues slash command (Cursor + Claude Code) backed by a stdlib Python script that builds a merged RICE priority table via gh GraphQL
  • Introduce canonical commands/ tree with .cursor/commands and .claude/commands symlinks (mirrors existing skills layout)
  • Wire unit tests into make script-test the same way as process-fix-result-test.py

Test plan

  • python3 skills/topissues/scripts/topissues_test.py
  • make script-test (full suite)
  • Manual: python3 skills/topissues/scripts/topissues.py in a repo with RICE-scored issues
  • Confirm /topissues appears in Cursor and Claude Code command help

Made with Cursor

@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown

Site preview

Preview: https://7c8f3b2b-site.fullsend-ai.workers.dev

Commit: bf540b1102c88457ba6b15443b4c7ac63a438459

@fullsend-ai-review

fullsend-ai-review Bot commented May 20, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink at .claude/commands -> ../commands, which falls under the .claude/ protected path. The change mirrors the established .claude/skills -> ../skills pattern and the rationale is described in the PR body, but no linked issue authorizes this change. Human approval is required for all protected-path modifications. Protected file: .claude/commands.
    Remediation: Link a tracking issue that authorizes the .claude/ directory change, or have a human maintainer explicitly approve this path.
Previous run

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a new symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue authorizing this change. While the description explains the intent (mirroring existing .claude/skills symlink pattern), protected-path modifications require a linked issue for traceability.
    Remediation: Link an issue authorizing the commands symlink addition, or have a human reviewer explicitly approve the protected-path change.

Medium

  • [correctness] Makefile:91 — The GH_TOKEN= GITHUB_TOKEN= prefix was removed from the go-test target. This guard explicitly cleared GitHub tokens to prevent Go tests from making real API calls when tokens exist in the environment. Removing it is an unrelated behavioral change that could cause tests to hit live GitHub APIs in CI or local environments where tokens are set.
    Remediation: Restore GH_TOKEN= GITHUB_TOKEN= in the go-test target, or split this change into a separate PR with justification for why the guard is no longer needed.

  • [correctness] Makefile:109 — The post-prioritize-test.sh invocation was removed from the script-test target, but the test file still exists at internal/scaffold/fullsend-repo/scripts/post-prioritize-test.sh. This silently drops test coverage for the prioritize post-script.
    Remediation: Either keep post-prioritize-test.sh in the script-test target, or if the test is intentionally being retired, remove the test file in the same PR and explain why.

Low

  • [intent-alignment] Makefile — Several changes in the Makefile are unrelated to the /topissues feature: removing the token guard from go-test, removing post-prioritize-test.sh, and changing the test target from lint-all to lint go-vet. These are not mentioned in the PR description. The lint-alllint change narrows pre-commit scope from all files to staged files only, which may weaken CI linting.
    Remediation: Split unrelated Makefile changes into a separate PR, or update the PR description to explain the rationale for each change.
Previous run (2)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue (closingIssuesReferences is empty) and the description does not reference an authorizing issue. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the rationale is clear from context, protected-path changes require a linked issue for traceability.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.
Previous run (3)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue (closingIssuesReferences is empty) and the description does not reference an authorizing issue. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the rationale is clear from context, protected-path changes require a linked issue for traceability.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.

Prior review note: the low-severity [correctness] finding from the previous review (re resolve_project_number env-var parsing without try/except ValueError) is resolved — the code now correctly wraps int(env_val) in try/except ValueError (lines 407–410).

Previous run (4)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue (closingIssuesReferences is empty) and the description does not reference an authorizing issue. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the rationale is clear from context, protected-path changes require a linked issue for traceability.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.

Low

  • [correctness] skills/topissues/scripts/topissues.py:390resolve_project_number parses the FULLSEND_PROJECT_NUMBER environment variable with a bare int(env_val) call (no try/except ValueError). If the env var is set to a non-numeric string (e.g. "abc"), the script raises an unhandled ValueError with a raw traceback instead of the friendly error message at the end of the function. The try_run_gh fallback path just below correctly wraps int() in try/except ValueError, but the env-var path does not.
    Remediation: Wrap the int(env_val) call in a try/except ValueError block that falls through to the friendly error message, matching the pattern used for the org-variable fallback.

Prior review note: the medium-severity [correctness] finding from the previous review (re resolve_project_number using run_gh) is resolved — the code now correctly uses try_run_gh, which returns None on failure and allows the friendly error message to be reached.

Previous run (5)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue (closingIssuesReferences is empty) and the description does not reference an authorizing issue. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the rationale is clear from context, protected-path changes require a linked issue for traceability.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.

Medium

  • [correctness] skills/topissues/scripts/topissues.py:388resolve_project_number attempts to fall through gracefully when gh variable get fails (lines 388–395), but run_gh calls sys.exit(3) on CalledProcessError instead of raising. SystemExit is not caught by except (subprocess.CalledProcessError, ValueError), so the friendly error message at lines 396–401 ("project number required; set FULLSEND_PROJECT_NUMBER…") is unreachable when the gh command fails. Users who lack the org variable get a raw gh CLI error (exit 3) instead of the intended guidance listing all three ways to provide the project number.
    Remediation: Use subprocess.run directly (without run_gh) for this call so CalledProcessError can be caught, or add a check=False variant of run_gh that returns the result instead of exiting.
Previous run (6)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue (closingIssuesReferences is empty) and the description does not reference an authorizing issue. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the rationale is clear from context, protected-path changes require a linked issue for traceability.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.

Info

  • [correctness] skills/topissues/scripts/topissues.py:31 — The ISSUES_QUERY fetches comments(first: 100) as a nested connection without pagination. Issues with more than 100 comments may have their RICE score comment missed if it falls outside the first 100. This is a practical limitation unlikely to matter for most repositories.
    Remediation: Consider paginating comments or fetching them in reverse chronological order (most recent RICE score is likely near the end).
Previous run (7)

Review

Findings

High

  • [protected-path] .claude/commands — This PR adds a symlink under .claude/, which is a protected path requiring human approval. The PR has no linked issue to authorize the change. While the symlink mirrors the existing .claude/skills -> ../skills pattern and the PR description explains the rationale, protected-path changes require a linked issue for traceability. Human review is required before merging.
    Remediation: Link a GitHub issue authorizing the .claude/commands symlink addition, or have a maintainer explicitly approve the protected-path change.

Low

  • [correctness] skills/topissues/scripts/topissues.py:76 — The RICE_SCORE_RE regex uses [\\d.]+ which can match invalid float strings like 1.2.3 or .... If such a string appears in a comment, float(match.group(1)) will raise an unhandled ValueError. While RICE scores are agent-generated and unlikely to be malformed, defensive parsing would be more robust.
    Remediation: Wrap the float() call in a try/except ValueError, or tighten the regex to \\d+(?:\\.\\d+)?.

Info

  • [correctness] skills/topissues/scripts/topissues.py:31 — The GraphQL query fetches comments(first: 100) without pagination. Issues with more than 100 comments may have their RICE score comments truncated, causing the script to miss or undercount scores. This is a practical limitation unlikely to matter for most repositories.
    Remediation: Consider paginating comments or fetching them in reverse chronological order (most recent RICE score is likely near the end).

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread skills/topissues/scripts/topissues.py Outdated
Comment thread skills/topissues/scripts/topissues.py
@ifireball ifireball self-assigned this May 20, 2026
@ifireball

Copy link
Copy Markdown
Member Author

Maintainer sign-off (@ifireball)

Protected-path (/.claude/commands): Approved on behalf of maintainers. This mirrors the existing .claude/skills../skills symlink pattern; the canonical commands/ tree is required for portable /topissues in Cursor and Claude Code.

Review agent findings:

  • RICE regex (low): Fixed in decf4df5 — see thread reply.
  • Comment pagination (info): Accepted v1 limitation — see thread reply.

(GitHub blocks self-approval on own PRs; this comment is explicit maintainer authorization for the protected-path change.)

@ifireball
ifireball marked this pull request as ready for review May 20, 2026 07:13

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread skills/topissues/scripts/topissues.py
@ifireball

Copy link
Copy Markdown
Member Author

Pushed 7f786811: /topissues now reads RICE Score from the org GitHub Project board only (no issue comments). Adds --project N and resolves project number from FULLSEND_PROJECT_NUMBER env or org variable.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread skills/topissues/scripts/topissues.py Outdated

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread skills/topissues/scripts/topissues.py

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

ralphbean

This comment was marked as low quality.

ralphbean

This comment was marked as low quality.

@ifireball

Copy link
Copy Markdown
Member Author

@ralphbean since you marked your review comments as low quality, should I regard that as a signal to ignore them and go ahead and merge the PR ?

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. :)

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread Makefile Outdated

go-test:
GH_TOKEN= GITHUB_TOKEN= go test -race -cover ./...
go test -race -cover ./...

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] correctness

GH_TOKEN= GITHUB_TOKEN= guard removed from go-test target. This was an explicit safeguard preventing Go tests from making real GitHub API calls. Removing it is an unrelated behavioral change.

Suggested fix: Restore the token-clearing prefix or justify removal in a separate PR.

Comment thread Makefile
Add a stdlib Python script and portable slash command that builds a merged
markdown/JSON RICE table from the org GitHub Project board (same fields as
post-prioritize.sh): top unassigned backlog issues plus the caller's assigned
work. Resolve FULLSEND_PROJECT_NUMBER via org variable with friendly fallbacks,
skip items with open PRs or blockedBy links, and wire unit tests into make
script-test. Introduce a canonical commands/ tree with .cursor/commands and
.claude/commands symlinks mirroring the skills layout.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread .claude/commands
@ifireball
ifireball added this pull request to the merge queue May 27, 2026
Merged via the queue into fullsend-ai:main with commit ad3531f May 27, 2026
12 checks passed
@ifireball
ifireball deleted the cursor/63c173df branch May 27, 2026 10:31
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.

3 participants