ci: keep release PRs mergeable#436
Merged
Merged
Conversation
github-actions
Bot
added
the
area:ci
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoves the ChangesRelease automation CODEOWNERS & workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@apps/desktop/tests/ci/release-codeowners-policy.test.ts`:
- Around line 14-20: The parser function codeownerPatterns currently discards
owner identities by returning only patterns; change it to parse and return both
the pattern and its owners (e.g., return an array of objects like {pattern,
owners}) so tests can assert owner membership; update any test logic that calls
codeownerPatterns (and the assertions around release-control checks) to verify
that the matching CODEOWNERS entry for release-control files includes the
required owner string (e.g., '`@hiqiancheng`') rather than only checking pattern
coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b80a3506-1644-4ed4-af3c-9f2110a6509a
📒 Files selected for processing (2)
.github/CODEOWNERSapps/desktop/tests/ci/release-codeowners-policy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Desktop E2E Smoke (Windows)
- GitHub Check: CodeQL (rust)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: Rust Checks
- GitHub Check: Frontend Quality
- GitHub Check: Frontend Tests
🔇 Additional comments (1)
.github/CODEOWNERS (1)
10-11: LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/tests/ci/release-codeowners-policy.test.ts (1)
40-45: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winImplement last-match-wins semantics for CODEOWNERS ownership.
The function uses
.some()to check if any matching entry includes@hiqiancheng, but CODEOWNERS semantics specify that when multiple patterns match the same path, the last matching pattern determines ownership. If overlapping rules are added later (e.g., a broader/apps/desktop/scripts/@teamafter `/apps/desktop/scripts/ci/ `@hiqiancheng), this test could pass even though actual ownership has changed.The past review suggested using
.filter().at(-1)to retrieve the last matching entry. This ensures the test correctly validates that the effective owner (per CODEOWNERS semantics) is@hiqiancheng, preventing false positives in lines 67-71.🔧 Apply the past review's suggested fix
+function ownersForPath(codeowners: string, path: string) { + const matches = codeownerEntries(codeowners).filter(({ pattern }) => + patternMatchesPath(pattern, path) + ); + return matches.at(-1)?.owners ?? []; +} + function ownedByMaintainer(codeowners: string, path: string) { - return codeownerEntries(codeowners).some( - ({ owners, pattern }) => - patternMatchesPath(pattern, path) && owners.includes('`@hiqiancheng`') - ); + return ownersForPath(codeowners, path).includes('`@hiqiancheng`'); }🤖 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 `@apps/desktop/tests/ci/release-codeowners-policy.test.ts` around lines 40 - 45, The ownedByMaintainer helper currently uses .some() which checks any matching entry, but CODEOWNERS uses last-match-wins; update ownedByMaintainer to collect matching entries from codeownerEntries(...) using patternMatchesPath(...) then pick the last match (e.g., .filter(...).at(-1) or .filter(...).slice(-1)[0]) and return whether that last entry's owners includes '`@hiqiancheng`'; reference the ownedByMaintainer function, codeownerEntries, and patternMatchesPath when making the change.
🤖 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.
Duplicate comments:
In `@apps/desktop/tests/ci/release-codeowners-policy.test.ts`:
- Around line 40-45: The ownedByMaintainer helper currently uses .some() which
checks any matching entry, but CODEOWNERS uses last-match-wins; update
ownedByMaintainer to collect matching entries from codeownerEntries(...) using
patternMatchesPath(...) then pick the last match (e.g., .filter(...).at(-1) or
.filter(...).slice(-1)[0]) and return whether that last entry's owners includes
'`@hiqiancheng`'; reference the ownedByMaintainer function, codeownerEntries, and
patternMatchesPath when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4a1300a7-c6a7-4a8e-a08b-47b3eab4461e
📒 Files selected for processing (1)
apps/desktop/tests/ci/release-codeowners-policy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Desktop E2E Smoke (Windows)
- GitHub Check: Rust Checks
- GitHub Check: Frontend Tests
- GitHub Check: Frontend Quality
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (rust)
🔇 Additional comments (5)
apps/desktop/tests/ci/release-codeowners-policy.test.ts (5)
1-12: LGTM!
14-24: LGTM!
26-34: LGTM!
36-38: LGTM!
48-62: LGTM!
hiqiancheng
changed the title
This was referenced Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RELEASE_PLEASE_TOKENinstead of falling back togithub.token, so release PR updates trigger required pull request checks.workflow_dispatchto the Release Please workflow so maintainers can manually refresh the release PR after token or workflow changes.Related issue or RFC
AI assistance disclosure
Testing evidence
TDD: yes. The new workflow policy tests failed first against the
github.tokenfallback and missingworkflow_dispatch, then passed after updatingrelease-please.yml.Risk notes
AgentService, runtime, MCP, or schema impact: none.RELEASE_PLEASE_TOKEN; missing token fails the Release Please workflow early with a clear message. Generated release metadata no longer requires code owner review solely for bot updates; release config, workflows, and release CI scripts remain owned.Screenshots or recordings
Not applicable; CI policy change only.
Checklist
[WIP]or similar title prefixes.AgentService, runtime, MCP, or schema boundaries, there is an accepted RFC.pnpm test:prfor this code PR, or this is a docs-only change.pnpm test:coverage:rustor relied on CI coverage evidence.pnpm test:e2elocally or documented why CI is the first valid proof.