btsk-115: fix all error

[Hyeonjun0527]

PR 설명

문제집삭제, 소스삭제안되는문제해결. jpa 캐시 관련 문제 해결
패스때문에 로그아웃 쿠키 삭제 안되는 문제 해결
비동기 방식으로 문제집 요청을 보내서 문제집 10개 생성할시 11개 생성되는 문제 멱등성키로 해결

Summary by CodeRabbit

Release Notes

• New Features

  • Added HTTPS/SSL support for secure connections
  • Introduced real-time notifications via Server-Sent Events (SSE)
  • Implemented idempotency protection—repeated question set submissions now prevent duplicates

• Bug Fixes

  • Corrected learning progress percentage calculation
  • Enhanced handling of concurrent updates with automatic retry logic

• Infrastructure

  • Added reverse proxy with SSL/TLS configuration
  • Improved service health monitoring and error handling for invalid operations

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

Important

Review skipped

More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.

192 files out of 299 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Added nginx SSL reverse proxy service with healthchecks, implemented idempotency framework using Spring AOP, enhanced repository methods for eager loading, migrated Gemini client to streaming API, updated learning progress calculation, and disabled auto-generated schema management.

Changes

Cohort / File(s)	Summary
Infrastructure: Docker & Nginx compose.yaml, nginx/certs/localhost.crt, nginx/certs/localhost.key, nginx/conf.d/default.conf	Added nginx service with SSL/TLS configuration (port 443), certificate files, and reverse proxy rules. Configured server block with SSE streaming support and websocket upgrade. Removed ports from pullit-local-api. Added environment variables (Gemini API key, AWS credentials, etc.) to pullit-local-api and pullit-local-worker. Added healthchecks to mariadb, redis, and rabbitmq with updated timeouts and test commands.
Idempotency Framework (New Feature) src/main/java/kr/it/pullit/shared/idempotency/Idempotent.java, src/main/java/kr/it/pullit/shared/idempotency/IdempotencyAspect.java, src/main/java/kr/it/pullit/shared/idempotency/IdempotencyExceptionHandler.java, src/main/java/kr/it/pullit/shared/idempotency/DuplicateRequestException.java	Introduced new idempotency annotation and AOP-based enforcement mechanism. Aspect intercepts annotated methods, manages Redis-based locks keyed on HTTP method/URI/user, prevents duplicate execution, and throws DuplicateRequestException on conflicts. Handler returns 409 Conflict response.
Repository Layer Enhancements src/main/java/kr/it/pullit/modules/questionset/repository/QuestionSetRepository.java, src/main/java/kr/it/pullit/modules/questionset/repository/QuestionSetRepositoryImpl.java, src/main/java/kr/it/pullit/modules/questionset/repository/adapter/jpa/QuestionSetJpaRepository.java	Added findByIdWithQuestions(Long id) method across repository layers to eager-load questions via LEFT JOIN FETCH, enabling single-query retrieval of QuestionSet with associated questions.
Service Layer Updates src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java, src/main/java/kr/it/pullit/modules/questionset/service/QuestionService.java, src/main/java/kr/it/pullit/modules/learningsource/source/service/SourceService.java	QuestionSetService: added @Retryable annotations for optimistic lock retry (3 attempts, 200ms backoff) and switched to findByIdWithQuestions. QuestionService: standardized on DEFAULT_MODEL_NAME, simplified LLM call signature, removed validateQuestionExists. SourceService: added pre-delete cleanup to detach source from associated question sets.
Controller & Web Configuration src/main/java/kr/it/pullit/modules/questionset/web/QuestionSetController.java, src/main/java/kr/it/pullit/platform/web/cookie/CookieManager.java	QuestionSetController: added @Idempotent annotation to createQuestionSet endpoint. CookieManager: introduced determinePathForCookie helper to route refresh_token cookies to REFRESH_TOKEN_COOKIE_PATH while others use "/".
Platform Security & Error Handling src/main/java/kr/it/pullit/platform/security/config/SecurityConfig.java, src/main/java/kr/it/pullit/shared/error/CommonErrorCode.java	SecurityConfig: added @Profile("!worker") to disable bean in worker profile. CommonErrorCode: added message field and three new error constants (UNSUPPORTED_HTTP_METHOD, UNHANDLED_EXCEPTION, DUPLICATE_REQUEST).
Domain Model & Projection src/main/java/kr/it/pullit/modules/projection/learnstats/domain/LearnStats.java, src/main/java/kr/it/pullit/modules/projection/outbox/domain/OutboxEvent.java	LearnStats: changed calculateLearningProgress to use totalCorrectQuestionCount instead of totalSolvedQuestionCount. OutboxEvent: changed payload columnDefinition from "json" to "longtext".
Client Integration src/main/java/kr/it/pullit/modules/questionset/client/GeminiClient.java	Migrated from single-response to streaming API: replaced generateContent with generateContentStream, aggregated partial responses via aggregateStreamResponse, updated parseResponse to accept aggregated String, enhanced error handling per-stream.
Configuration src/main/resources/application.yml, src/main/resources/data.sql	application.yml: disabled Hibernate DDL auto-generation (generate-ddl: false, ddl-auto: none) and added forward-headers-strategy. data.sql: removed full data dump.
Tests src/test/java/kr/it/pullit/modules/questionset/client/GeminiClientTest.java, src/test/java/kr/it/pullit/platform/web/cookie/CookieManagerTest.java	GeminiClientTest: updated mocks to use generateContentStream and ResponseStream, added streaming response iteration and chunked JSON parsing. CookieManagerTest: added expiresRefreshTokenCookieWithCorrectPath test for refresh token path handling.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant IdempotencyAspect
    participant Redis
    participant Handler
    participant Exception Handler

    Client->>IdempotencyAspect: Request with Idempotency-Key header
    IdempotencyAspect->>IdempotencyAspect: Extract idempotency key<br/>(header + method + URI + user)
    IdempotencyAspect->>IdempotencyAspect: Build Redis key (hashed)
    IdempotencyAspect->>Redis: Check if key exists
    alt Key exists
        Redis-->>IdempotencyAspect: Key found
        IdempotencyAspect->>Exception Handler: Throw DuplicateRequestException
        Exception Handler-->>Client: 409 Conflict
    else Key does not exist
        Redis-->>IdempotencyAspect: Key not found
        IdempotencyAspect->>IdempotencyAspect: Attempt to acquire lock
        rect rgb(200, 220, 255)
            Note over IdempotencyAspect,Handler: Execute method
            IdempotencyAspect->>Handler: proceed()
            Handler-->>IdempotencyAspect: Response
        end
        IdempotencyAspect->>Redis: Set key = "DONE"<br/>with TTL
        IdempotencyAspect-->>Client: Response
    end

Loading

sequenceDiagram
    participant Client
    participant GeminiClient
    participant GeminiAPI
    participant Parser

    Client->>GeminiClient: callGeminiApi(request)
    GeminiClient->>GeminiAPI: generateContentStream(request)
    GeminiAPI-->>GeminiClient: ResponseStream<GenerateContentResponse>
    
    rect rgb(220, 240, 220)
        Note over GeminiClient: aggregateStreamResponse
        loop For each response in stream
            GeminiAPI-->>GeminiClient: GenerateContentResponse (chunk)
            GeminiClient->>GeminiClient: Validate finish reason<br/>(STOP or FINISH_REASON_UNSPECIFIED)
            GeminiClient->>GeminiClient: Concatenate text field
        end
    end
    
    GeminiClient->>Parser: parseResponse(aggregatedString)
    Parser-->>GeminiClient: Parsed object
    GeminiClient-->>Client: Result

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

• Idempotency aspect: New AOP implementation with Redis coordination logic, requires careful review of lock semantics and edge cases (missing context, header validation, concurrent access patterns)
• Streaming GeminiClient refactor: Significant control flow change from single response to stream aggregation with error handling per-chunk; requires verification of aggregation correctness and exception semantics
• Retry logic in QuestionSetService: OptimisticLockingFailureException retry behavior needs validation against concurrency expectations
• Repository eager-loading pattern: Verify LEFT JOIN FETCH query correctness and impact on N+1 query patterns
• Cross-layer consistency: Verify idempotency annotation placement, @Profile condition on SecurityConfig, and environment variable routing in docker-compose

Poem

🐰 With nginx guarding the gateway so tight,
And idempotency keys shining bright,
Streaming responses flow like a breeze,
Retry logic puts databases at ease,
Our warren of code springs forth with delight! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The PR title 'btsk-115: fix all error' is vague and overly generic. It fails to clearly convey what the main changes are, using non-descriptive language ('fix all error') rather than highlighting the specific problems addressed.	Consider using a more descriptive title that reflects the main changes, such as 'btsk-115: Add idempotency, fix cache/cookie issues' or specify the primary concern. The current title obscures the actual changes made.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/kr/it/pullit/modules/projection/learnstats/domain/LearnStats.java (1)

63-68: Add data validation constraints to prevent integrity violations from this breaking semantic change.

The breaking change from totalSolvedQuestionCount (attempted) to totalCorrectQuestionCount (correct answers) in the calculateLearningProgress() method is confirmed—this shifts the metric from measuring engagement to mastery. While semantically meaningful, this change introduced data integrity risks.

Critical issues:

1. No constraints enforce totalCorrectQuestionCount ≤ totalSolvedQuestionCount — if business logic elsewhere is buggy, the system could record more correct answers than attempted questions.
2. Missing documentation — no migration notes for this breaking semantic change are present in the codebase.
3. Insufficient test coverage — the test at QuestionSetWithStatsFacadeImplTest.java:99 sets values but doesn't validate constraints.

Required fixes:

Add validation in increaseCorrectQuestionCount():

public void increaseCorrectQuestionCount(long correctCount) {
  if (correctCount > 0) {
    if (this.totalCorrectQuestionCount + correctCount > this.totalSolvedQuestionCount) {
      throw new IllegalStateException("Correct count cannot exceed solved count");
    }
    this.totalCorrectQuestionCount += correctCount;
  }
}

Document the breaking change in migration notes, and add test cases verifying the constraint.

🧹 Nitpick comments (7)

src/test/java/kr/it/pullit/modules/questionset/client/GeminiClientTest.java (1)

60-65: Suggest adding a helper method to reduce duplication.

The mock setup for ResponseStream (mocking both iterator() and spliterator()) is repeated in lines 86-93, 108-115, and here. Consider extracting this into a helper method to improve maintainability.

Add this helper method to the test class:

private ResponseStream<GenerateContentResponse> mockResponseStream(List<GenerateContentResponse> responses) {
  ResponseStream<GenerateContentResponse> mockStream = Mockito.mock(ResponseStream.class);
  when(mockStream.iterator()).thenReturn(responses.iterator());
  when(mockStream.spliterator()).thenReturn(responses.spliterator());
  return mockStream;
}

Then simplify the test setup:

-    List<GenerateContentResponse> responses =
-        List.of(successResponse(responseJsonChunk1), successResponse(responseJsonChunk2));
-
-    ResponseStream<GenerateContentResponse> mockResponseStream = Mockito.mock(ResponseStream.class);
-    when(mockResponseStream.iterator()).thenReturn(responses.iterator());
-    when(mockResponseStream.spliterator()).thenReturn(responses.spliterator());
+    List<GenerateContentResponse> responses =
+        List.of(successResponse(responseJsonChunk1), successResponse(responseJsonChunk2));
+    ResponseStream<GenerateContentResponse> mockResponseStream = mockResponseStream(responses);

nginx/conf.d/default.conf (1)

1-10: LGTM! SSL configuration is appropriate for local development.

The TLS 1.2/1.3 protocols and cipher suite are reasonable for development. For production deployment, consider using a more specific cipher suite following Mozilla SSL Configuration Generator recommendations.

compose.yaml (2)

2-14: Consider adding a healthcheck for the nginx service.

The nginx service configuration is correct, but adding a healthcheck would improve orchestration reliability.

Apply this diff to add a healthcheck:

     depends_on:
       - pullit-local-api
+    healthcheck:
+      test: ["CMD", "curl", "-f", "-k", "https://localhost/actuator/health"]
+      interval: 10s
+      timeout: 5s
+      retries: 5
     networks:
       - pullit-local-network

---

62-69: Consider reducing environment variable duplication.

The same environment variables are duplicated between pullit-local-api and pullit-local-worker. Consider using YAML anchors or a shared .env file to reduce maintenance burden.

Example using YAML anchors:

x-common-env: &common-env
  APP_GEMINI_APIKEY: ${APP_GEMINI_APIKEY}
  AWS_REGION: ${AWS_REGION}
  GOOGLE_API_KEY: ${GOOGLE_API_KEY}
  KAKAO_CLIENT_SECRET: ${KAKAO_CLIENT_SECRET}
  KAKAO_REDIRECT_URI: ${KAKAO_REDIRECT_URI}
  KAKAO_REST_API_KEY: ${KAKAO_REST_API_KEY}
  S3_ACCESS_KEY: ${S3_ACCESS_KEY}
  S3_SECRET_KEY: ${S3_SECRET_KEY}

services:
  pullit-local-api:
    environment:
      - SPRING_PROFILES_ACTIVE=local
      # ... other vars ...
      <<: *common-env

src/main/java/kr/it/pullit/modules/questionset/repository/adapter/jpa/QuestionSetJpaRepository.java (1)

16-24: Add DISTINCT to the fetch-join query

Without DISTINCT, the fetch join will emit one row per question, and depending on JPA provider settings that can surface as duplicate-entity trouble (or even NonUniqueResultException) when wrapping the result in an Optional. Adding DISTINCT keeps the ORM from tripping over duplicates while still eagerly loading the questions.

Apply:

-       SELECT qs
+       SELECT DISTINCT qs

src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java (1)

222-225: Retry the “update and complete” path as well

Great call adding retry to update(...), but updateAndMarkAsComplete(...) mutates the same entity and is exposed to the identical optimistic-lock failure. Leaving it without retry means concurrent completions will still bubble an ObjectOptimisticLockingFailureException back to clients. Please annotate that method with the same @Retryable configuration so both flows behave consistently.

A minimal change:

   @Override
   @Transactional
+  @Retryable(
+      value = {ObjectOptimisticLockingFailureException.class},
+      maxAttempts = 3,
+      backoff = @Backoff(delay = 200))
   public void updateAndMarkAsComplete(
       Long questionSetId, QuestionSetUpdateRequestDto request, Long memberId) {

src/main/java/kr/it/pullit/shared/error/CommonErrorCode.java (1)

10-14: Fix code naming inconsistency.

The error code naming is inconsistent: C_001 and C_002 use underscores, while C004, C005, and C006 do not. Consider standardizing to either C_004, C_005, C_006 or removing the underscore from the earlier codes.

Apply this diff to standardize the naming:

-  INVALID_CONFIGURATION(HttpStatus.INTERNAL_SERVER_ERROR, "C_001", "서버 설정이 올바르지 않습니다: %s"),
-  INVALID_INPUT_VALUE(HttpStatus.BAD_REQUEST, "C_002", "잘못된 입력 값입니다."),
-  UNSUPPORTED_HTTP_METHOD(HttpStatus.METHOD_NOT_ALLOWED, "C004", "지원하지 않는 HTTP Method입니다."),
-  UNHANDLED_EXCEPTION(HttpStatus.INTERNAL_SERVER_ERROR, "C005", "알 수 없는 서버 에러입니다."),
-  DUPLICATE_REQUEST(HttpStatus.CONFLICT, "C006", "이미 처리된 요청입니다.");
+  INVALID_CONFIGURATION(HttpStatus.INTERNAL_SERVER_ERROR, "C_001", "서버 설정이 올바르지 않습니다: %s"),
+  INVALID_INPUT_VALUE(HttpStatus.BAD_REQUEST, "C_002", "잘못된 입력 값입니다."),
+  UNSUPPORTED_HTTP_METHOD(HttpStatus.METHOD_NOT_ALLOWED, "C_004", "지원하지 않는 HTTP Method입니다."),
+  UNHANDLED_EXCEPTION(HttpStatus.INTERNAL_SERVER_ERROR, "C_005", "알 수 없는 서버 에러입니다."),
+  DUPLICATE_REQUEST(HttpStatus.CONFLICT, "C_006", "이미 처리된 요청입니다.");

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 777f9ee and bca55b6.

📒 Files selected for processing (25)

• compose.yaml (6 hunks)
• nginx/certs/localhost.crt (1 hunks)
• nginx/certs/localhost.key (1 hunks)
• nginx/conf.d/default.conf (1 hunks)
• src/main/java/kr/it/pullit/modules/learningsource/source/service/SourceService.java (1 hunks)
• src/main/java/kr/it/pullit/modules/projection/learnstats/domain/LearnStats.java (1 hunks)
• src/main/java/kr/it/pullit/modules/projection/outbox/domain/OutboxEvent.java (1 hunks)
• src/main/java/kr/it/pullit/modules/questionset/client/GeminiClient.java (3 hunks)
• src/main/java/kr/it/pullit/modules/questionset/repository/QuestionSetRepository.java (1 hunks)
• src/main/java/kr/it/pullit/modules/questionset/repository/QuestionSetRepositoryImpl.java (1 hunks)
• src/main/java/kr/it/pullit/modules/questionset/repository/adapter/jpa/QuestionSetJpaRepository.java (1 hunks)
• src/main/java/kr/it/pullit/modules/questionset/service/QuestionService.java (1 hunks)
• src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java (3 hunks)
• src/main/java/kr/it/pullit/modules/questionset/web/QuestionSetController.java (2 hunks)
• src/main/java/kr/it/pullit/platform/security/config/SecurityConfig.java (1 hunks)
• src/main/java/kr/it/pullit/platform/web/cookie/CookieManager.java (1 hunks)
• src/main/java/kr/it/pullit/shared/error/CommonErrorCode.java (1 hunks)
• src/main/java/kr/it/pullit/shared/idempotency/DuplicateRequestException.java (1 hunks)
• src/main/java/kr/it/pullit/shared/idempotency/IdempotencyAspect.java (1 hunks)
• src/main/java/kr/it/pullit/shared/idempotency/IdempotencyExceptionHandler.java (1 hunks)
• src/main/java/kr/it/pullit/shared/idempotency/Idempotent.java (1 hunks)
• src/main/resources/application.yml (2 hunks)
• src/main/resources/data.sql (0 hunks)
• src/test/java/kr/it/pullit/modules/questionset/client/GeminiClientTest.java (5 hunks)
• src/test/java/kr/it/pullit/platform/web/cookie/CookieManagerTest.java (1 hunks)

💤 Files with no reviewable changes (1)

• src/main/resources/data.sql

🧰 Additional context used

🧬 Code graph analysis (2)

src/main/java/kr/it/pullit/modules/questionset/client/GeminiClient.java (2)

src/main/java/kr/it/pullit/modules/questionset/client/exception/LlmResponseParseException.java (1)

• LlmResponseParseException (5-14)

src/main/java/kr/it/pullit/modules/questionset/client/exception/LlmException.java (1)

• LlmException (5-22)

src/main/java/kr/it/pullit/platform/security/config/SecurityConfig.java (2)

src/main/java/kr/it/pullit/platform/security/jwt/filter/DevAuthenticationFilter.java (1)

• Profile (15-33)

src/main/java/kr/it/pullit/platform/security/jwt/handler/LocalAuthenticationHandler.java (1)

• Profile (12-47)

🪛 Gitleaks (8.28.0)

nginx/certs/localhost.key

[high] 1-28: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.

(private-key)

⏰ 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). (1)

• GitHub Check: code_check

🔇 Additional comments (24)

src/main/java/kr/it/pullit/platform/web/cookie/CookieManager.java (2)

49-50: Excellent fix for the cookie deletion issue!

This correctly addresses the logout bug by ensuring cookies are expired with the same path they were created with. The refresh_token cookie is created with path /auth/refresh (line 35), and this change ensures it's expired with the same path, which is required for proper cookie deletion.

Also applies to: 55-55

---

63-68: LGTM! Clean helper method.

The logic is clear and correctly implements path determination. The string comparison uses .equals() appropriately to avoid NPE when the constant is checked against the parameter.

src/test/java/kr/it/pullit/platform/web/cookie/CookieManagerTest.java (1)

118-135: LGTM! Comprehensive test coverage.

The test properly verifies that the refresh token cookie is expired with the correct path (/auth/refresh), ensuring the bug fix works as expected. The test follows established patterns and validates all critical cookie attributes.

src/main/java/kr/it/pullit/platform/security/config/SecurityConfig.java (1)

30-30: Based on the verification results, I can now provide the final response:

---

No security concerns with excluding SecurityConfig for the worker profile.

The worker profile is exclusively for RabbitMQ message-driven background processing. QuestionGenerationWorker uses @RabbitListener to consume messages from queues, not HTTP endpoints. No HTTP server configuration or alternative security is needed since the worker process doesn't expose any HTTP interfaces.

src/main/java/kr/it/pullit/modules/questionset/client/GeminiClient.java (1)

97-107: LGTM: Enhanced error logging for parse failures.

The signature change to accept a String parameter aligns well with the streaming aggregation approach, and the error logging at lines 101-104 will be helpful for debugging malformed responses.

src/test/java/kr/it/pullit/modules/questionset/client/GeminiClientTest.java (1)

57-69: Excellent test coverage for streaming aggregation.

The test correctly validates that multiple JSON chunks can be aggregated and parsed successfully. Splitting the JSON mid-object ({"title":"AI Quiz", and "questions":[]}) is a great edge case to test.

src/main/resources/application.yml (2)

24-26: LGTM! Improved schema management approach.

Disabling Hibernate's DDL auto-generation and moving to explicit schema.sql management prevents accidental schema modifications and is more appropriate for non-ephemeral environments.

---

110-110: LGTM! Necessary for nginx reverse proxy.

This configuration enables Spring to correctly process X-Forwarded-* headers from the nginx reverse proxy, ensuring proper URL generation and security context.

nginx/certs/localhost.crt (1)

1-24: LGTM! Development certificate for local SSL.

This mkcert-generated certificate for localhost is appropriate for local development and pairs with the private key. The expiration date (2028-02-06) provides sufficient time for local testing.

nginx/conf.d/default.conf (2)

14-28: Verify the 2-hour keepalive timeout is intentional.

The SSE configuration looks correct overall:

• proxy_buffering off and proxy_cache off enable real-time streaming
• Connection '' prevents connection closure between proxy and backend
• proxy_read_timeout 1h is appropriate for long-lived SSE connections

However, keepalive_timeout 7200s (2 hours) is unusually long. Please confirm this value aligns with your SSE connection lifecycle requirements.

---

30-38: LGTM! Standard reverse proxy with WebSocket support.

The general proxy configuration correctly forwards client information and includes WebSocket support via the Upgrade and Connection headers.

compose.yaml (5)

20-20: LGTM! Port exposure correctly removed.

Removing direct port 8080 exposure is correct since nginx now serves as the sole entry point on port 443.

---

91-95: LGTM! MariaDB healthcheck properly configured.

The healthcheck correctly tests database connectivity with authentication and selects the target database.

---

137-137: LGTM! Redis timeout adjustment is reasonable.

The timeout reduction from 10s to 5s is appropriate for Redis's fast response times.

---

149-151: LGTM! RabbitMQ healthcheck updated appropriately.

Using rabbitmq-diagnostics check_running is the recommended way to verify RabbitMQ availability.

---

30-37: Document required environment variables in .env.example or README.

Verified: The compose.yaml contains 9 environment variables without fallback values across three services (pullit-local-api, pullit-local-worker, and grafana). If these external API credentials (Gemini, AWS, Google, Kakao, S3, Grafana admin) are not set on the host, the services will fail to start or run with undefined values.

No .env.example file currently exists in the repository. Adding documentation of required variables is recommended. For sensitive values (API keys and secrets), defaults should not be provided; instead, document them as required and consider adding setup instructions in README or a .env.example template showing which variables must be configured.

src/main/java/kr/it/pullit/shared/idempotency/IdempotencyExceptionHandler.java (1)

14-16: Guard against null response values

Map.of rejects null entries. If a DuplicateRequestException is raised without a non-null message (default ctor or an explicit null), this handler will throw a NullPointerException, masking the intended 409 response. Either guarantee that every throw site supplies a message or defensively fall back to the error-code message before building the map.

If you decide to guard it locally, a diff like this would do:

+import java.util.Objects;
+import kr.it.pullit.shared.error.CommonErrorCode;
...
-    return ResponseEntity.status(HttpStatus.CONFLICT)
-        .body(Map.of("error", "duplicate_request", "message", ex.getMessage()));
+    return ResponseEntity.status(HttpStatus.CONFLICT)
+        .body(
+            Map.of(
+                "error",
+                "duplicate_request",
+                "message",
+                Objects.requireNonNullElse(
+                    ex.getMessage(), CommonErrorCode.DUPLICATE_REQUEST.getMessage())));

src/main/java/kr/it/pullit/shared/idempotency/Idempotent.java (1)

10-18: LGTM! Well-structured idempotency annotation.

The annotation design is solid with sensible defaults (10-minute TTL, standard header name). The runtime retention and method target are correct for AOP interception.

src/main/java/kr/it/pullit/modules/questionset/service/QuestionService.java (1)

140-156: LGTM! Good refactoring for consistency.

Consolidating the model name to use DEFAULT_MODEL_NAME throughout simplifies the code and ensures consistent model selection across all LLM calls.

src/main/java/kr/it/pullit/shared/idempotency/IdempotencyAspect.java (5)

30-49: Verify the duplicate request behavior aligns with requirements.

The current implementation throws DuplicateRequestException for duplicate requests rather than returning a cached result. Line 43 stores only "DONE" as a marker, not the actual response.

This means:

• First request: Executes and returns result
• Duplicate request within TTL: Throws exception (HTTP 409 Conflict)

If the requirement is to return the cached result for idempotent requests (common in payment APIs), you would need to serialize and store the actual return value instead of just "DONE".

Can you confirm whether duplicate requests should:

1. Throw an exception (current behavior), or
2. Return the cached result from the first request?

---

60-64: Document the fail-open behavior for missing idempotency headers.

When the Idempotency-Key header is missing, the aspect logs a warning but allows the request to proceed without idempotency protection (returns null, which causes line 34 to bypass the check).

This "fail-open" behavior means:

• Requests without the header are processed normally
• No duplicate protection is applied

Consider:

1. If this header should be required for annotated endpoints, throw an exception here instead of returning null
2. If optional, document this behavior clearly so developers understand that idempotency is opt-in per request

Which behavior is intended?

• Opt-in: Requests without the header proceed normally (current)
• Required: Requests without the header are rejected

---

77-80: LGTM! Appropriate use of MD5 for key generation.

Using MD5 to hash the scope string is suitable for generating cache keys. While MD5 is not cryptographically secure, it's perfectly adequate for this non-security use case (no collision concerns at this scale).

---

82-96: LGTM! Thread-safe lock acquisition.

The implementation correctly handles concurrent access:

1. The initial check (line 85-86) provides early detection and better logging
2. The atomic setIfAbsent (line 91) ensures thread-safety even if multiple threads pass the first check
3. Different log messages distinguish between "already processed" vs "concurrent race"

The two-stage check (get + setIfAbsent) is intentional and provides better observability without sacrificing correctness.

---

98-101: LGTM! Safe duration conversion.

The null check and conversion logic are correct.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify this private key is for local development only.

Static analysis correctly flagged this as a private key. While the filename and corresponding certificate (mkcert-issued) suggest this is for local development, please confirm:

1. This certificate/key pair is only used for local development
2. Developers are instructed to regenerate their own certificates using mkcert
3. Consider adding a README.md in the nginx/certs/ directory documenting the certificate generation process

To verify there are no production credentials in the repository, run:

---

🏁 Script executed:

#!/bin/bash
# Check for any references to production domains or credentials
rg -i "production|prod\.pull\.it\.kr|api\.pull\.it\.kr" nginx/

Length of output: 0

---

---

🏁 Script executed:

#!/bin/bash
# Verify nginx directory structure and certificate files
echo "=== Listing nginx/certs/ directory ===" 
fd . nginx/certs/ 2>/dev/null || echo "nginx/certs/ not found or empty"

echo ""
echo "=== Checking .gitignore for cert/key patterns ==="
rg -n "\.key|\.crt|\.pem|certs/" .gitignore 2>/dev/null || echo "No cert-related entries in .gitignore"

echo ""
echo "=== Looking for nginx-related README or documentation ==="
fd README nginx/ 2>/dev/null || echo "No README files in nginx/"

echo ""
echo "=== Checking nginx configuration files ===="
fd "\.conf" nginx/ 2>/dev/null | head -20

Length of output: 309

---

🏁 Script executed:

#!/bin/bash
# Check if mkcert certificate is typical development cert
echo "=== Checking certificate subject and issuer ==="
openssl x509 -in nginx/certs/localhost.crt -text -noout 2>/dev/null | grep -A2 "Subject:\|Issuer:" || echo "Certificate file not accessible or not found"

Length of output: 92

---

---

🏁 Script executed:

#!/bin/bash
# Check how certificates are referenced in nginx config
echo "=== nginx/conf.d/default.conf content ==="
cat nginx/conf.d/default.conf 2>/dev/null | head -50

echo ""
echo "=== Check root .gitignore for patterns ==="
cat .gitignore 2>/dev/null | head -30

echo ""
echo "=== Check for docker-compose or similar that might use these certs ==="
fd -e yml -e yaml . 2>/dev/null | xargs grep -l "localhost\|certs" 2>/dev/null | head -5

Length of output: 2016

---

Add .gitignore entry and documentation for certificate files.

Verification confirms this is development-only (mkcert-issued certificate, localhost-only nginx config targeting local API service). However, two gaps were identified:

1. Missing .gitignore: Private key files (*.key) are not excluded from version control. Add to .gitignore:

   nginx/certs/*.key
   

2. Missing developer guidance: No documentation exists explaining certificate generation. Create nginx/certs/README.md with instructions for regenerating certificates using mkcert, or document this in a main README.

Either protect the key file via .gitignore, or add explicit documentation that developers can reference. Preferably do both.

🧰 Tools

🪛 Gitleaks (8.28.0)

[high] 1-28: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.

(private-key)

🤖 Prompt for AI Agents

In nginx/certs/localhost.key (lines 1-28) the private key is committed without
.gitignore entry or developer guidance; add an entry to .gitignore to exclude
nginx/certs/*.key, create nginx/certs/README.md (or update the main README) with
step-by-step mkcert instructions to regenerate the localhost certificate and
key, and remove or rotate the committed key from the repository history (or
replace it with a placeholder) so secrets are not stored in version control.