Skip to content

ci: 共通アクションでコメント最小化を集約 - #1282

Merged
dyoshikawa merged 5 commits into
mainfrom
feat/gha-common-minimize-comments
Mar 8, 2026
Merged

ci: 共通アクションでコメント最小化を集約#1282
dyoshikawa merged 5 commits into
mainfrom
feat/gha-common-minimize-comments

Conversation

@dyoshikawa-claw

@dyoshikawa-claw dyoshikawa-claw commented Mar 7, 2026

Copy link
Copy Markdown
Collaborator

概要

  • コメント最小化処理を reusable composite action に切り出し
  • issue/PR コメント起点ジョブの両方で共通アクションを利用
  • 既存のbotコメント最小化対象(opencode-agent, github-actions系)をデフォルトで維持

変更内容

  • 追加: .github/actions/minimize-comments/action.yml
    • trigger_comment_node_id がある場合はトリガーコメントを最小化
    • owner/repo/pr_number 指定時はPRレビューコメントとIssueコメントの旧botコメントを最小化
    • bot_logins はカンマ区切りで上書き可能(デフォルトは現行互換)
  • 更新: .github/workflows/comment-to-ai.yml
    • handle-issue / handle-pr-comment で新アクションを利用

動作確認

  • ローカルでの yamllint は環境に存在せず未実施
  • 既存処理の置き換えのみで、挙動は互換を維持

@dyoshikawa-claw

This comment has been minimized.

@dyoshikawa

This comment has been minimized.

Co-authored-by: dyoshikawa <dyoshikawa@users.noreply.github.com>
@github-actions

This comment has been minimized.

@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@dyoshikawa-claw

This comment has been minimized.

@github-actions

github-actions Bot commented Mar 8, 2026

Copy link
Copy Markdown
Contributor

PR #1282 Review Summary

Overall Mergeability Verdict: Mergeable (No Medium/High/Critical Blockers)

The PR successfully extracts reusable comment minimization logic into a composite action with good code quality and security practices.


Code Review Findings

  1. Medium - Undocumented Behavior Change

    • Location: .github/actions/minimize-comments/action.yml:31-35
    • The new code catches minimization failures and continues with a warning, which is an improvement over the original (which would fail the entire step). However, this behavior change should be documented.
  2. Low - Client-Side Filtering Performance

    • Location: .github/actions/minimize-comments/action.yml:87-93
    • Original filtered server-side via jq select(), new code fetches all comments and filters in bash. Minimal impact for typical PRs.
  3. Low - Empty Login Edge Case

    • Location: .github/actions/minimize-comments/action.yml:73-74
    • Checks for empty node_id but not empty login. Could affect deleted accounts.
  4. Low - Unrelated Config Change

    • Location: .serena/project.yml:117-120
    • Addition of read_only_memory_patterns: [] appears unrelated to this PR's purpose.
  5. Low - Missing Usage Example

    • The action lacks a usage example in its description.

Security Review Findings

  1. ✅ PASSING - Script Injection Prevention

    • All inputs passed through environment variables, not interpolated into shell commands.
  2. ✅ PASSING - GraphQL Injection Prevention

    • Uses parameterized queries via -f id="${COMMENT_NODE_ID}".
  3. ✅ PASSING - Secrets Handling

    • GITHUB_TOKEN correctly passed via environment variables.
  4. ✅ PASSING - Input Validation

    • Proper validation added for empty inputs (lines 24, 38, 57-60).
  5. ✅ PASSING - Error Handling

    • Graceful failure handling with warning messages - an improvement over the original.
  6. Informational - Node ID Logged

    • node_id appears in warning logs on failure (not sensitive data).
  7. Informational - Configurable Bot Logins

    • bot_logins input is properly sanitized and only used for string comparison, not execution.

Summary

Category Status
Security ✅ Safe (follows best practices)
Code Quality ✅ Acceptable (1 medium documentation issue, several low)

Recommendation: The PR is mergeable. Consider documenting the behavior change (issue #1) either in the PR description or as a follow-up. The low-severity issues are optional improvements.

github run

github run

@dyoshikawa
dyoshikawa merged commit 856911a into main Mar 8, 2026
11 checks passed
@dyoshikawa
dyoshikawa deleted the feat/gha-common-minimize-comments branch March 8, 2026 08:39
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.

2 participants