refactor(tools): update GitHub tool slugs for clarity and remove deprecated entry#2766
Conversation
📝 WalkthroughWalkthroughThe PR updates the GitHub Composio provider tool catalog to remove a dedicated close-issue entry in favor of inline guidance using update-issue with a state parameter, and migrates the admin deletion tool from branch-specific to generic reference deletion. ChangesGitHub Composio Tool Catalog Entry Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@YellowSnnowmann hey! the code looks good to me — slug updates are correct, the error message in provider.rs is consistent, and the test assertions are aligned. a few notes:
Undocumented changes in tools.rs: The PR body only mentions GITHUB_GET_THE_AUTHENTICATED_USER and GITHUB_SEARCH_ISSUES_AND_PULL_REQUESTS, but the diff also renames GITHUB_LIST_REPOSITORIES_FOR_THE_AUTHENTICATED_USER, GITHUB_CREATE_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER, GITHUB_DELETE_A_BRANCH → GITHUB_DELETE_A_REFERENCE, and removes GITHUB_CLOSE_AN_ISSUE. These all look intentional and correct, but worth documenting in the PR body for the changelog — especially the CLOSE_AN_ISSUE removal and the branch→reference rename, since those are behavioral changes from the caller's perspective.
GITHUB_DELETE_A_REFERENCE: Worth a note in the tools comment that callers must pass a full ref path (e.g. refs/heads/branch-name), not just a branch name — DELETE_A_REFERENCE is more general than DELETE_A_BRANCH and would also delete tags if given the wrong ref. Low risk since this is curated tools, not user-facing input, but good to document.
Otherwise this is a clean fix. CI is nearly green — just one macOS e2e job still pending. once that clears, i'll come back and approve.
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Code Review
Overview
Small, focused slug audit — the runtime-critical constants in provider.rs are updated consistently with the curated list, and the doc comments/error strings are kept in sync. Validation evidence in the PR body (backend returning ConnectedAccountNotFound vs ToolNotFound) is the right way to confirm these slugs exist in Composio v3. One required fix before merge.
Required
Test coverage gap — curated_tools_contains_core_actions only asserts two of the six changed slugs. The three additional renames and the GITHUB_CLOSE_AN_ISSUE removal are uncovered; a future accidental revert won't be caught.
Suggested additions to tests.rs:
assert!(slugs.contains(&"GITHUB_LIST_REPOSITORIES_FOR_THE_AUTHENTICATED_USER"));
assert!(slugs.contains(&"GITHUB_CREATE_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER"));
assert!(slugs.contains(&"GITHUB_DELETE_A_REFERENCE"));
assert!(
!slugs.contains(&"GITHUB_CLOSE_AN_ISSUE"),
"GITHUB_CLOSE_AN_ISSUE removed — use GITHUB_UPDATE_AN_ISSUE with state:closed"
);Minor
GITHUB_DELETE_A_BRANCH → GITHUB_DELETE_A_REFERENCE is a semantic change. The Composio v3 action maps to GitHub's DELETE /repos/tinyhumansai/openhuman/git/refs/{ref}, which deletes any git reference (branches and tags alike), not just branches. Any caller that passes a ref expecting branch-only behavior could silently delete a tag. I've confirmed no agent prompts reference the old slug, so this is low risk — but worth a note in the commit message for ops awareness.
Nit
The removal comment in tools.rs is helpful; adding the concrete parameter makes it immediately actionable:
// GITHUB_CLOSE_AN_ISSUE — removed: no dedicated Composio slug.
// Use GITHUB_UPDATE_AN_ISSUE with {"state": "closed"} instead.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/memory_sync/composio/providers/github/tools.rs (1)
180-180: ⚡ Quick winConfirm old slug usage: only remains in the Composio GitHub fixture
The rename to
GITHUB_CANCEL_A_WORKFLOW_RUNmatches the Composio v3 naming convention, and there are no Rust/test references to the oldGITHUB_CANCEL_WORKFLOW_RUNoutside of test data. The only remaining occurrence of the old slug is intests/fixtures/composio_github.json. Consider regenerating that fixture if it’s meant to mirror the current tool dump to avoid fixture drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_sync/composio/providers/github/tools.rs` at line 180, The tool slug was renamed to GITHUB_CANCEL_A_WORKFLOW_RUN (previously GITHUB_CANCEL_WORKFLOW_RUN) but the test fixture file composio_github.json still contains the old slug; update that fixture to use GITHUB_CANCEL_A_WORKFLOW_RUN (or regenerate the Composio GitHub fixture from the current tool dump) so tests match the new slug used by the provider code (look for occurrences of GITHUB_CANCEL_WORKFLOW_RUN and replace/regenerate accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/memory_sync/composio/providers/github/tools.rs`:
- Line 180: The tool slug was renamed to GITHUB_CANCEL_A_WORKFLOW_RUN
(previously GITHUB_CANCEL_WORKFLOW_RUN) but the test fixture file
composio_github.json still contains the old slug; update that fixture to use
GITHUB_CANCEL_A_WORKFLOW_RUN (or regenerate the Composio GitHub fixture from the
current tool dump) so tests match the new slug used by the provider code (look
for occurrences of GITHUB_CANCEL_WORKFLOW_RUN and replace/regenerate
accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 904d1acb-1fe0-469b-9243-4bee11726a0f
📒 Files selected for processing (1)
src/openhuman/memory_sync/composio/providers/github/tools.rs
…rated_tools_contains_core_actions Addresses review feedback on tinyhumansai#2766: the previous assertion only covered two of the six changed slugs, leaving LIST_REPOSITORIES, CREATE_A_REPOSITORY, DELETE_A_REFERENCE, and the CLOSE_AN_ISSUE removal undetected by tests. Also adds an inline comment on GITHUB_DELETE_A_REFERENCE documenting that callers must pass a full ref path (refs/heads/ or refs/tags/) — the action maps to GitHub's generic DELETE /git/refs/{ref} endpoint which can delete tags as well as branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/memory_sync/composio/providers/github/tests.rs (1)
170-171: ⚡ Quick winConfirm
GITHUB_CURATEDcontains the new repository-action slugs.
GITHUB_LIST_REPOSITORIES_FOR_THE_AUTHENTICATED_USERandGITHUB_CREATE_A_REPOSITORY_FOR_THE_AUTHENTICATED_USERboth exist insrc/openhuman/memory_sync/composio/providers/github/tools.rswithin theGITHUB_CURATEDarray, so the newtests.rsassertions align with the curated source.- Consider adding a brief comment explaining why these repository actions are intentionally included relative to the PR’s stated scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_sync/composio/providers/github/tests.rs` around lines 170 - 171, The test should explicitly assert the two repository-action slugs are present and explain why they’re included: keep the assertions for "GITHUB_LIST_REPOSITORIES_FOR_THE_AUTHENTICATED_USER" and "GITHUB_CREATE_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER" in src/openhuman/memory_sync/composio/providers/github/tests.rs, and add a short inline comment above those asserts stating that these slugs are present because they are part of the curated GITHUB_CURATED array in src/openhuman/memory_sync/composio/providers/github/tools.rs and are intentionally included despite the PR’s narrower scope; reference the GITHUB_CURATED symbol and the two slug strings in the comment so future reviewers see the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/memory_sync/composio/providers/github/tests.rs`:
- Around line 170-171: The test should explicitly assert the two
repository-action slugs are present and explain why they’re included: keep the
assertions for "GITHUB_LIST_REPOSITORIES_FOR_THE_AUTHENTICATED_USER" and
"GITHUB_CREATE_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER" in
src/openhuman/memory_sync/composio/providers/github/tests.rs, and add a short
inline comment above those asserts stating that these slugs are present because
they are part of the curated GITHUB_CURATED array in
src/openhuman/memory_sync/composio/providers/github/tools.rs and are
intentionally included despite the PR’s narrower scope; reference the
GITHUB_CURATED symbol and the two slug strings in the comment so future
reviewers see the rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2eb415c-a385-4c66-8556-6d171bd220c8
📒 Files selected for processing (2)
src/openhuman/memory_sync/composio/providers/github/tests.rssrc/openhuman/memory_sync/composio/providers/github/tools.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/memory_sync/composio/providers/github/tools.rs
|
Thanks for the thorough reviews, @graycyrus and @M3gA-Mind! Pushed cb6540b addressing all feedback: @M3gA-Mind — required:
@graycyrus + @M3gA-Mind —
@graycyrus — undocumented changes:
@M3gA-Mind — nit ( |
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
LGTM — the core fix (adding as a live entry) is correct and the expanded test coverage is solid. Approving.
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
LGTM — the core fix (adding GITHUB_DELETE_A_REFERENCE as a live CuratedTool entry) is correct and the expanded test coverage is solid. Approving.
4 participants
fix(composio): update GitHub action slugs
Summary
GITHUB_GET_THE_AUTHENTICATED_USERGITHUB_SEARCH_ISSUES_AND_PULL_REQUESTSValidation
ActionExecute_ConnectedAccountNotFound, which confirms the tool slug is recognized but the account is not connected.GITHUB_GET_AUTHENTICATED_USERreturnedTool_ToolNotFound.cargo test --manifest-path /Users/cardinal/Desktop/projects/main/openhuman/Cargo.toml curated_tools_contains_core_actions --libResult: passed.
Closes: #2768
Notes
Summary by CodeRabbit