-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Adding optional sticky_comment_app_bot_id and sticky_comment_app_bot_name options
#411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@ashwin-ant this looks reasonable to me. Do you see any security concerns of users potentially masking Claude's bot ID with another? |
|
could i make one slight request for an addition? it'd be great if we could include a string value to match against, that is injected as a hidden html comment within the comment. this would be helpful if we used sticky comments in the experimental review mode which uses the default github token. |
That seems like a great potential enhancement, not sure if it should be scope creeping this PR though :) This was a gentle un-hardcoding of a few constants .. adding new logic would probably require adding some tests or something .. which I'm not really up for contributing right now :) |
Understandable! I'll probably make a pr for this! |
As far as I could tell, this bot user ID is ONLY ever used to create the initial comment, and in there its only used to determine if an existing comment came from the bot, so there isn't really anything its guarding from some kind of masking .. |
…ot_name` options
|
Rebased with current main since there was conflicts |
|
Hi @km-anthropic, would it be possible to have this change merged? |
|
@km-anthropic do you know when this feature will be merged ? |
|
@ashwin-ant |
Implement dual-path architecture to support both internal and cross-repository pull request reviews while navigating GitHub's OIDC security boundaries. ## Architecture Changes ### Dual-trigger system - Add `pull_request` trigger alongside existing `pull_request_target` - Internal PRs use pull_request + OIDC authentication (claude[bot] identity) - Fork PRs use pull_request_target + github_token (github-actions[bot] identity) - Job-level XOR logic ensures only one path executes per PR **Why:** GitHub prevents OIDC with pull_request_target to protect secrets from untrusted forks. claude-code-action requires OIDC for claude[bot] identity, forcing us to use github_token for forks (creates github-actions[bot] instead). ### Conditional action steps Replace single action step with two conditional steps: **Internal Review:** - Trigger: pull_request event + same repository - Auth: claude_code_oauth_token (OIDC) - Identity: claude[bot] - Features: Full sticky comment support **Fork Review:** - Trigger: pull_request_target event + different repository - Auth: github_token (GITHUB_TOKEN secret) - Identity: github-actions[bot] - Features: allowed_non_write_users: '*' for fork contributors ## Review Quality Improvements ### Comprehensive review prompt Move from inline checklist to sophisticated multi-stage review process: **Context gathering phase:** - Check CI status via mcp__github_ci__get_ci_status - Fetch previous reviews via gh pr view --comments - Track previously identified issues across commits - Analyze workflow failures via mcp__github_ci__get_workflow_run_details **Review approach:** - First review: Comprehensive analysis of all changes - Subsequent reviews: Incremental feedback tracking issue status (Fixed/Unresolved/New) - Incorporate CI/test failure context into feedback - Prohibit meta-commentary about review process itself **Review criteria:** - Code quality and best practices - Potential bugs and security implications - Performance considerations and test coverage - assistant-ui requirements (changesets, documentation) ### Inline comment quality controls - Restrict to critical code issues or relevant suggestions - Check for existing comments to prevent duplicates - Prohibit use for good practices or positive observations - Require actionable, precise feedback ### Tool restrictions - Remove gh pr comment permissions (prevents spam) - Require use of mcp__github_comment__update_claude_comment - Allow read-only gh commands (pr view, diff, list) - Enable inline comments via mcp__github_inline_comment__create_inline_comment ## Fork PR Comment Management Add GraphQL minimization script for fork PRs: - Collapses previous github-actions[bot] comments as OUTDATED - Runs only on cross-repository PRs (pull_request_target) - Supports pagination for handling many comments - No external dependencies (pure github-script@v8) **Why:** Fork PRs can't use sticky comments due to bot identity change (github-actions[bot] instead of claude[bot]). Minimizing old comments keeps discussion clean until sticky comment support is added. **Note:** Temporary workaround until anthropics/claude-code-action#411 merges. PR #411 adds sticky_comment_app_bot_id/name config for github-actions[bot]. ## CI Integration Enable progress tracking and CI tool access: - Set TRACK_PROGRESS: 'true' to load mcp__github_ci__* tools - Add additional_permissions: actions: read - Enable Claude to access test results and workflow run details - Support context-aware reviews based on CI status ## Configuration Improvements ### DRY environment variables Extract shared configuration to env block: - REVIEW_PROMPT: Comprehensive review instructions - CLAUDE_ARGS: Tool allowlist and restrictions - TRACK_PROGRESS: Enable CI MCP tools - USE_STICKY_COMMENT: Enable sticky comment mode - ADDITIONAL_PERMISSIONS: Grant actions: read access ### Checkout optimization - Change from head.repo.full_name + head.ref to refs/pull/{number}/head - Universal reference works for both internal and cross-repository PRs - Ensures correct code is reviewed regardless of fork status ### Resource controls - Add timeout-minutes: 15 to prevent runaway costs - Upgrade to checkout@v5 and github-script@v8 ## claude.yml Refinements Reduce workflow noise and modernize dependencies: - Remove pull_request_review trigger (eliminates duplicate check runs) - Simplify bot filtering (delegate to claude-code-action) - Remove manual github.event.sender.type == 'User' checks - Upgrade actions/github-script@v7 → @v8 - Upgrade actions/checkout@v4 → @v5
Implement dual-path architecture to support both internal and cross-repository pull request reviews while navigating GitHub's OIDC security boundaries. ## Architecture Changes ### Dual-trigger system - Add `pull_request` trigger alongside existing `pull_request_target` - Internal PRs use pull_request + OIDC authentication (claude[bot] identity) - Fork PRs use pull_request_target + github_token (github-actions[bot] identity) - Job-level XOR logic ensures only one path executes per PR **Why:** GitHub prevents OIDC with pull_request_target to protect secrets from untrusted forks. claude-code-action requires OIDC for claude[bot] identity, forcing us to use github_token for forks (creates github-actions[bot] instead). ### Conditional action steps Replace single action step with two conditional steps: **Internal Review:** - Trigger: pull_request event + same repository - Auth: claude_code_oauth_token (OIDC) - Identity: claude[bot] - Features: Full sticky comment support **Fork Review:** - Trigger: pull_request_target event + different repository - Auth: github_token (GITHUB_TOKEN secret) - Identity: github-actions[bot] - Features: allowed_non_write_users: '*' for fork contributors ## Review Quality Improvements ### Comprehensive review prompt Move from inline checklist to sophisticated multi-stage review process: **Context gathering phase:** - Check CI status via mcp__github_ci__get_ci_status - Fetch previous reviews via gh pr view --comments - Track previously identified issues across commits - Analyze workflow failures via mcp__github_ci__get_workflow_run_details **Review approach:** - First review: Comprehensive analysis of all changes - Subsequent reviews: Incremental feedback tracking issue status (Fixed/Unresolved/New) - Incorporate CI/test failure context into feedback - Prohibit meta-commentary about review process itself **Review criteria:** - Code quality and best practices - Potential bugs and security implications - Performance considerations and test coverage - assistant-ui requirements (changesets, documentation) ### Inline comment quality controls - Restrict to critical code issues or relevant suggestions - Check for existing comments to prevent duplicates - Prohibit use for good practices or positive observations - Require actionable, precise feedback ### Tool restrictions - Remove gh pr comment permissions (prevents spam) - Require use of mcp__github_comment__update_claude_comment - Allow read-only gh commands (pr view, diff, list) - Enable inline comments via mcp__github_inline_comment__create_inline_comment ## Fork PR Comment Management Add GraphQL minimization script for fork PRs: - Collapses previous github-actions[bot] comments as OUTDATED - Runs only on cross-repository PRs (pull_request_target) - Supports pagination for handling many comments - No external dependencies (pure github-script@v8) **Why:** Fork PRs can't use sticky comments due to bot identity change (github-actions[bot] instead of claude[bot]). Minimizing old comments keeps discussion clean until sticky comment support is added. **Note:** Temporary workaround until anthropics/claude-code-action#411 merges. PR #411 adds sticky_comment_app_bot_id/name config for github-actions[bot]. ## CI Integration Enable progress tracking and CI tool access: - Set TRACK_PROGRESS: 'true' to load mcp__github_ci__* tools - Add additional_permissions: actions: read - Enable Claude to access test results and workflow run details - Support context-aware reviews based on CI status ## Configuration Improvements ### DRY environment variables Extract shared configuration to env block: - REVIEW_PROMPT: Comprehensive review instructions - CLAUDE_ARGS: Tool allowlist and restrictions - TRACK_PROGRESS: Enable CI MCP tools - USE_STICKY_COMMENT: Enable sticky comment mode - ADDITIONAL_PERMISSIONS: Grant actions: read access ### Checkout optimization - Change from head.repo.full_name + head.ref to refs/pull/{number}/head - Universal reference works for both internal and cross-repository PRs - Ensures correct code is reviewed regardless of fork status ### Resource controls - Add timeout-minutes: 15 to prevent runaway costs - Upgrade to checkout@v5 and github-script@v8 ## claude.yml Refinements Reduce workflow noise and modernize dependencies: - Remove pull_request_review trigger (eliminates duplicate check runs) - Simplify bot filtering (delegate to claude-code-action) - Remove manual github.event.sender.type == 'User' checks - Upgrade actions/github-script@v7 → @v8 - Upgrade actions/checkout@v4 → @v5
This is to address the issues in #207 around this being quite hardcoded to Claude's own github app, adding two options to override the default behavior and have more flexible sticky comment behaviour.
sticky_comment_app_bot_idcan just be set to your custom GH App idsticky_comment_app_bot_nameif for some reason you can't just use the ID, you can also override the name being matched, this allows you to match for any bot user with its name containing this string