General: Restore LTI Moodle integration broken by Spring Boot 4 upgrade - #12769
Conversation
Spring Framework 7 removed UriComponentsBuilder.fromHttpUrl(String); the upstream uk.ac.ox.ctl:spring-security-lti13:0.3.4 still calls it inside OIDCInitiatingLoginRequestResolver.expandRedirectUri, so every Step 1 (third-party initiated login) request crashed with NoSuchMethodError once Artemis moved to Spring Boot 4 (#12381). Reported in #12739. Add Lti13InitiatingLoginRequestResolver as a one-line patched copy of the upstream resolver, placed in the same package so it can still throw the package-private InvalidClientRegistrationIdException and preserve the filter's 404/500 bucketing. Wire it into CustomLti13Configurer in place of the upstream class. The class header documents removal once an upstream Spring 7-compatible release ships (oxctl PR #60). Close the regression coverage gap: add Lti13InitiationIntegrationTest that exercises the full Spring Security filter chain end-to-end through the new resolver, extend Lti13LaunchIntegrationTest with symmetric deep-link redirect proxy tests and additional auth-login routing tests, and add a focused unit test for the resolver that catches the same bug without Docker. Closes #12739.
LTI: Restore Moodle integration broken by Spring Boot 4 upgradeGeneral: Restore LTI Moodle integration broken by Spring Boot 4 upgrade
|
@Claudia-Anthropica review |
There was a problem hiding this comment.
Pull request overview
Fixes a Spring Boot 4 / Spring Framework 7 runtime regression in the LTI 1.3 OIDC initiation flow (Step 1) by replacing the upstream resolver that calls the removed UriComponentsBuilder.fromHttpUrl(String), and adds end-to-end + unit test coverage to prevent future regressions in Moodle/LMS launches.
Changes:
- Introduce a Spring 7-compatible drop-in
Lti13InitiatingLoginRequestResolvershim (upstream-package copy withfromUriString). - Wire the shim into
CustomLti13Configurerso Step 1 no longer crashes withNoSuchMethodError. - Add Step 1 integration tests and expand Step 3 redirect-proxy / filter-wiring coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/de/tum/cit/aet/artemis/lti/config/CustomLti13Configurer.java |
Switches initiation filter wiring to use the patched resolver and documents the Spring 7 shims. |
src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java |
Adds the patched resolver implementation (copied from upstream with a one-line Spring 7 API replacement). |
src/test/java/de/tum/cit/aet/artemis/lti/Lti13InitiationIntegrationTest.java |
New server integration tests for LTI Step 1 initiation redirect and key error cases (404/400). |
src/test/java/de/tum/cit/aet/artemis/lti/Lti13LaunchIntegrationTest.java |
Extends coverage for /deep-link redirect proxy and additional /auth-login filter wiring scenarios. |
src/test/java/de/tum/cit/aet/artemis/lti/config/Lti13InitiatingLoginRequestResolverTest.java |
New unit tests validating the shim resolver behavior and guarding against reintroducing the removed Spring API call. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@krusche got it — starting the review right away. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds Lti13InitiatingLoginRequestResolver (Spring 7-compatible), wires it into CustomLti13Configurer, and adds unit and integration tests for initiation, deep-link, and auth-login flows; the resolver validates initiation parameters, expands redirect URIs, and builds OAuth2AuthorizationRequest objects. ChangesLTI 1.3 Initiation Resolver Implementation
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Resolver as Lti13InitiatingLoginRequestResolver
participant RegResolver as OIDCInitiationRegistrationResolver
participant ClientRepo as ClientRegistrationRepository
participant Platform as PlatformAuthorizationEndpoint
Browser->>Resolver: GET /initiate-login?iss=...&login_hint=...&target_link_uri=...
Resolver->>RegResolver: resolveClientRegistrationId(request)
RegResolver-->>Resolver: registrationId
Resolver->>ClientRepo: findByRegistrationId(registrationId)
ClientRepo-->>Resolver: ClientRegistration
Resolver->>Resolver: validate params, expand redirect_uri, generate state/nonce
Resolver->>Platform: redirect to authorizationUri with OAuth2 params (response_type=id_token, response_mode=form_post, login_hint, prompt=none, state, nonce, registration_id)
Platform-->>Browser: 302 Location -> authorizationUri?...
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/de/tum/cit/aet/artemis/lti/Lti13InitiationIntegrationTest.java (1)
46-47: ⚡ Quick winUse fixed registration IDs in tests instead of random UUIDs.
These tests can stay isolated with deterministic IDs, which improves reproducibility and aligns with the test-data guideline.
🔧 Suggested fix
- private static final String AUTH_URI = "https://platform.example.com/mod/lti/auth.php"; + private static final String AUTH_URI = "https://platform.example.com/mod/lti/auth.php"; + private static final String REGISTRATION_ID_REDIRECT = "test-platform-redirect"; + private static final String REGISTRATION_ID_MISSING_ISS = "test-platform-missing-iss"; ... - String registrationId = "test-platform-" + UUID.randomUUID(); + String registrationId = REGISTRATION_ID_REDIRECT; ... - String registrationId = "test-platform-" + UUID.randomUUID(); + String registrationId = REGISTRATION_ID_MISSING_ISS;As per coding guidelines: "
src/test/java/**/*.java: ... fixed_data: true".Also applies to: 77-78
🤖 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 `@src/test/java/de/tum/cit/aet/artemis/lti/Lti13InitiationIntegrationTest.java` around lines 46 - 47, Replace non-deterministic registration IDs that use UUID.randomUUID() with fixed, deterministic strings to comply with test-data guidelines; change the variable registrationId assignment (and the other occurrence using UUID.randomUUID()) to use a stable literal such as "test-platform-1" (and a distinct fixed value for the second case, e.g., "test-platform-2") and keep the subsequent savePlatform(registrationId) calls unchanged so tests remain isolated but reproducible.
🤖 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
`@src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java`:
- Around line 87-100: The current Lti13InitiatingLoginRequestResolver code only
checks for null parameters (iss, login_hint, target_link_uri) and therefore
allows empty or whitespace-only values; update the validation in the method that
reads request.getParameter(...) so that after fetching each parameter (iss,
login_hint, target_link_uri) you normalize by trimming and reject when the
trimmed value is empty (e.g., treat null or trimmed.isEmpty() as invalid) by
throwing the same InvalidInitiationRequestException with the existing messages.
---
Nitpick comments:
In
`@src/test/java/de/tum/cit/aet/artemis/lti/Lti13InitiationIntegrationTest.java`:
- Around line 46-47: Replace non-deterministic registration IDs that use
UUID.randomUUID() with fixed, deterministic strings to comply with test-data
guidelines; change the variable registrationId assignment (and the other
occurrence using UUID.randomUUID()) to use a stable literal such as
"test-platform-1" (and a distinct fixed value for the second case, e.g.,
"test-platform-2") and keep the subsequent savePlatform(registrationId) calls
unchanged so tests remain isolated but reproducible.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 59323aa2-0dac-4032-aa55-c259261b5f88
📒 Files selected for processing (5)
src/main/java/de/tum/cit/aet/artemis/lti/config/CustomLti13Configurer.javasrc/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.javasrc/test/java/de/tum/cit/aet/artemis/lti/Lti13InitiationIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/lti/Lti13LaunchIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/lti/config/Lti13InitiatingLoginRequestResolverTest.java
- Add explicit Apache 2.0 attribution header to Lti13InitiatingLoginRequestResolver matching the JHipster pattern used in CachingHttpHeadersFilter. - Qualify ambiguous PR #60 reference as oxctl/spring-security-lti13#60 with full upstream URL in CustomLti13Configurer Javadoc.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
End-to-End Test Results
Test Strategy: Two-phase execution
|
…uri) CodeRabbit follow-up review on Lti13InitiatingLoginRequestResolver. The upstream class only rejects null parameters; blank/whitespace values slip through and either disappear silently or propagate as empty additional parameters to the platform. LTI 1.3 spec requires these to be non-empty strings. Add isBlank() checks alongside the existing null checks. Document this as a second behavioural divergence from upstream in the resolver header, and add three unit tests (blank iss/login_hint/target_link_uri) plus one integration test (blank iss returns 400 through the full filter chain) so the new behaviour has explicit regression coverage.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java (1)
101-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject blank required initiation parameters (not only null).
iss,login_hint, andtarget_link_uricurrently accept whitespace-only values. Please trim and reject blank values with the same exception messages to keep initiation validation strict.🔧 Suggested patch
- String iss = request.getParameter("iss"); - if (iss == null) { + String iss = request.getParameter("iss"); + if (iss == null || iss.isBlank()) { throw new InvalidInitiationRequestException("Required parameter iss was not supplied."); } - String loginHint = request.getParameter("login_hint"); - if (loginHint == null) { + String loginHint = request.getParameter("login_hint"); + if (loginHint == null || loginHint.isBlank()) { throw new InvalidInitiationRequestException("Required parameter login_hint was not supplied."); } - String targetLinkUri = request.getParameter("target_link_uri"); - if (targetLinkUri == null) { + String targetLinkUri = request.getParameter("target_link_uri"); + if (targetLinkUri == null || targetLinkUri.isBlank()) { throw new InvalidInitiationRequestException("Required parameter target_link_uri was not supplied"); }Based on learnings: "Always validate and sanitize inputs in server-side Java code... Treat user input as potentially unsafe and codify input validation, normalization, and error handling as part of the core request handling and service layers."
🤖 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 `@src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java` around lines 101 - 114, The null-only checks in Lti13InitiatingLoginRequestResolver accept whitespace-only values: update the validation for iss, login_hint, and target_link_uri to trim and reject blank values (e.g., if (iss == null || iss.trim().isEmpty()) ...) and throw the same InvalidInitiationRequestException messages; apply the same trim+empty check for loginHint and targetLinkUri so whitespace-only inputs are treated as missing.
🤖 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
`@src/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java`:
- Around line 101-114: The null-only checks in
Lti13InitiatingLoginRequestResolver accept whitespace-only values: update the
validation for iss, login_hint, and target_link_uri to trim and reject blank
values (e.g., if (iss == null || iss.trim().isEmpty()) ...) and throw the same
InvalidInitiationRequestException messages; apply the same trim+empty check for
loginHint and targetLinkUri so whitespace-only inputs are treated as missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2805afd2-ab24-4d90-940f-fcdfa0526276
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/lti/config/CustomLti13Configurer.javasrc/main/java/uk/ac/ox/ctl/lti13/security/oauth2/client/lti/web/Lti13InitiatingLoginRequestResolver.java
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche Clean, surgical fix for the Spring 7 LTI regression. I diffed the new Lti13InitiatingLoginRequestResolver against the disassembled upstream OIDCInitiatingLoginRequestResolver 0.3.4 — it's a faithful copy with exactly the one fromHttpUrl→fromUriString change (upstream already used fromUriString for the redirect URI), so it matches upstream PR #60 and the package placement for the package-private exceptions / 404-vs-500 bucketing is the right call. The added Step 1 integration coverage plus the blank-param hardening are nicely done, and all 24 LTI tests pass locally for me. The earlier red server-style was the develop-wide Modeling/TextRepositoryArchitectureTest::enforceUsageOfTestRepository failure (from #12570), now resolved by the develop merge — arch tests pass green here. Nice work.
- Document second behavioural divergence from upstream (blank-rejection) in the resolver class-level Javadoc so it matches the file-header comment; reword the inline comment that still claimed only one change exists. - Split copyright header cleanly: upstream notice attributes original code to University of Oxford; Artemis maintainers hold copyright on the modifications only, both under Apache 2.0. - Soften 'drop-in replacement' Javadoc claim and explicitly call out that the upstream constructor taking a String authorizationRequestBaseUri is omitted because Artemis always supplies Lti13PathRegistrationResolver. - Fix misleading Javadoc on resolveDoesNotThrowNoSuchMethodErrorFromRemovedSpringApi: the unit test cannot detect a CustomLti13Configurer wiring revert; clarify that the wiring guard is Lti13InitiationIntegrationTest's responsibility. - Strengthen initiateLoginWithUnknownRegistrationReturnsNotFound by saving a real platform under a different registrationId first, so the 404 proves the lookup actively rejects unknown IDs (not merely an empty repository). - Add integration tests for blank login_hint and blank target_link_uri (the Artemis-specific tightening most likely to silently regress on a future upstream re-sync). - Trim the Step 3b preamble in Lti13LaunchIntegrationTest from a 10-line wall to a 4-line note.
|
CI status note All checks that exercise this PR's code paths are green: server-tests, server-style, bean-instantiation-check, client-compilation (after one retry of an
None of these tests touch LTI, the Spring Security filter chain, the new resolver, or any other code path modified in this PR. The same Phase 2 job is also failing on Local LTI suite is green: 172 tests, 171 pass, 1 unrelated pre-existing skip, 54.1 s. Local integration tests ( |
|
@Claudia-Anthropica review |
|
@krusche got it — starting the review right away. |
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche Re-reviewed after the deep self-review pass. The shim is still a faithful copy of upstream OIDCInitiatingLoginRequestResolver with just the fromHttpUrl→fromUriString swap (matching upstream PR #60), and the same-package placement remains the right call so it can throw the package-private exceptions and keep the filter's 404-vs-500 bucketing intact. The blank-param hardening for iss/login_hint/target_link_uri is well documented and mirrored at both unit and integration level, and the new Step 1 + deep-link coverage closes the gap that let this regression ship. Copilot's license header / PR #60 qualifier and CodeRabbit's blank-param feedback are both addressed. server-tests and the arch tests are green here; the only red CI check is an unrelated flaky C-submission E2E test. Nice work.
- Restore upstream's expandRedirectUri comment block describing the
supported URI variables; the file header still claims only two
intentional modifications and missing comments would silently make
that a third.
- Fully qualify {@link Lti13PathRegistrationResolver} in the shim's
class Javadoc — the unqualified link did not resolve from the
uk.ac.ox.ctl... package and triggered a Javadoc warning.
- Add @AfterEach cleanupPlatforms() to Lti13InitiationIntegrationTest
that deletes inserted LtiPlatformConfiguration rows. The shared
integration base only resets Mockito spies (no DB rollback), so
without this cleanup the UUID-keyed rows leak into any LTI test
class running afterwards in the same JVM and can produce
order-dependent failures in LtiIntegrationTest /
OAuth2JWKSIntegrationTest.
0b6ca77
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche The latest pass is just polish on an already-solid fix: fully-qualifying the Lti13PathRegistrationResolver Javadoc link, documenting the redirect-URI template variables, and adding the @AfterEach that wipes the UUID-keyed LtiPlatformConfiguration rows so they don't leak into later LTI test classes. The shim is still the faithful upstream copy with the fromHttpUrl→fromUriString swap plus the blank-param hardening, and the cleanup is sound since this class only persists standalone platforms. server-style (incl. arch tests) and the WAR build are green; server-tests is still running but the only behavioural delta here is that test cleanup. Nice work.
This @AfterEach was introduced alongside the integration test in #12769 to prevent platform-row leakage, but other LTI tests (LtiIntegrationTest.getAllConfiguredLtiPlatformsAsAdmin and updateLtiPlatformConfigurationAsAdmin) hardcode platform id 1 and implicitly rely on persistent rows — the deleteAll() resets the sequence and causes optimistic-locking failures on subsequent inserts with the same id. The same fix is the entire content of #12772, which now becomes redundant once this PR lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Spring Framework 7 removed
UriComponentsBuilder.fromHttpUrl(String). The upstreamuk.ac.ox.ctl:spring-security-lti13:0.3.4still calls it during the very first step of an LTI 1.3 launch, so every Moodle (or any other LMS) login crashed withNoSuchMethodErrorafter the Spring Boot 4 upgrade. This PR ships a one-line patched copy of the upstream resolver — placed in the same package so it preserves the existing 404/500 semantics — and closes the regression coverage gap with an end-to-end server integration test for Step 1 plus broader LTI step coverage.Checklist
General
Server
Motivation and Context
Fixes #12739 ("LTI/Moodle Integration broken").
Reported scenario: clicking the Artemis link inside a Moodle course returns "Ihre Anfrage kann nicht bearbeitet werden."; the Artemis server log shows
UriComponentsBuilder.fromHttpUrl(String)was deprecated in Spring 6 and removed in Spring 7.spring-security-lti13:0.3.4was compiled against Spring 6.1.12 and still references the method; once Artemis upgraded to Spring Boot 4 / Spring Framework 7 (#12381), every Step 1 of the LTI 1.3 OIDC third-party initiated login flow crashed.Rolling back Spring Boot 4 is not an option. An upstream PR fixing this is open (oxctl/spring-security-lti13#60) but unreleased.
Description
Library audit first. Exhaustive grep of
spring-security-lti13:0.3.4confirmed only two Spring 6→7 removed-API call sites:UriComponentsBuilder.fromHttpUrlinOIDCInitiatingLoginRequestResolver:173.AntPathRequestMatcherinPathOIDCInitiationRegistrationResolver— already shimmed inDevelopment: Upgrade to Spring Boot 4, Spring Framework 7, and Spring AI 2.0.0-M4 #12381 byLti13PathRegistrationResolver.No other removed APIs (
NimbusJwtDecoder,JwtDecoder,OAuth2AuthorizationRequest,AbstractHttpConfigurer,OncePerRequestFilter,HttpSecurity#with, etc. all still exist in Spring 7).RestTemplateis deprecated but functional, and Artemis does not use the upstreamTokenRetriever/NamesRoleServiceanyway — it has its ownLti13TokenRetriever. Upstream PR #60 also fixes only these same two sites; the remaining diff entries (pom.xml version bumps, three test-side migrations) have no Artemis impact.Fix. Mirror the existing
Lti13PathRegistrationResolverworkaround:Lti13InitiatingLoginRequestResolver— a copy ofOIDCInitiatingLoginRequestResolverwith two intentional modifications: (a)fromHttpUrl→fromUriString(matches upstream PRoxctl/spring-security-lti13#60), (b) reject blank/whitespaceiss/login_hint/target_link_uriin addition to nulls (LTI 1.3 spec compliance — Artemis-specific tightening on top of upstream). Placed in the upstream package so it can still throw the package-privateInvalidClientRegistrationIdExceptionand the filter's 404 (unknown registration) / 500 (other failures) bucketing keeps working unchanged.CustomLti13Configurer.configureInitiationFilternow instantiates the patched resolver instead of the upstream one. Removal path documented in the shim's Javadoc: once the upstream library releases a Spring 7-compatible version, delete the shim and revert one import line; if modification (b) is to be preserved, contribute it upstream first.Why this wasn't caught in #12381. Three independent reasons lined up: (1) the bug is a runtime linkage error, not a compile error — the library jar is precompiled against Spring 6.1.12, and the JVM only resolves the static method at first invocation, so application startup and health endpoints all pass; (2) the pre-existing
Lti13LaunchIntegrationTestcovers Step 3 (auth-callback) only — its header comment explicitly says "Step 1 ... does not require additional testing here"; (3) the Spring Boot 4 upgrade did notice the same library was breaking — and shimmedAntPathRequestMatcher— but stopped at the first symptom instead of asking "is this library Spring-7-compatible at all?"Closing the coverage gap. Added a comprehensive server integration test suite:
Lti13InitiationIntegrationTest(new, 6 tests) — exercises the full Spring Security filter chain end-to-end for Step 1: happy path with a persistedLtiPlatformConfigurationasserts 302 to platform'sauthorization_uriwith all required OIDC parameters; unknown registration returns 404 (with a real platform under a differentregistrationIdsaved first, so the assertion proves active rejection rather than an empty repository); missingissreturns 400; blankiss/login_hint/target_link_urieach return 400 to lock in the spec-compliance tightening.Lti13LaunchIntegrationTestextended (+6 tests) — symmetric/deep-linkredirect-proxy coverage (happy + 3 negative paths) and additional/auth-loginfilter routing tests (POST variant forform_post, anonymous-user variant).Lti13InitiatingLoginRequestResolverTest(new, 7 unit tests) — focused unit tests for the resolver, including an explicitNoSuchMethodErrorregression guard for re-introduction offromHttpUrlinside the shim itself and dedicated tests for each blank-rejection case. Runs in ~1 s without Docker.Total: +19 LTI tests. Full LTI suite is 172 tests / 171 pass / 1 unrelated pre-existing skip, 54.1 s.
Steps for Testing
Prerequisites:
develop).authorization_uri,client_id,token_uri, andjwk_set_uri.develophead), click the Artemis tool link inside a Moodle course. The browser shows "Ihre Anfrage kann nicht bearbeitet werden." and the server log contains theNoSuchMethodErrorstack trace pointing atOIDCInitiatingLoginRequestResolver:173.Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Summary by CodeRabbit
New Features
Bug Fixes
Tests