Skip to content

Conversation

@kdy1
Copy link
Member

@kdy1 kdy1 commented Oct 30, 2025

Summary

  • Adds issues: write permission to the publish workflow to fix milestone management failures

Problem

The "Manage milestone" step in the publish workflow was failing with:

❌ Error managing milestone: Resource not accessible by integration

This occurred because the workflow lacked the necessary permissions to create and modify GitHub milestones through the API.

Solution

Added issues: write permission to the workflow's permission block. This grants the workflow the required access to:

  • Create new milestones for releases
  • Update milestone assignments on issues and PRs
  • Move items from the "Planned" milestone to version-specific milestones

Test plan

🤖 Generated with Claude Code

Fixes the "Resource not accessible by integration" error that occurs
when the publish workflow attempts to create or modify milestones.
The manage-milestone step requires issues:write permission to interact
with GitHub's milestone API.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@kdy1 kdy1 requested a review from a team as a code owner October 30, 2025 04:22
@changeset-bot
Copy link

changeset-bot bot commented Oct 30, 2025

⚠️ No Changeset found

Latest commit: c3bdfeb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kdy1 kdy1 merged commit 6c68a1e into main Oct 30, 2025
22 checks passed
@kdy1 kdy1 deleted the kdy1/perm branch October 30, 2025 04:22
@claude
Copy link
Contributor

claude bot commented Oct 30, 2025

Claude finished @kdy1's task —— View job


PR Review: Add issues write permission for milestone management

Review Tasks:

  • Read and analyze the workflow file changes
  • Evaluate security implications of the permission change
  • Check if the permission scope is appropriate
  • Assess potential risks and alternatives
  • Provide comprehensive feedback

✅ Review Summary

This PR appropriately adds the minimal required permission to fix milestone management failures in the publish workflow. The change is well-documented and addresses a specific operational issue.

📋 Detailed Analysis

Code Quality: ✅ Good

  • Minimal change: Only adds the required issues: write permission on line 19
  • Proper placement: Permission added in the correct permissions block
  • Clear intent: The change directly addresses the documented error

Security Analysis: ✅ Acceptable

Permission Scope:

  • issues: write is the minimal required permission for milestone operations
  • ✅ Scoped correctly to the workflow level, not job level
  • ✅ Only grants access to issues and milestones, not repository contents beyond existing contents: write

Risk Assessment:

  • Low Risk: This permission allows the workflow to:
    • Create milestones (.github/bot/src/milestone-manager.ts:24)
    • List existing milestones (.github/bot/src/milestone-manager.ts:58)
    • Update issue/PR milestone assignments (.github/bot/src/milestone-manager.ts:97)
  • Mitigation: Workflow only runs on workflow_dispatch from main branch, limiting exposure
  • Context: Automated milestone management is a legitimate CI/CD operation for release workflows

Technical Implementation: ✅ Excellent

Specific Operations Requiring This Permission:

  1. octokit.issues.createMilestone() - Creates new release milestones
  2. octokit.issues.listMilestones() - Lists existing milestones
  3. octokit.issues.update() - Moves completed issues/PRs from "Planned" to version-specific milestone

Alternative Analysis:

  • ❌ Could not use metadata: read - insufficient for milestone creation/updates
  • ❌ Could not scope to job level - milestone operations span multiple jobs
  • ✅ Current approach is optimal for the requirements

Performance Considerations: ✅ Good

  • No performance impact from permission change
  • Script includes proper error handling and batch processing
  • Reasonable rate limiting with GitHub API calls

Test Coverage: ⚠️ Partially Covered

  • ✅ Error scenario verified (failed workflow run link provided)
  • ⚠️ Success scenario testing noted as pending in test plan
  • Recommendation: Monitor the next publish workflow run to confirm the fix works

🔍 Code Analysis: .github/bot/src/milestone-manager.ts:128-142

The milestone management logic is well-structured:

  • Proper error handling for existing milestones (.github/bot/src/milestone-manager.ts:36)
  • Only moves closed issues/PRs (.github/bot/src/milestone-manager.ts:92-94)
  • Comprehensive logging for debugging
  • Graceful handling of missing "Planned" milestone

📊 Overall Assessment

Aspect Rating Notes
Security ✅ Good Minimal necessary permissions
Code Quality ✅ Good Clean, focused change
Documentation ✅ Good Clear problem/solution description
Testing ⚠️ Partial Success case pending verification

🎯 Recommendations

  1. Immediate: No changes needed - this is a solid implementation
  2. Follow-up: Verify the workflow succeeds in the next publish run
  3. Future: Consider adding automated tests for milestone management logic

This change effectively resolves the milestone management failures while maintaining security best practices. The minimal permission scope and clear documentation make this an exemplary infrastructure fix.


@github-actions
Copy link
Contributor

Binary Sizes

File Size
swc.linux-x64-gnu.node 31M (32023392 bytes)

Commit: 0fdc717

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 30, 2025

CodSpeed Performance Report

Merging #11204 will not alter performance

Comparing kdy1/perm (c3bdfeb) with main (71dc547)

Summary

✅ 138 untouched
⏩ 13 skipped1

Footnotes

  1. 13 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants