chore: add breaking API change detection#53
Conversation
📝 WalkthroughWalkthroughAdds two OpenAPI comparison steps to the upstream sync GitHub Actions workflow: one to detect breaking changes and one to generate a changelog. The PR body is updated to include a breaking-changes indicator and a collapsible changelog section populated from the changelog step output. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as "GitHub Actions\n(upstream-sync.yml)"
participant OASDiffBreaking as "oasdiff-action/breaking"
participant OASDiffChangelog as "oasdiff-action/changelog"
participant GitHubAPI as "GitHub API\n(create PR)"
participant Repo as "Repository\n(specs & tags)"
Workflow->>Repo: read `stored_tag` and `tag_to_use` specs
Workflow->>OASDiffBreaking: run breaking check (base=stored_tag, rev=tag_to_use)
OASDiffBreaking-->>Workflow: outcome (ok|breaking) [continues on error]
Workflow->>OASDiffChangelog: run changelog (base=stored_tag, rev=tag_to_use)
OASDiffChangelog-->>Workflow: outputs.changelog (markdown)
Workflow->>GitHubAPI: create/update PR with body including breaking indicator and collapsible changelog
GitHubAPI-->>Repo: PR created/updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
timonrieger
changed the title
![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
🤖 Fix all issues with AI agents
In @.github/workflows/upstream-sync.yml:
- Line 176: The current check uses steps.oasdiff_breaking.outcome == 'failure'
which can be true for non-breaking failures; change the condition to explicitly
verify the step's conclusion and/or its breaking output from the
oasdiff_breaking step (for example require steps.oasdiff_breaking.conclusion ==
'failure' AND steps.oasdiff_breaking.outputs.breaking == 'true', or just check
steps.oasdiff_breaking.outputs.breaking == 'true' if the action sets that),
and/or add a PR-body note when the check is true to prompt manual verification;
update the workflow expression referencing the oasdiff_breaking step
accordingly.
🧹 Nitpick comments (2)
.github/workflows/upstream-sync.yml (2)
87-87: Pin action to a specific SHA (same concern as above).Same security recommendation applies here. Additionally, consider adding
continue-on-error: truefor resilience—if the changelog generation fails (e.g., network issues fetching the OpenAPI specs), the PR creation will be blocked entirely. You may want the sync PR to still be created with a note that changelog generation failed.
78-78: Pin oasdiff-action to a specific SHA for supply chain security.Both usages of
oasdiff-action(breakingat line 78 andchangelogat line 87) are referenced as@main, which is inconsistent with other actions in this workflow that are pinned to specific commit SHAs (e.g.,actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8). Using@mainexposes the workflow to potential supply chain attacks if the upstream repository is compromised.
| Automatically generated sync to Immich ${{ steps.tag_to_use.outputs.tag }}. | ||
| Automatically generated client for [Immich ${{ steps.tag_to_use.outputs.tag }}](https://github.com/immich-app/immich/releases/tag/${{ steps.tag_to_use.outputs.tag }}). | ||
|
|
||
| **Breaking Changes?:** ${{ steps.oasdiff_breaking.outcome == 'failure' }} |
There was a problem hiding this comment.
Potential false positive if the breaking check fails for non-breaking reasons.
The expression steps.oasdiff_breaking.outcome == 'failure' will evaluate to true if the step fails for any reason (network timeout, malformed spec, etc.), not just when actual breaking changes are detected. This could mislead reviewers.
Consider checking the step's conclusion alongside its outputs, or documenting in the PR body that a true value warrants manual verification.
🤖 Prompt for AI Agents
In @.github/workflows/upstream-sync.yml at line 176, The current check uses
steps.oasdiff_breaking.outcome == 'failure' which can be true for non-breaking
failures; change the condition to explicitly verify the step's conclusion and/or
its breaking output from the oasdiff_breaking step (for example require
steps.oasdiff_breaking.conclusion == 'failure' AND
steps.oasdiff_breaking.outputs.breaking == 'true', or just check
steps.oasdiff_breaking.outputs.breaking == 'true' if the action sets that),
and/or add a PR-body note when the check is true to prompt manual verification;
update the workflow expression referencing the oasdiff_breaking step
accordingly.
1 participant
Summary by CodeRabbit