Conversation
Reduce semver violations and improve API clarity: - Rename `close(err)` → `abort(err)` on all producer types to distinguish error termination from clean close - Replace `TrackProducer::finish()` with `append_finish()` and `insert_finish(sequence)` for explicit control over the final group - Rename `FrameProducer::write_chunk()` → `write()` for simplicity - Add doc comments to public producer/consumer APIs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kixelated
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request introduces semantic versioning enforcement into the project's CI/CD pipeline. Changes include: documentation of a branching strategy in CLAUDE.md; addition of cargo-semver-checks and release-plz tools to project dependencies; new Justfile targets for semver verification and version bumping; a new GitHub Actions workflow that runs semver checks on pull requests; and modification of the pull request trigger filter in the existing check workflow to apply to all pull requests rather than only those targeting the main branch. 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rs/moq-lite/src/model/track.rs (1)
173-181:⚠️ Potential issue | 🟠 Major
append_groupcurrently hard-closes immediately after anyinsert_finish.Line [175] blocks all appends when
finexists. Ifinsert_finishis set to a future boundary, this prevents valid in-range appends before that boundary.Suggested fix
pub fn append_group(&mut self) -> Result<GroupProducer> { let mut state = self.state.modify()?; - if state.fin.is_some() { - return Err(Error::Closed); - } - - let now = tokio::time::Instant::now(); - let sequence = state.max_sequence.map_or(0, |s| s + 1); + let sequence = state.max_sequence.map_or(0, |s| s + 1); + if let Some(fin) = state.fin + && sequence >= fin + { + return Err(Error::Closed); + } + let now = tokio::time::Instant::now(); let group = Group { sequence }.produce();Also applies to: 213-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rs/moq-lite/src/model/track.rs` around lines 173 - 181, The current append_group function rejects all appends if state.fin.is_some(), but it should only close appends once the fin boundary has passed; change the gating logic to compute now = tokio::time::Instant::now() and return Err(Error::Closed) only when state.fin.map_or(false, |f| now >= f) (i.e., fin exists and now is at-or-after the boundary) instead of state.fin.is_some(); apply the same fix to the other identical fin-check later in the file (the second occurrence near lines 213-214) so both places use the time comparison rather than a simple is_some() check.
🧹 Nitpick comments (1)
justfile (1)
371-374: Add a short rationale for--exclude libmoq.Line 374 introduces a permanent-looking exclusion without context. A one-line reason here will prevent accidental misuse later.
Suggested small diff
- cargo semver-checks check-release --workspace --exclude libmoq + # libmoq is intentionally excluded from crates.io semver checks. + cargo semver-checks check-release --workspace --exclude libmoq🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 371 - 374, Add a one-line rationale comment immediately above the cargo semver-checks invocation that explains why --exclude libmoq is needed (for example: libmoq is a non-published/internal crate or intentionally unstable), so future readers understand the permanent-looking exclusion; locate the line with "cargo semver-checks check-release --workspace --exclude libmoq" and insert the brief explanatory comment above it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dev.yml:
- Around line 18-23: Replace mutable action refs with immutable full commit
SHAs: change actions/checkout@v6 to actions/checkout@<commit-sha>,
DeterminateSystems/nix-installer-action@main,
DeterminateSystems/magic-nix-cache-action@main, and
DeterminateSystems/flake-checker-action@main to their respective full-length
commit SHAs, and add inline comments showing the original tag (e.g., // was `@v6`
or // was `@main`) for clarity; repeat the same SHA-pinning pattern in the other
workflows mentioned (main.yml, cachix.yml, release-js.yml, update-flake.yml).
In `@rs/moq-lite/src/model/track.rs`:
- Around line 204-209: append_finish (and similarly insert_finish) currently
overwrites fin unconditionally and can set an invalid final; change them to
validate and freeze the final boundary by: require that state.max_sequence is
Some(max) (return Err if not), require state.fin.is_none() (return Err if fin
already set), and then set state.fin = Some(max) (don’t accept sequence < max or
allow rewriting fin). Refer to the append_finish and insert_finish methods and
the state.max_sequence/state.fin fields when implementing these checks.
- Around line 74-79: The current logic returns Poll::Ready(None) as soon as
self.fin is set, which can prematurely terminate consumers; update the
Stream/iterator logic that currently checks self.fin (the block returning
Poll::Ready(None) and the similar checks in the methods using self.fin) to only
return Ready(None) when fin is set AND there are no pending groups and no future
groups can still be created (i.e., the internal queue/buffer is empty and
create_group would not accept any sequence < fin). Replace the early Ready(None)
returns with Poll::Pending (or equivalent) in the interim, and keep terminal
None only when self.fin.is_some() && queue.is_empty() && no allowed late
sequences remain so next_group/get_group won't stop early. Ensure the same
change is applied to the other occurrences (the checks around lines referencing
self.fin, create_group, next_group, get_group).
---
Outside diff comments:
In `@rs/moq-lite/src/model/track.rs`:
- Around line 173-181: The current append_group function rejects all appends if
state.fin.is_some(), but it should only close appends once the fin boundary has
passed; change the gating logic to compute now = tokio::time::Instant::now() and
return Err(Error::Closed) only when state.fin.map_or(false, |f| now >= f) (i.e.,
fin exists and now is at-or-after the boundary) instead of state.fin.is_some();
apply the same fix to the other identical fin-check later in the file (the
second occurrence near lines 213-214) so both places use the time comparison
rather than a simple is_some() check.
---
Nitpick comments:
In `@justfile`:
- Around line 371-374: Add a one-line rationale comment immediately above the
cargo semver-checks invocation that explains why --exclude libmoq is needed (for
example: libmoq is a non-published/internal crate or intentionally unstable), so
future readers understand the permanent-looking exclusion; locate the line with
"cargo semver-checks check-release --workspace --exclude libmoq" and insert the
brief explanatory comment above it.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/dev.yml.github/workflows/main.ymlCLAUDE.mdflake.nixjustfilers/hang/src/container/frame.rsrs/moq-lite/src/ietf/subscriber.rsrs/moq-lite/src/lite/subscriber.rsrs/moq-lite/src/model/broadcast.rsrs/moq-lite/src/model/frame.rsrs/moq-lite/src/model/group.rsrs/moq-lite/src/model/state.rsrs/moq-lite/src/model/track.rsrs/moq-mux/src/catalog.rsrs/moq-mux/src/import/fmp4.rsrs/moq-relay/src/cluster.rs
Comment on lines
+18
to
+23
| uses: actions/checkout@v6 | ||
|
|
||
| - uses: DeterminateSystems/nix-installer-action@main | ||
| - uses: DeterminateSystems/magic-nix-cache-action@main | ||
| - uses: DeterminateSystems/flake-checker-action@main | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "dev.yml" -o -name "dev.yaml" | head -20Repository: moq-dev/moq
Length of output: 82
🏁 Script executed:
cat -n .github/workflows/dev.ymlRepository: moq-dev/moq
Length of output: 936
🏁 Script executed:
find .github/workflows -name "*.yml" -o -name "*.yaml" | head -20Repository: moq-dev/moq
Length of output: 295
🏁 Script executed:
for file in .github/workflows/*.yml; do
echo "=== $file ==="
grep -n "uses:" "$file" | head -20
doneRepository: moq-dev/moq
Length of output: 2013
🌐 Web query:
GitHub Actions best practices pinning commit SHA vs version tags security
💡 Result:
Pinning strategy: commit SHA vs tags (security-focused)
Recommended default: pin to a full-length commit SHA
- GitHub’s security guidance says pinning to a full-length commit SHA is currently the only way to use an action as an immutable release, reducing the risk from repo compromise and tag retargeting (with the practical caveat to ensure the SHA is from the upstream repo, not a fork). [1]
- GitHub’s Well-Architected guidance likewise calls full SHAs the most reliable immutable reference, noting tag immutability can depend on repo settings. [2]
Example:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7(Adding the version comment is explicitly recommended to keep reviews/audits sane.) [2]
When a tag can be OK: only if it’s an “immutable release/tag”
- GitHub’s immutable releases lock the release assets and the associated Git tag so the tag can’t be moved/deleted after publication, which is meant to prevent supply-chain tampering of published releases. [3]
- If an action author is using immutable releases, pinning to that release tag can be an acceptable “human-friendly” immutable reference; otherwise, tags (including
v1,v4, etc.) may be intentionally moved to point at newer commits. [4]
Operational best practices
- Prefer “SHA that corresponds to a release tag” (not a random commit on
main) so you get immutability and clearer provenance; GitHub specifically recommends tying SHAs to tagged releases and labeling them. [2] - Use Dependabot version updates for GitHub Actions so SHA-pinned actions still get automated update PRs instead of silently floating to new code. [5]
- If you’re an org/enterprise admin, enforce SHA pinning via policy (GitHub added explicit enforcement so workflows fail if actions aren’t pinned to a full SHA). [6]
Practical rule of thumb
- Most repos: pin every third-party
uses:to a full SHA. - Only pin a tag if you’ve verified it’s backed by GitHub immutable releases for that action—and you’re confident it can’t be retargeted. [3][4]
Pin GitHub Actions to immutable commit SHAs instead of moving refs.
Lines 18 and 20-22 use mutable refs (@v6, @main). GitHub's security guidance confirms that pinning to full-length commit SHAs is the only way to achieve immutability and prevent tag retargeting attacks. Version tags like @v6 and @main can be moved by maintainers after release, creating supply-chain exposure.
Update each action to its commit SHA with a version comment for clarity:
Example:
- - uses: actions/checkout@v6
+ - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- - uses: DeterminateSystems/nix-installer-action@main
- - uses: DeterminateSystems/magic-nix-cache-action@main
- - uses: DeterminateSystems/flake-checker-action@main
+ - uses: DeterminateSystems/nix-installer-action@<commit-sha> # v0.x.x
+ - uses: DeterminateSystems/magic-nix-cache-action@<commit-sha> # v1.x.x
+ - uses: DeterminateSystems/flake-checker-action@<commit-sha> # v0.x.xNote: This pattern appears in multiple workflows; apply the same hardening to main.yml, cachix.yml, release-js.yml, and update-flake.yml.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/dev.yml around lines 18 - 23, Replace mutable action refs
with immutable full commit SHAs: change actions/checkout@v6 to
actions/checkout@<commit-sha>, DeterminateSystems/nix-installer-action@main,
DeterminateSystems/magic-nix-cache-action@main, and
DeterminateSystems/flake-checker-action@main to their respective full-length
commit SHAs, and add inline comments showing the original tag (e.g., // was `@v6`
or // was `@main`) for clarity; repeat the same SHA-pinning pattern in the other
workflows mentioned (main.yml, cachix.yml, release-js.yml, update-flake.yml).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
rs/moq-lite/src/model/track.rs (1)
214-226:insert_finish(sequence)accepts mismatched expected boundaries.Right now
sequence > max_sequencesucceeds but still freezes tomax_sequence. That can silently finalize earlier than the caller expected. Consider enforcing exact match (sequence == max) or renaming/docs to reflect “minimum expected” semantics.💡 Suggested fix (strict expected-boundary validation)
- if state.fin.is_some() || sequence < max { + if state.fin.is_some() || sequence != max { return Err(Error::Closed); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rs/moq-lite/src/model/track.rs` around lines 214 - 226, The insert_finish method currently allows sequence > max_sequence and still sets state.fin to max_sequence, which can silently finalize earlier than the caller expects; change the validation in insert_finish (function insert_finish, variables sequence, state.max_sequence, and state.fin) to require an exact match by returning Err(Error::Closed) unless sequence == max (i.e., replace the current condition that checks sequence < max with a strict equality check), then set state.fin = Some(max) as before and preserve the existing error path when fin is already set; also update the doc comment to state that insert_finish requires the caller to pass the exact final boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rs/moq-lite/src/model/track.rs`:
- Around line 647-648: The async test attribute needs to include start_paused =
true to follow the repo convention; update the #[tokio::test] annotation on the
next_group_finishes_without_waiting_for_gaps test (and the other new async test
referenced around lines 665-666) to #[tokio::test(start_paused = true)] so the
tests run with the tokio time paused and can use simulated time APIs.
- Line 173: The increment of state.max_sequence (sequence =
state.max_sequence.map_or(0, |s| s + 1)) can overflow; in the append_group path
replace the naive +1 with checked_add on state.max_sequence and, if checked_add
returns None (u64::MAX), return an appropriate error from append_group instead
of allowing overflow/wrap. Update any code that uses the resulting sequence
variable (and any state update of max_sequence) to handle the
Result/early-return error and keep the original semantics when checked_add
succeeds.
---
Nitpick comments:
In `@rs/moq-lite/src/model/track.rs`:
- Around line 214-226: The insert_finish method currently allows sequence >
max_sequence and still sets state.fin to max_sequence, which can silently
finalize earlier than the caller expects; change the validation in insert_finish
(function insert_finish, variables sequence, state.max_sequence, and state.fin)
to require an exact match by returning Err(Error::Closed) unless sequence == max
(i.e., replace the current condition that checks sequence < max with a strict
equality check), then set state.fin = Some(max) as before and preserve the
existing error path when fin is already set; also update the doc comment to
state that insert_finish requires the caller to pass the exact final boundary.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
justfilers/moq-lite/src/model/track.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- justfile
kixelated
changed the title
kixelated
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rs/moq-lite/src/model/broadcast.rs`:
- Around line 185-189: BroadcastDynamic::abort currently only calls
self.state.abort(err) and leaves entries in self.state.requests (so
queued/pending track producers remain alive); to fix, before closing broadcast
state iterate and cancel all pending requests: drain or take ownership of
self.state.requests and for each request call its cancellation method (e.g.,
request.abort(err.clone()) or send the error down its response channel) to
ensure their producers are closed, then call self.state.abort(err); update Error
usage to allow cloning or accept &Error if needed and ensure requests are
removed from state (use drain() or clear()) so no pending producers remain.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CLAUDE.mdflake.nixjustfilers/hang/src/container/consumer.rsrs/moq-lite/src/ietf/subscriber.rsrs/moq-lite/src/lite/subscriber.rsrs/moq-lite/src/model/broadcast.rsrs/moq-lite/src/model/frame.rsrs/moq-lite/src/model/track.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- flake.nix
- justfile
BroadcastDynamic::abort now cascades to child tracks and drains pending requests, matching BroadcastProducer::abort behavior. Previously pending TrackProducers were left alive and the Drop handler couldn't clean up since the state was already closed. Track finish now stores final_sequence as one past max_sequence (the total number of groups), allowing finish() on empty tracks. Renamed `fin` to `final_sequence` for clarity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rs/moq-lite/src/model/broadcast.rs (1)
348-349:⚠️ Potential issue | 🟡 MinorUse
start_paused = truefor this async tokio test.The
closedasync test uses#[tokio::test]instead of#[tokio::test(start_paused = true)].✅ Minimal fix
- #[tokio::test] + #[tokio::test(start_paused = true)] async fn closed() {As per coding guidelines
**/*.rs: Use async/await with#[tokio::test(start_paused = true)]for async tests in Rust to simulate time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rs/moq-lite/src/model/broadcast.rs` around lines 348 - 349, The async test function closed is missing the Tokio test option to pause time; update its attribute from #[tokio::test] to #[tokio::test(start_paused = true)] so the test uses a paused clock for simulated time; locate the closed function declaration in broadcast.rs and change the attribute above the async fn closed() accordingly.
🧹 Nitpick comments (1)
rs/moq-lite/src/model/broadcast.rs (1)
109-125: Consolidate duplicated abort cascade logic into one helper.
BroadcastProducer::abortandBroadcastDynamic::abortcurrently duplicate the same cascade logic. Centralizing this reduces drift risk when abort semantics evolve.♻️ Suggested refactor
+fn abort_broadcast_state(state: &mut State, err: Error) { + // Cascade abort to all child tracks. + for weak in state.tracks.values() { + weak.abort(err.clone()); + } + + // Abort pending dynamic requests. + for mut request in state.requests.drain(..) { + request.abort(err.clone()).ok(); + } + + state.abort(err); +} + impl BroadcastProducer { pub fn abort(&mut self, err: Error) -> Result<(), Error> { let mut state = self.state.modify()?; - - // Cascade abort to all child tracks. - for weak in state.tracks.values() { - weak.abort(err.clone()); - } - - // Abort any pending dynamic track requests. - for mut request in state.requests.drain(..) { - request.abort(err.clone()).ok(); - } - - state.abort(err); + abort_broadcast_state(&mut state, err); Ok(()) } } impl BroadcastDynamic { pub fn abort(&mut self, err: Error) -> Result<(), Error> { let mut state = self.state.modify()?; - - // Cascade abort to all child tracks. - for weak in state.tracks.values() { - weak.abort(err.clone()); - } - - // Abort any pending dynamic track requests. - for mut request in state.requests.drain(..) { - request.abort(err.clone()).ok(); - } - - state.abort(err); + abort_broadcast_state(&mut state, err); Ok(()) } }Also applies to: 185-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rs/moq-lite/src/model/broadcast.rs` around lines 109 - 125, Both BroadcastProducer::abort and BroadcastDynamic::abort duplicate the same cascade logic; extract that repeated sequence (iterating state.tracks to call weak.abort(...), draining state.requests and calling request.abort(...), then state.abort(...)) into a single helper method (e.g., BroadcastState::cascade_abort or Broadcast::cascade_abort) and call it from both BroadcastProducer::abort and BroadcastDynamic::abort instead of duplicating the code; ensure the helper takes &mut self and an Error parameter and uses the existing weak.abort(err.clone()) and request.abort(err.clone()) calls so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rs/moq-lite/src/model/broadcast.rs`:
- Around line 348-349: The async test function closed is missing the Tokio test
option to pause time; update its attribute from #[tokio::test] to
#[tokio::test(start_paused = true)] so the test uses a paused clock for
simulated time; locate the closed function declaration in broadcast.rs and
change the attribute above the async fn closed() accordingly.
---
Nitpick comments:
In `@rs/moq-lite/src/model/broadcast.rs`:
- Around line 109-125: Both BroadcastProducer::abort and BroadcastDynamic::abort
duplicate the same cascade logic; extract that repeated sequence (iterating
state.tracks to call weak.abort(...), draining state.requests and calling
request.abort(...), then state.abort(...)) into a single helper method (e.g.,
BroadcastState::cascade_abort or Broadcast::cascade_abort) and call it from both
BroadcastProducer::abort and BroadcastDynamic::abort instead of duplicating the
code; ensure the helper takes &mut self and an Error parameter and uses the
existing weak.abort(err.clone()) and request.abort(err.clone()) calls so
behavior remains identical.
Tests that use sleep/timeout now call tokio::time::pause() explicitly. Tests that don't need time simulation use plain #[tokio::test]. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/semver.yml (1)
11-14: Consider renaming the job for clarity.The job is named
checkwith display name "Check", which is identical to the job incheck.yml. This could cause confusion in the GitHub Actions UI when viewing PR status checks. Consider using a more descriptive name likesemveror "Semver Check".♻️ Proposed rename for clarity
jobs: - check: - name: Check + semver: + name: Semver Check runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/semver.yml around lines 11 - 14, The job in .github/workflows/semver.yml is named "check" with display name "Check", which duplicates another job and is confusing; rename the job identifier from check to semver (or similar unique id) and change the runs-on display name to "Semver Check" (update the job key `check` and the `name: Check` field) so the workflow uses a distinct job id and a clear UI label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/semver.yml:
- Around line 11-14: The job in .github/workflows/semver.yml is named "check"
with display name "Check", which duplicates another job and is confusing; rename
the job identifier from check to semver (or similar unique id) and change the
runs-on display name to "Semver Check" (update the job key `check` and the
`name: Check` field) so the workflow uses a distinct job id and a clear UI
label.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/check.yml.github/workflows/semver.ymlCLAUDE.mdjustfile
🚧 Files skipped from review as they are similar to previous changes (2)
- justfile
- CLAUDE.md
phoewass
pushed a commit
to phoewass/moq
that referenced
this pull request
Apr 3, 2026
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.
Merging into main will require semver backwards compatibility.