feat: implement crud operation for codebase - #5
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds authenticated codebase management, repository indexing controls, and SSE-based code chat. It adds backend services and endpoints, frontend schemas, hooks, routes, dialogs, dashboard components, chat rendering, configuration updates, API specifications, and broad formatting changes. ChangesCodebase feature
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)frontend/src/styles.cssFile contains syntax errors that prevent linting: Line 3: Tailwind-specific syntax is disabled.; Line 7: Tailwind-specific syntax is disabled.; Line 120: Tailwind-specific syntax is disabled.; Line 367: Tailwind-specific syntax is disabled. 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}
There was a problem hiding this comment.
Actionable comments posted: 30
🤖 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 `@frontend/specs/codebase.md`:
- Around line 49-60: Update the Responses and Errors sections in
frontend/specs/codebase.md lines 49-60 and server/specs/codebase.md lines 49-60
to document separate 400 response examples: retain field-level data for
validation failures and show INVALID_CLONE_URL as a domain error with data:
null. Apply the same distinct examples and descriptions in both files.
- Around line 75-93: Update the PATCH codebase contract in
frontend/specs/codebase.md lines 75-93 and server/specs/codebase.md lines 76-94
so changing branch cannot leave indexed data silently stale: explicitly reject
branch changes, queue a reindex, or mark the codebase stale and prevent chat
from using that index until refreshed. Keep both specifications consistent.
In `@frontend/src/features/codebase/api/codebase.api.ts`:
- Around line 121-145: In the message handler’s JSON parsing within the
onmessage callback, remove the content, message, and text object-payload
branches because ChatService emits serialized string values for this event;
retain parsing that unwraps JSON string literals and the raw-data fallback.
- Around line 67-93: Update streamChatApi and its startStream flow to refresh an
expired access token, not only a missing one, and retry the SSE connection once
after a 401 response using the refreshed token. Preserve the existing
AbortController and callback behavior, and ensure the implementation cannot
retry indefinitely; keep the doc comment accurate with the resulting refresh
behavior.
- Around line 154-156: Update the done branch in the SSE event handling switch
to JSON-decode event.data before passing it to callbacks.onDone, matching the
parsing behavior of the message, citations, and error branches. Ensure onDone
receives the raw chat ID without surrounding JSON string quotes.
In `@frontend/src/features/codebase/components/chat-message-item.tsx`:
- Around line 159-163: Update handleCopy to handle both synchronous clipboard
access errors and rejected writeText promises, setting copied only after a
successful write and preventing unhandled rejections. Track the reset timeout
and clear it when replaced or when the component unmounts, using the component’s
existing lifecycle hooks and imports.
In `@frontend/src/features/codebase/components/citation-badge.tsx`:
- Around line 27-30: Update the disclosure button in citation-badge’s button
element to include an aria-expanded state tied to the existing expanded value
managed by setExpanded. Keep the current toggle behavior unchanged, and ensure
the button reflects whether the details panel is open or closed for assistive
technologies.
In `@frontend/src/features/codebase/components/codebase-card.tsx`:
- Around line 75-88: Update formatDate to validate the Date created from dateStr
before calling toLocaleDateString, returning null when the timestamp is invalid;
retain the existing formatted output for valid dates and the null handling for
missing values.
In `@frontend/src/features/codebase/components/codebase-chat-container.tsx`:
- Around line 148-177: Update the messages scroll container wrapping
chatMessages and the streaming/error content to include role="log" and
aria-live="polite". Add an accessible name to the textarea near the input
controls using aria-label or an associated visually hidden label, while
preserving its existing placeholder and behavior.
- Around line 42-49: Update scrollToBottom and its useEffect so streaming
updates do not restart smooth scrolling: use auto behavior while isStreaming is
true, and skip scrolling when the chat container is not already near the bottom.
Preserve smooth scrolling for eligible non-streaming message updates and use the
existing refs to determine bottom proximity.
- Around line 91-96: Update handleKeyDown to skip preventDefault and handleSend
when e.nativeEvent.isComposing is true, while preserving the existing
Enter-without-Shift submission behavior outside IME composition.
- Around line 52-68: The transition handling in codebase-chat-container’s
useEffect is committing aborted streaming content after handleClearChat clears
the chat. Add a reset-specific flag in the clear/reset path, set it when abort()
is triggered by handleClearChat, and have the effect skip the
prevStreamingRef.current true-to-false commit when that flag is set. Keep the
existing append behavior for normal stream completion in the same effect and
continue updating prevStreamingRef.current as the source of truth for streaming
state.
In `@frontend/src/features/codebase/components/codebase-edit-dialog.tsx`:
- Around line 32-38: Update the reset useEffect in codebase-edit-dialog.tsx so
it depends on the codebase identity fields instead of the full codebase object.
Keep the existing reset behavior when isOpen is true, but key the effect off
codebase.id (and any other stable identity fields needed) so refetches from
useCodebases do not overwrite in-progress form edits; leave setName, setBranch,
and setFormError behavior unchanged.
In `@frontend/src/features/codebase/components/codebase-import-dialog.tsx`:
- Around line 81-85: Replace the hand-rolled overlays with the shared accessible
dialog or project UI dialog primitive across
frontend/src/features/codebase/components/codebase-import-dialog.tsx lines
81-85, frontend/src/features/codebase/components/codebase-delete-dialog.tsx
lines 22-26, and
frontend/src/features/codebase/components/codebase-edit-dialog.tsx lines 83-87.
Ensure each dialog supports dialog semantics, aria-modal, Escape-to-close, and
focus trapping; focus the Repository Name input in the import dialog, Cancel
button in the delete dialog, and Display Name input in the edit dialog. Remove
the import dialog’s no-op stopPropagation handler.
In `@frontend/src/features/codebase/hooks/use-codebase.ts`:
- Around line 88-100: Add a cleanup effect in useChatStream that calls the
existing abort() helper when the hook unmounts, so the active AbortController is
cleared and the POST stream is closed. Keep the current abortRef/current,
setIsStreaming, and stream callback behavior unchanged, and anchor the change
around useChatStream and abort().
- Around line 114-135: Update the onMessage state updater in the ChatService
streaming flow to treat every non-empty chunk as a delta and append it
unconditionally to the previous message. Remove the cumulative-content prefix
heuristics, while preserving the existing handling for an empty previous state
and empty chunks.
In `@frontend/src/routes/_app/codebases/`$codebaseId.chat.tsx:
- Around line 11-15: Update the useCodebases call and route rendering in the
chat page to use its isPending and isError states. Show a pending state while
the codebase query is unresolved, and show a not-found state after resolution or
failure when codebaseInfo is undefined; only render the header, branch content,
and CodebaseChatContainer once a valid codebase is found.
- Around line 11-15: Add a single-codebase GET endpoint in codebase.api.ts,
expose a matching useCodebase(id) hook, and update the chat route loader to
fetch the detail record by codebaseId instead of calling useCodebases and
searching the full list. Use the detail request result for the name and branch,
and propagate its not-found response so direct navigation produces a natural
404.
In `@frontend/src/routes/index.tsx`:
- Around line 162-179: Remove the hardcoded codebaseId from the Link in the
homepage card and stop rendering the fixed ID text in the index route card.
Update the Link in the index.tsx route so it no longer navigates directly to
/codebases/$codebaseId/chat with a constant params value; instead point the card
to the codebase list/select flow and let the user choose a codebase before
entering chat. Keep the existing card styling and surrounding layout unchanged.
In `@server/specs/codebase.md`:
- Around line 98-104: Repair the PATCH response table in the codebase API
documentation by removing the stray repository link, restoring the four-column
header separator, and placing the 200, 400, 403, and 404 response rows in
separate valid Markdown rows. Match the table structure and response contract
used in the corresponding frontend codebase specification.
In `@server/src/main/java/com/meet/server/feature/chat/ChatController.java`:
- Around line 26-32: Update streamChat to safely resolve and validate the caller
identity before invoking chatService.stream: handle null or anonymous
Authentication and malformed non-UUID principal names by rejecting with HTTP
401, while preserving the existing service call for valid UUID identities.
In `@server/src/main/java/com/meet/server/feature/chat/ChatService.java`:
- Around line 48-49: Remove `@Transactional` from stream and extract the
codebaseRepository.findById lookup plus ownership validation into a separate
read-only transactional method. Update stream to call that method before
building the Flux, while preserving the existing validation behavior and
downstream flatMap, concatWith, and onErrorResume flow.
- Around line 88-92: Keep the existing SLF4J logging call and its trailing
Throwable unchanged, but update the onErrorResume fallback in ChatService to
include a stable error code in the SSE error payload alongside the message.
Ensure the code consistently identifies the chat-stream failure so clients can
use it when deciding whether to offer a retry.
In `@server/src/main/java/com/meet/server/feature/chat/dto/CodeChatRequest.java`:
- Line 1: Update the message field in CodeChatRequest, which currently uses
`@NotBlank`, to add a `@Size`(max = ...) constraint matching the model context
budget. Leave chatId unchanged, and ensure validation rejects oversized messages
before ChatService.stream invokes embeddingModel.embed.
In
`@server/src/main/java/com/meet/server/feature/codebase/CodebaseController.java`:
- Around line 38-72: Extract the repeated
UUID.fromString(authentication.getName()) logic into a private helper in
CodebaseController, such as a principal-to-user-ID method, and use it in
importCodebase, list, update, delete, and reindex. Preserve the existing handler
behavior while centralizing the principal-name contract and ensuring invalid
UUID input is handled consistently rather than surfacing independently from each
handler.
In `@server/src/main/java/com/meet/server/feature/codebase/CodebaseService.java`:
- Around line 216-223: Update CodebaseRepository with a pessimistic-write finder
for the codebase ID, and change CodebaseService.ownedCodebase to use it so
ownership checks and status transitions are serialized. In process,
conditionally transition to PROCESSING only when the codebase is still eligible;
otherwise stop without writing chunks when it was reset or removed.
- Around line 101-108: Update CodebaseService.updateCodebase so a branch change
does not leave the codebase falsely marked as INDEXED: detect when
request.branch() differs from the current branch on the owned Codebase, and then
reuse the same reindexing path as reindexCodebase instead of only saving the
name and branch. Keep the existing save-and-return flow for name-only updates,
but when the branch changes make the indexing state explicit by queueing a
reindex (or, if you choose the alternative, reset the indexing metadata
consistently so stale chunks and status cannot be reported as current).
In
`@server/src/main/java/com/meet/server/feature/codebase/dto/CodebaseUpdateRequest.java`:
- Around line 5-8: Update CodebaseUpdateRequest and its updateCodebaseApi flow
to support partial PATCH payloads: make name and branch optional, apply only
non-null values in the update handler, and preserve existing values for omitted
fields; if the client is confirmed to always send both, retain required
validation instead. Add appropriate `@Size` bounds to both fields consistent with
the entity/database limits.
In `@server/src/main/java/com/meet/server/feature/codebase/GitService.java`:
- Around line 232-258: Update deletePath to retry only the explicitly transient
deletion failure, while immediately propagating non-transient IOException types
such as DirectoryNotEmptyException. Change deleteRepository to continue deleting
all paths, collect failures from deletePath, and throw one aggregate
CodebaseException after traversal completes. Align the InterruptedException
handling in deletePath with CodebaseService.process so the import worker does
not continue with a stale interrupt flag; preserve interruption status only if
the caller handles it appropriately.
In `@server/src/main/resources/application.yaml`:
- Line 42: Replace the invalid user:emails OAuth scope entry in the application
configuration with the valid user:email scope, preserving the surrounding scope
configuration.
🪄 Autofix
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: b8fa4d88-5682-4bba-9c02-39d82e254bcd
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
frontend/.cta.jsonfrontend/README.mdfrontend/components.jsonfrontend/eslint.config.jsfrontend/package.jsonfrontend/prettier.config.jsfrontend/specs/auth.mdfrontend/specs/codebase.mdfrontend/src/components/ui/button.tsxfrontend/src/components/ui/input.tsxfrontend/src/components/ui/label.tsxfrontend/src/components/ui/select.tsxfrontend/src/components/ui/slider.tsxfrontend/src/components/ui/switch.tsxfrontend/src/components/ui/textarea.tsxfrontend/src/features/auth/api/auth.api.tsfrontend/src/features/auth/hooks/use-auth.tsfrontend/src/features/auth/schemas/auth.schema.tsfrontend/src/features/auth/types/auth.types.tsfrontend/src/features/codebase/api/codebase.api.tsfrontend/src/features/codebase/components/chat-message-item.tsxfrontend/src/features/codebase/components/citation-badge.tsxfrontend/src/features/codebase/components/codebase-card.tsxfrontend/src/features/codebase/components/codebase-chat-container.tsxfrontend/src/features/codebase/components/codebase-delete-dialog.tsxfrontend/src/features/codebase/components/codebase-edit-dialog.tsxfrontend/src/features/codebase/components/codebase-import-dialog.tsxfrontend/src/features/codebase/hooks/use-codebase.tsfrontend/src/features/codebase/index.tsfrontend/src/features/codebase/schemas/codebase.schema.tsfrontend/src/features/codebase/types/codebase.types.tsfrontend/src/lib/api-client.tsfrontend/src/lib/utils.tsfrontend/src/routeTree.gen.tsfrontend/src/router.tsxfrontend/src/routes/__root.tsxfrontend/src/routes/_app/codebases.tsxfrontend/src/routes/_app/codebases/$codebaseId.chat.tsxfrontend/src/routes/_app/codebases/index.tsxfrontend/src/routes/_app/route.tsxfrontend/src/routes/_auth/login.tsxfrontend/src/routes/_auth/register.tsxfrontend/src/routes/_auth/route.tsxfrontend/src/routes/index.tsxfrontend/src/routes/oauth2/callback.tsxfrontend/src/styles.cssfrontend/src/types/api.types.tsfrontend/tsconfig.jsonserver/build.gradleserver/specs/codebase.mdserver/src/main/java/com/meet/server/common/ratelimit/config/RateLimitConfig.javaserver/src/main/java/com/meet/server/feature/chat/ChatController.javaserver/src/main/java/com/meet/server/feature/chat/ChatService.javaserver/src/main/java/com/meet/server/feature/chat/dto/CodeChatRequest.javaserver/src/main/java/com/meet/server/feature/chat/dto/CodeCitation.javaserver/src/main/java/com/meet/server/feature/codebase/CodebaseController.javaserver/src/main/java/com/meet/server/feature/codebase/CodebaseRepository.javaserver/src/main/java/com/meet/server/feature/codebase/CodebaseService.javaserver/src/main/java/com/meet/server/feature/codebase/GitService.javaserver/src/main/java/com/meet/server/feature/codebase/dto/CodebaseResponse.javaserver/src/main/java/com/meet/server/feature/codebase/dto/CodebaseUpdateRequest.javaserver/src/main/java/com/meet/server/feature/codebase/mapper/CodebaseMapper.javaserver/src/main/java/com/meet/server/feature/repositoryfile/RepositoryFileRepository.javaserver/src/main/java/com/meet/server/feature/retriver/CodeRetriever.javaserver/src/main/java/com/meet/server/feature/user/UserRepository.javaserver/src/main/java/com/meet/server/feature/user/UserService.javaserver/src/main/resources/application.yaml
💤 Files with no reviewable changes (2)
- frontend/src/routes/__root.tsx
- frontend/src/types/api.types.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
frontend/specs/auth.md
[uncategorized] ~163-~163: The official name of this software platform is spelled with a capital “H”.
Context: ...s | Configured registrations: google, github. | - Request body: None. - Response: ...
(GITHUB)
🪛 PMD (7.26.0)
server/src/main/java/com/meet/server/feature/chat/ChatService.java
[Low] 89-90: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 3 arguments but found 4
(InvalidLogMessageFormat (Error Prone))
🔇 Additional comments (62)
frontend/specs/codebase.md (3)
1-30: LGTM!Also applies to: 38-48, 61-74, 94-103, 104-144, 146-172, 187-243, 245-262
173-186: 🗄️ Data Integrity & IntegrationVerify and document exact SSE payloads.
The event names and data encodings must match both the server emitter and frontend parser.
frontend/specs/codebase.md#L173-L186: document exact payload examples formessage,citations,done, anderror.server/specs/codebase.md#L175-L188: verify the emitted event names and serialization format.
31-37: 🔒 Security & PrivacySSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Repository import needs destination authorization, not only URL syntax.
cloneUrlflows into server-side cloning. HTTPS, host, and no-user-info checks do not establish destination authorization.
frontend/specs/codebase.md#L31-L37: document the required private-address, DNS-rebinding, and redirect controls.server/specs/codebase.md#L31-L37: document the same controls and verifyGitServiceenforces them.server/specs/codebase.md (1)
1-30: LGTM!Also applies to: 38-48, 61-75, 95-97, 106-146, 148-174, 189-265
server/build.gradle (1)
38-38: 🎯 Functional CorrectnessVerify test provider configuration.
This change selects the OpenAI model starter. The test dependencies still include Testcontainers Ollama. Verify that test profiles and model tests no longer require
spring.ai.ollamaor anOllamaContainer.#!/bin/bash set -euo pipefail rg -n -C 4 \ 'OllamaContainer|spring\.ai\.ollama|testcontainers-ollama|spring\.ai\.openai|AZURE_OPENAI' \ serverserver/src/main/resources/application.yaml (1)
5-24: LGTM!Also applies to: 43-45
server/src/main/java/com/meet/server/common/ratelimit/config/RateLimitConfig.java (1)
24-37: LGTM!frontend/package.json (1)
18-18: LGTM!Also applies to: 66-66
frontend/src/features/codebase/schemas/codebase.schema.ts (1)
3-55: LGTM!frontend/src/features/codebase/types/codebase.types.ts (1)
22-26: 🗄️ Data Integrity & IntegrationVerify the SSE event names against the backend emitter.
ChatStreamEventhard-codes the event namesmessage,citations,done, anderror, and the payload shape of each one. The backend emitter is not part of this file set. If a name or payload differs, the consumer silently drops those events.#!/bin/bash # Description: Compare the frontend SSE event contract with the backend emitter. set -euo pipefail # Locate the chat streaming backend sources. fd -t f 'ChatService.java|ChatController.java' server # Inspect emitted SSE event names and payloads. fd -t f 'ChatService.java|ChatController.java' server --exec rg -n -C4 'event\(|SseEmitter|ServerSentEvent|"message"|"citations"|"done"|"error"' # Inspect the frontend consumer that parses these events. fd -t f 'use-codebase.ts|codebase.api.ts' frontend --exec rg -n -C4 "'message'|'citations'|'done'|'error'|onmessage|EventSource|fetchEventSource"frontend/src/features/codebase/components/chat-message-item.tsx (1)
18-85: LGTM!Also applies to: 87-154, 193-215
frontend/src/features/codebase/components/citation-badge.tsx (1)
12-23: LGTM!server/src/main/java/com/meet/server/feature/codebase/mapper/CodebaseMapper.java (1)
10-22: LGTM!server/src/main/java/com/meet/server/feature/codebase/CodebaseRepository.java (2)
18-28: 🗄️ Data Integrity & IntegrationVerify the JPQL entity join and the entity/field names.
The query joins an unrelated entity with an explicit
onclause and mixes a constructor expression withcount(f.id). Confirm two points:
- The declared Hibernate version accepts
left join RepositoryFile f on f.codebase.id = c.id.RepositoryFileexposes acodebaseassociation andCodebaseResponsedeclares a constructor whose parameter types and order match the projected columns, with a numeric type that acceptsLong.A mismatch in either point fails at context startup, not at request time.
#!/bin/bash # Description: Verify the projection contract and the declared JPA/Hibernate versions. set -euo pipefail # Confirm the DTO constructor signature used by the constructor expression. fd -t f 'CodebaseResponse.java' server --exec cat -n # Confirm the RepositoryFile entity name and its codebase association. fd -t f 'RepositoryFile.java' server --exec rg -n -C3 '`@Entity`|`@Table`|codebase|class RepositoryFile' # Confirm declared Spring Boot / Hibernate versions. fd -t f 'build.gradle|build.gradle.kts|gradle.properties' server --exec rg -n 'springframework.boot|hibernate|spring-boot-starter-data-jpa|version'
16-16: LGTM!server/src/main/java/com/meet/server/feature/repositoryfile/RepositoryFileRepository.java (1)
15-20: LGTM!server/src/main/java/com/meet/server/feature/user/UserRepository.java (1)
21-23: LGTM!server/src/main/java/com/meet/server/feature/user/UserService.java (1)
24-28: LGTM!server/src/main/java/com/meet/server/feature/codebase/CodebaseController.java (1)
59-63: 🗄️ Data Integrity & IntegrationVerify that the frontend client accepts a bodyless 204 from
DELETE.Every other endpoint returns the
ApiResponse<>envelope.deletereturns204 No Contentwith no body. If the shared client always parses JSON or always readsdatafrom the envelope, the delete call fails after a successful server-side deletion.#!/bin/bash # Description: Check how the frontend API client handles responses without a body. set -euo pipefail # Inspect the shared client response handling. fd -t f 'api-client.ts' frontend --exec cat -n # Inspect the codebase delete call site. fd -t f 'codebase.api.ts' frontend --exec rg -n -C6 'delete|DELETE|204'server/src/main/java/com/meet/server/feature/codebase/CodebaseService.java (1)
47-67: LGTM!Also applies to: 156-163, 184-191
server/src/main/java/com/meet/server/feature/chat/dto/CodeCitation.java (1)
1-1: 🗄️ Data Integrity & IntegrationConfirm that no reference to the old package remains.
The record moved from
com.meet.server.feature.codebase.dtotocom.meet.server.feature.chat.dto. The JSON field set is unchanged, so the frontendcodeCitationSchemastays compatible. Any remaining import of the old fully qualified name fails the build.#!/bin/bash # Description: Find stale references to the previous CodeCitation package. set -euo pipefail # Search for the old fully qualified name and for any remaining declaration. rg -n 'com\.meet\.server\.feature\.codebase\.dto\.CodeCitation' server || echo "no stale FQN references" # List every file that uses the type, with its import line. rg -n -C2 '\bCodeCitation\b' server --type=javafrontend/src/features/codebase/hooks/use-codebase.ts (1)
21-86: LGTM!frontend/src/features/codebase/index.ts (1)
1-2: 📐 Maintainability & Code QualityVerify that the schema and type modules do not export the same names.
export *from both./schemas/codebase.schemaand./types/codebase.typesfails to compile if the two modules export any identical name. Zod feature modules commonly define a schema and an inferred type with related names, so confirm the exported names are distinct.#!/bin/bash # Description: List exported names from the codebase schema and type modules to detect collisions. fd -t f 'codebase.(schema|types).ts$' frontend/src --exec ast-grep outline {} --items allserver/src/main/java/com/meet/server/feature/codebase/dto/CodebaseResponse.java (1)
8-19: 🔒 Security & PrivacySensitive Data Exposure (CWE-522): Insufficiently Protected Credentials
Reachability: External
Reachability path
● Entry frontend/src/routes/_app/codebases/index.tsx:27 Route │ ▼ ● Hop frontend/src/routes/_app/codebases/$codebaseId.chat.tsx:5 Route │ ▼ ● Hop frontend/src/features/codebase/hooks/use-codebase.ts:29 CodebaseResponse │ ▼ ● Sink server/src/main/java/com/meet/server/feature/codebase/dto/CodebaseResponse.javaConfirm that
cloneUrlcannot carry embedded credentials back to the client.A Git clone URL can embed credentials, for example
https://user:token@host/org/repo.git. If the import flow persists the submitted URL unchanged, this response returns the token to the client, and the token then reaches browser caches and any response log. Confirm that the import path rejects or strips user-info from the URL, or that the mapper redacts it before it enters this record.#!/bin/bash # Description: Inspect the codebase mapper, entity, and import validation for clone URL handling. fd -t f 'CodebaseMapper.java|Codebase.java|CodebaseService.java|CodebaseImportRequest.java' server/src --exec cat -n {} # Look for any user-info stripping or URL validation. rg -n -C3 'getUserInfo|userInfo|cloneUrl' server/src --type=javaserver/src/main/java/com/meet/server/feature/retriver/CodeRetriever.java (1)
3-3: LGTM!server/src/main/java/com/meet/server/feature/chat/ChatService.java (1)
55-57: 🔒 Security & Privacy | ⚡ Quick winDenial of Service (CWE-770): Allocation of Resources Without Limits or Throttling
Reachability: External
Reachability path
● Entry server/src/main/java/com/meet/server/feature/chat/ChatController.java:25 streamChat │ ▼ ● Sink server/src/main/java/com/meet/server/feature/chat/ChatService.javaBound
chatIdbefore you use it as the chat-memory key.
chatIdcomes from the request body.CodeChatRequestdeclareschatIdwithout any constraint, so the value has no length or charset bound. Line 56 concatenates it intoconversationId, whichMessageChatMemoryAdvisoruses as the partition key.MessageWindowChatMemoryretains up to 20 messages for each distinct key.One authenticated codebase owner can therefore allocate an unbounded number of conversations by varying
chatIdon each request. Each new key retains its own message window. Add a@Sizeconstraint and a charset pattern onCodeChatRequest.chatId, and confirm that the configuredChatMemoryRepositoryevicts idle conversations.Cross-tenant key forgery is not possible here, because the
userIdprefix is server-derived and a UUID cannot contain:.#!/bin/bash # Description: Inspect the CodeChatRequest constraints and the configured chat memory repository. fd -t f 'CodeChatRequest.java' server/src --exec cat -n {} rg -n -C5 'ChatMemoryRepository|MessageWindowChatMemory|chat-memory|chatMemory' server/src --type=java rg -n -C5 'chat' server/src/main/resources/application.yamlfrontend/src/features/codebase/components/codebase-chat-container.tsx (1)
154-166: 🔒 Security & PrivacyXSS (CWE-79): Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')
Reachability: External
Reachability path
● Entry frontend/src/routes/_app/codebases/index.tsx:27 Route │ ▼ ● Hop frontend/src/routes/_app/codebases/$codebaseId.chat.tsx:5 Route │ ▼ ● Sink frontend/src/features/codebase/components/codebase-chat-container.tsxVerify that
ChatMessageItemescapes the streamed model output.This container passes raw model output to
ChatMessageItemasmessage.content. Every interpolation in this file is a JSX text child, so React escapes it here.ChatMessageItemis not part of this review context. If it renders the content as Markdown or HTML, confirm that it does not usedangerouslySetInnerHTMLon unsanitized output and that any rendered link cannot carry ajavascript:URL.#!/bin/bash # Description: Inspect the chat message renderer for unsafe HTML or URL sinks. fd -t f 'chat-message-item.tsx|citation-badge.tsx' frontend/src --exec cat -n {} rg -n -C4 'dangerouslySetInnerHTML|rehype-raw|allowDangerous|innerHTML' frontend/srcserver/src/main/java/com/meet/server/feature/chat/ChatController.java (1)
31-31: 🔒 Security & Privacy | ⚡ Quick winIDOR (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: External
Verify that
ChatService.streamenforces codebase ownership.The controller passes
codebaseIddirectly from the path. Any authenticated user can submit an arbitrary codebase id. The ownership check must exist inChatService.stream; that implementation is not in the provided context. If the check is missing, a user can stream code content from another user's codebase.#!/bin/bash # Inspect ChatService.stream and the ownership lookup it performs. fd -g 'ChatService.java' --exec ast-grep outline {} --items all fd -g 'ChatService.java' --exec rg -n -C6 'stream|findBy|userId|owner|AccessDenied|NotFound' {} +frontend/src/routes/_app/codebases/index.tsx (1)
33-124: LGTM!Also applies to: 254-357
frontend/src/features/codebase/components/codebase-import-dialog.tsx (1)
48-51: 🔒 Security & Privacy | ⚡ Quick winSSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Reachability path
● Entry frontend/src/routeTree.gen.ts │ ▼ ● Hop frontend/src/routes/_auth/login.tsx:82 handleSubmit │ ▼ ● Hop frontend/src/features/codebase/components/codebase-edit-dialog.tsx:123 handleSubmit │ ▼ ● Sink frontend/src/features/codebase/components/codebase-import-dialog.tsxVerify server-side clone URL validation.
This regex only requires an
https://prefix in the browser. The server clones the submitted URL, so it must validate the scheme and restrict the host. Confirm that the backend rejects internal hosts, credentials in the URL, and non-git endpoints.#!/bin/bash # Check backend validation of cloneUrl before the clone operation. fd -g 'GitService.java' -g 'CodebaseService.java' -g 'CodebaseImportRequest.java' --exec rg -n -C6 'cloneUrl|https|URI|InetAddress|Pattern|`@Pattern`|allow' {} +frontend/src/routes/_app/codebases.tsx (1)
3-9: LGTM!frontend/src/routeTree.gen.ts (1)
13-20: LGTM!Also applies to: 55-65, 202-226
frontend/src/router.tsx (1)
5-5: LGTM!frontend/src/routes/index.tsx (1)
4-10: LGTM!Also applies to: 48-60, 74-76, 118-120, 144-160, 196-198
frontend/src/routes/_app/route.tsx (1)
38-43: 📐 Maintainability & Code QualityInspect redirect detection before changing this guard.
The available evidence only covers TanStack documentation; the repository source and lockfile should be checked to determine whether
'to' in erroris an unnecessary shape check in this route.frontend/src/features/codebase/components/codebase-card.tsx (2)
214-218: 🎯 Functional CorrectnessKeep the Tailwind important modifier where it already works.
!text-whiteis valid Tailwind CSS syntax and generates the important-color utility; changing it to postfix syntax is optional cleanup, not a required fix.> Likely an incorrect or invalid review comment.
211-223: 🎯 Functional CorrectnessNo change needed for disabled link behavior.
TanStack Router
Linksupportsdisabled; when it is true, navigation/preload is prevented and the rendered anchor does not get anhref, so keyboard activation is not enabled bypointer-events-nonealone.frontend/.cta.json (1)
25-25: LGTM!frontend/README.md (1)
35-35: LGTM!Also applies to: 43-43, 56-56, 64-64, 73-77, 95-95, 163-167, 220-220
frontend/components.json (1)
21-21: LGTM!frontend/eslint.config.js (1)
18-18: LGTM!frontend/src/features/auth/api/auth.api.ts (1)
11-18: LGTM!Also applies to: 20-28, 30-32
frontend/src/features/auth/schemas/auth.schema.ts (1)
20-31: LGTM!frontend/src/features/auth/types/auth.types.ts (1)
1-2: LGTM!frontend/src/lib/api-client.ts (1)
58-58: LGTM!frontend/prettier.config.js (1)
7-10: LGTM!frontend/specs/auth.md (1)
18-22: LGTM!Also applies to: 38-63, 73-112, 123-137, 148-163, 197-211, 222-243
frontend/src/features/auth/hooks/use-auth.ts (1)
4-7: LGTM!Also applies to: 30-30, 58-58, 83-83, 108-108
frontend/src/routes/_auth/register.tsx (1)
3-13: LGTM!Also applies to: 51-52, 77-78, 91-95, 104-105, 115-116, 205-209, 218-220, 245-247, 286-299, 342-346, 358-373, 391-391
frontend/src/routes/_auth/route.tsx (1)
36-38: LGTM!Also applies to: 47-53, 62-103, 114-144, 154-162, 171-175
frontend/src/routes/oauth2/callback.tsx (1)
42-48: LGTM!frontend/src/styles.css (1)
225-235: 🎯 Functional CorrectnessVerify the cascade behavior of the new
@layer basewrapper.These selectors were previously unlayered. The wrapper changes their cascade precedence, so Tailwind utility classes can now override
aanda:hover. Confirm the intended colors and hover behavior for links on the authentication pages before merging.frontend/tsconfig.json (1)
2-9: LGTM!frontend/src/components/ui/button.tsx (1)
1-52: LGTM!frontend/src/components/ui/input.tsx (1)
1-14: LGTM!frontend/src/components/ui/label.tsx (1)
1-6: LGTM!Also applies to: 16-17
frontend/src/components/ui/select.tsx (1)
1-5: LGTM!Also applies to: 27-39, 54-66, 75-77, 95-95, 111-111, 135-135, 149-150, 167-168
frontend/src/components/ui/slider.tsx (1)
1-6: LGTM!Also applies to: 23-23, 34-48
frontend/src/components/ui/switch.tsx (1)
1-26: LGTM!frontend/src/components/ui/textarea.tsx (1)
1-11: LGTM!frontend/src/lib/utils.ts (1)
11-11: LGTM!Also applies to: 54-54
frontend/src/routes/_auth/login.tsx (1)
29-30: LGTM!Also applies to: 43-47, 76-77, 134-138, 181-185, 197-212, 232-239
| - Responses: | ||
|
|
||
| | Status | Body | Example | | ||
| |---|---|---| | ||
| | `202 Accepted` | `ApiResponse<CodebaseImportResponse>` | `{"success":true,"message":"Codebase import queued","data":{"codebaseId":"3fa85f64-5717-4562-b3fc-2c963f66afa6","status":"QUEUED","fileCount":0}}` | | ||
| | `400 Bad Request` | Validation or clone URL error envelope | `{"success":false,"message":"Validation failed","data":{"cloneUrl":"cloneUrl must use HTTPS"}}` | | ||
| | `409 Conflict` | `ApiResponse<Void>` | `{"success":false,"message":"A user can have at most 5 codebases","data":null}` | | ||
|
|
||
| - Errors: | ||
| - `CODEBASE_LIMIT_REACHED` (`409`) when the user already has five persisted codebases. All statuses count. | ||
| - `INVALID_CLONE_URL` (`400`) when service-level URI validation fails. | ||
| - Validation failures (`400`) for invalid request fields. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document separate 400 response shapes.
Validation failures use field-level data, while INVALID_CLONE_URL is a domain error with data: null.
frontend/specs/codebase.md#L49-L60: add distinct examples for validation and service-level clone URL failures.server/specs/codebase.md#L49-L60: add the same distinct examples.
📍 Affects 2 files
frontend/specs/codebase.md#L49-L60(this comment)server/specs/codebase.md#L49-L60
🤖 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 `@frontend/specs/codebase.md` around lines 49 - 60, Update the Responses and
Errors sections in frontend/specs/codebase.md lines 49-60 and
server/specs/codebase.md lines 49-60 to document separate 400 response examples:
retain field-level data for validation failures and show INVALID_CLONE_URL as a
domain error with data: null. Apply the same distinct examples and descriptions
in both files.
| ### PATCH /api/codebases/{codebaseId} | ||
|
|
||
| - Purpose: Update codebase metadata without reindexing. | ||
| - Authentication/authorization: Authenticated user required; the codebase must belong to the authenticated user. | ||
| - Request headers/path/query parameters: | ||
|
|
||
| | Name | Type | Required | Constraints | Description | | ||
| |---|---|---:|---|---| | ||
| | `Authorization` | string | Yes | Authentication is enforced by security configuration. | Authenticated principal. | | ||
| | `codebaseId` | UUID | Yes | Valid UUID path value. | Target codebase. | | ||
|
|
||
| - Request body: `CodebaseUpdateRequest` | ||
|
|
||
| | Field | Type | Required | Nullability | Constraints | Description | | ||
| |---|---|---:|---|---|---| | ||
| | `name` | string | Yes | Non-null | `@NotBlank` | New display name. | | ||
| | `branch` | string | Yes | Non-null | `@NotBlank` | New branch name. | | ||
|
|
||
| `cloneUrl` cannot be updated by this endpoint. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Branch changes must invalidate or rebuild the index.
A metadata-only branch update can report the new branch while indexed files and chat context still represent the old branch.
frontend/specs/codebase.md#L75-L93: reject branch changes, queue reindexing, or mark the codebase stale.server/specs/codebase.md#L76-L94: apply the same contract and prevent chat from using stale indexed data.
📍 Affects 2 files
frontend/specs/codebase.md#L75-L93(this comment)server/specs/codebase.md#L76-L94
🤖 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 `@frontend/specs/codebase.md` around lines 75 - 93, Update the PATCH codebase
contract in frontend/specs/codebase.md lines 75-93 and server/specs/codebase.md
lines 76-94 so changing branch cannot leave indexed data silently stale:
explicitly reject branch changes, queue a reindex, or mark the codebase stale
and prevent chat from using that index until refreshed. Keep both specifications
consistent.
| /** | ||
| * Opens a POST-based SSE connection to the chat stream endpoint. | ||
| * Proactively checks/refreshes the access token if needed before initiating the stream. | ||
| * Returns an AbortController so the caller can cancel the stream. | ||
| */ | ||
| export function streamChatApi( | ||
| codebaseId: string, | ||
| data: CodeChatRequest, | ||
| callbacks: StreamChatCallbacks, | ||
| ): AbortController { | ||
| const ctrl = new AbortController() | ||
|
|
||
| const startStream = async () => { | ||
| let token = tokenManager.getAccessToken() | ||
|
|
||
| // If no access token in memory, attempt refresh using HttpOnly cookie first | ||
| if (!token) { | ||
| try { | ||
| const refreshRes = await refreshApi() | ||
| if (refreshRes.success && refreshRes.data?.accessToken) { | ||
| tokenManager.setAccessToken(refreshRes.data.accessToken) | ||
| token = refreshRes.data.accessToken | ||
| } | ||
| } catch { | ||
| // Continue to attempt stream request | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Refresh the token when it is present but expired.
The refresh branch runs only when tokenManager.getAccessToken() returns no token. If a token exists and has expired, the code sends it unchanged. The server then rejects the stream with 401, onopen throws, and the user sees a stream error instead of an automatic recovery. The apiClient interceptors handle this case for the other functions in this file, so the SSE path behaves differently from every other request.
The doc comment on line 69 states that the function checks and refreshes the token "if needed". Update the behavior or the comment so they agree.
One option is to retry once on a 401 response: refresh the token in onopen, then restart startStream with the new token.
🤖 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 `@frontend/src/features/codebase/api/codebase.api.ts` around lines 67 - 93,
Update streamChatApi and its startStream flow to refresh an expired access
token, not only a missing one, and retry the SSE connection once after a 401
response using the refreshed token. Preserve the existing AbortController and
callback behavior, and ensure the implementation cannot retry indefinitely; keep
the doc comment accurate with the resulting refresh behavior.
| onmessage: (event) => { | ||
| switch (event.event) { | ||
| case 'message': { | ||
| let chunkText = event.data | ||
| if (typeof event.data === 'string') { | ||
| try { | ||
| const parsed = JSON.parse(event.data) | ||
| if (typeof parsed === 'string') { | ||
| chunkText = parsed | ||
| } else if (parsed && typeof parsed === 'object' && parsed !== null) { | ||
| if ('content' in parsed && typeof parsed.content === 'string') { | ||
| chunkText = parsed.content | ||
| } else if ('message' in parsed && typeof parsed.message === 'string') { | ||
| chunkText = parsed.message | ||
| } else if ('text' in parsed && typeof parsed.text === 'string') { | ||
| chunkText = parsed.text | ||
| } | ||
| } | ||
| } catch { | ||
| // Keep raw event.data | ||
| } | ||
| } | ||
| callbacks.onMessage(chunkText) | ||
| break | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Confirm the shape of the message payload with the backend.
ChatService emits sse("message", text) where text is a String. writeValueAsString produces a JSON string literal, so JSON.parse returns a plain string and line 129 assigns it. The content, message, and text object branches on lines 131-137 have no matching producer in ChatService. Remove the unused branches, or keep them only if another producer emits object payloads on this event.
🤖 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 `@frontend/src/features/codebase/api/codebase.api.ts` around lines 121 - 145,
In the message handler’s JSON parsing within the onmessage callback, remove the
content, message, and text object-payload branches because ChatService emits
serialized string values for this event; retain parsing that unwraps JSON string
literals and the raw-data fallback.
| case 'done': | ||
| callbacks.onDone(event.data) | ||
| break |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Decode the done payload before you call onDone.
ChatService.sse serializes every payload with jsonMapper.writeValueAsString(data). For the done event the data is the chatId string, so the wire value is a JSON string literal such as "default" including the quote characters. Line 155 forwards event.data unchanged, so useChatStream stores chatId as "default" with quotes. Any follow-up request that reuses this value sends the wrong conversation identifier.
Note that the message, citations, and error branches already parse the payload, so only this branch is affected.
🐛 Proposed fix to parse the JSON-encoded chat id
- case 'done':
- callbacks.onDone(event.data)
- break
+ case 'done': {
+ let resolvedChatId = event.data
+ try {
+ const parsed = JSON.parse(event.data)
+ if (typeof parsed === 'string') {
+ resolvedChatId = parsed
+ }
+ } catch {
+ // Keep raw event.data
+ }
+ callbacks.onDone(resolvedChatId)
+ break
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'done': | |
| callbacks.onDone(event.data) | |
| break | |
| case 'done': { | |
| let resolvedChatId = event.data | |
| try { | |
| const parsed = JSON.parse(event.data) | |
| if (typeof parsed === 'string') { | |
| resolvedChatId = parsed | |
| } | |
| } catch { | |
| // Keep raw event.data | |
| } | |
| callbacks.onDone(resolvedChatId) | |
| break | |
| } |
🤖 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 `@frontend/src/features/codebase/api/codebase.api.ts` around lines 154 - 156,
Update the done branch in the SSE event handling switch to JSON-decode
event.data before passing it to callbacks.onDone, matching the parsing behavior
of the message, citations, and error branches. Ensure onDone receives the raw
chat ID without surrounding JSON string quotes.
| @Transactional | ||
| public CodebaseResponse updateCodebase(UUID userId, UUID codebaseId, CodebaseUpdateRequest request) { | ||
| var codebase = ownedCodebase(userId, codebaseId); | ||
| codebase.setName(request.name().trim()); | ||
| codebase.setBranch(request.branch().trim()); | ||
| codebaseRepository.save(codebase); | ||
| return codebaseMapper.toCodebaseResponse(codebase, repositoryFileRepository.countByCodebaseId(codebaseId)); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
A branch change leaves the index stale while the status stays INDEXED.
updateCodebase writes the new branch but keeps status, lastCommitSha, indexedAt, and every indexed chunk and file. After a caller changes the branch, the codebase reports INDEXED while the stored chunks still belong to the previous branch. Code chat then answers from content that does not exist on the selected branch, and nothing signals the mismatch to the caller.
Choose one behavior and make it explicit:
- Queue a reindex when
branchchanges, in the same wayreindexCodebasedoes. - Or reset the indexing metadata so the codebase no longer reports
INDEXED.
🛠️ Proposed fix: queue a reindex when the branch changes
`@Transactional`
public CodebaseResponse updateCodebase(UUID userId, UUID codebaseId, CodebaseUpdateRequest request) {
var codebase = ownedCodebase(userId, codebaseId);
+ var newBranch = request.branch().trim();
+ var branchChanged = !newBranch.equals(codebase.getBranch());
+ if (branchChanged && (codebase.getStatus() == CodebaseStatus.QUEUED
+ || codebase.getStatus() == CodebaseStatus.PROCESSING)) {
+ throw new CodebaseException("CODEBASE_BUSY", "Codebase is currently being indexed", HttpStatus.CONFLICT);
+ }
codebase.setName(request.name().trim());
- codebase.setBranch(request.branch().trim());
+ codebase.setBranch(newBranch);
+ if (branchChanged) {
+ codeChunkRepository.deleteByCodebaseId(codebaseId);
+ repositoryFileRepository.deleteByCodebaseId(codebaseId);
+ codebase.setLastCommitSha(null);
+ codebase.setIndexedAt(null);
+ codebase.setStatus(CodebaseStatus.QUEUED);
+ }
codebaseRepository.save(codebase);
+ if (branchChanged) {
+ TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
+ `@Override`
+ public void afterCommit() {
+ processAsync(codebaseId);
+ }
+ });
+ }
return codebaseMapper.toCodebaseResponse(codebase, repositoryFileRepository.countByCodebaseId(codebaseId));
}🤖 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 `@server/src/main/java/com/meet/server/feature/codebase/CodebaseService.java`
around lines 101 - 108, Update CodebaseService.updateCodebase so a branch change
does not leave the codebase falsely marked as INDEXED: detect when
request.branch() differs from the current branch on the owned Codebase, and then
reuse the same reindexing path as reindexCodebase instead of only saving the
name and branch. Keep the existing save-and-return flow for name-only updates,
but when the branch changes make the indexing state explicit by queueing a
reindex (or, if you choose the alternative, reset the indexing metadata
consistently so stale chunks and status cannot be reported as current).
| private Codebase ownedCodebase(UUID userId, UUID codebaseId) { | ||
| var codebase = codebaseRepository.findById(codebaseId).orElseThrow(() -> | ||
| new CodebaseException("CODEBASE_NOT_FOUND", "Codebase not found", HttpStatus.NOT_FOUND)); | ||
| if (codebase.getUser() == null || !userId.equals(codebase.getUser().getId())) { | ||
| throw new CodebaseException("CODEBASE_FORBIDDEN", "You do not own this codebase", HttpStatus.FORBIDDEN); | ||
| } | ||
| String chatId = request.chatId() == null || request.chatId().isBlank() ? "default" : request.chatId().trim(); | ||
| String conversationId = userId + ":" + codebaseId + ":" + chatId; | ||
| var citations = new AtomicReference<List<CodeCitation>>(List.of()); | ||
|
|
||
| return chatClient.prompt().user(request.message().trim()) | ||
| .advisors(advisors -> advisors | ||
| .param(ChatMemory.CONVERSATION_ID, conversationId) | ||
| .param(CodeAdvisor.CODEBASE_ID_CONTEXT, codebaseId)) | ||
| .stream().chatClientResponse() | ||
| .map(response -> { | ||
| var responseCitations = response.context().get(CodeAdvisor.CITATIONS_CONTEXT); | ||
| if (responseCitations instanceof List<?> values) { | ||
| citations.set(values.stream() | ||
| .filter(CodeCitation.class::isInstance) | ||
| .map(CodeCitation.class::cast) | ||
| .toList()); | ||
| } | ||
| var text = Objects.requireNonNull(Objects.requireNonNull(response.chatResponse()).getResult()) | ||
| .getOutput().getText(); | ||
| return sse("message", text); | ||
| }) | ||
| .concatWith(Flux.defer(() -> Flux.just( | ||
| sse("citations", citations.get()), | ||
| sse("done", chatId)))) | ||
| .onErrorResume(error -> Flux.just(sse("error", Map.of("message", "Unable to complete chat")))); | ||
| return codebase; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
ownedCodebase reads the status without a lock, so the busy guard is a TOCTOU check.
ownedCodebase loads the entity with findById and no lock. reindexCodebase and deleteCodebase then decide on codebase.getStatus(). The background indexer writes that same status from another thread through statusService. Two concurrent requests can both observe INDEXED and both pass the guard.
Concrete outcome: two concurrent reindexCodebase calls each delete the chunks and files, each set QUEUED, and each schedule processAsync. Two indexing tasks then write chunk rows for the same codebase at the same time, which produces a duplicated or interleaved index. A deleteCodebase racing a reindexCodebase removes the row while a task is already scheduled, and process fails with CODEBASE_NOT_FOUND.
This is the root cause for the guards at Line 113 and Line 208. Load the row with a pessimistic write lock so the status check and the status write are serialized.
🔒 Proposed fix
Add a locking finder to CodebaseRepository:
`@Lock`(LockModeType.PESSIMISTIC_WRITE)
`@Query`("select c from Codebase c where c.id = :codebaseId")
Optional<Codebase> findByIdForUpdate(`@Param`("codebaseId") UUID codebaseId);Then use it for the ownership lookup:
private Codebase ownedCodebase(UUID userId, UUID codebaseId) {
- var codebase = codebaseRepository.findById(codebaseId).orElseThrow(() ->
+ var codebase = codebaseRepository.findByIdForUpdate(codebaseId).orElseThrow(() ->
new CodebaseException("CODEBASE_NOT_FOUND", "Codebase not found", HttpStatus.NOT_FOUND));
if (codebase.getUser() == null || !userId.equals(codebase.getUser().getId())) {
throw new CodebaseException("CODEBASE_FORBIDDEN", "You do not own this codebase", HttpStatus.FORBIDDEN);
}
return codebase;
}Also make process set PROCESSING conditionally, so a task whose codebase was reset or removed stops instead of writing chunks.
🤖 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 `@server/src/main/java/com/meet/server/feature/codebase/CodebaseService.java`
around lines 216 - 223, Update CodebaseRepository with a pessimistic-write
finder for the codebase ID, and change CodebaseService.ownedCodebase to use it
so ownership checks and status transitions are serialized. In process,
conditionally transition to PROCESSING only when the codebase is still eligible;
otherwise stop without writing chunks when it was reset or removed.
| public record CodebaseUpdateRequest( | ||
| @NotBlank String name, | ||
| @NotBlank String branch | ||
| ) {} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Both fields are mandatory, so this record does not support a partial PATCH.
updateCodebaseApi in frontend/src/features/codebase/api/codebase.api.ts sends a PATCH request with this body. @NotBlank on both name and branch rejects a body that omits either field with a 400 response. If the edit dialog submits only the changed field, the request fails. Either make the fields optional and apply each non-null value, or confirm that the client always sends both fields.
Add length bounds as well. Neither field has a @Size constraint, so the caller can persist an arbitrarily long name or branch.
#!/bin/bash
# Description: Check whether the edit dialog submits both fields and inspect the update handler.
fd -t f 'codebase-edit-dialog.tsx' frontend/src --exec cat -n {}
rg -n -C6 'CodebaseUpdateRequest' server/src --type=java🤖 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
`@server/src/main/java/com/meet/server/feature/codebase/dto/CodebaseUpdateRequest.java`
around lines 5 - 8, Update CodebaseUpdateRequest and its updateCodebaseApi flow
to support partial PATCH payloads: make name and branch optional, apply only
non-null values in the update handler, and preserve existing values for omitted
fields; if the client is confirmed to always send both, retain required
validation instead. Add appropriate `@Size` bounds to both fields consistent with
the entity/database limits.
| private void deletePath(Path path) { | ||
| try { | ||
| Files.deleteIfExists(path); | ||
| } catch (IOException e) { | ||
| throw new CodebaseException( | ||
| "CODEBASE_CLEANUP_FAILED", | ||
| "Unable to delete cloned repository path", | ||
| HttpStatus.INTERNAL_SERVER_ERROR, | ||
| e); | ||
| IOException lastFailure = null; | ||
| for (int attempt = 1; attempt <= CLEANUP_ATTEMPTS; attempt++) { | ||
| try { | ||
| Files.deleteIfExists(path); | ||
| return; | ||
| } catch (IOException exception) { | ||
| lastFailure = exception; | ||
| if (attempt == CLEANUP_ATTEMPTS) { | ||
| break; | ||
| } | ||
| try { | ||
| Thread.sleep(CLEANUP_RETRY_DELAY_MILLIS); | ||
| } catch (InterruptedException interruptedException) { | ||
| Thread.currentThread().interrupt(); | ||
| lastFailure = new IOException("Repository cleanup interrupted", interruptedException); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw new CodebaseException( | ||
| "CODEBASE_CLEANUP_FAILED", | ||
| "Unable to delete cloned repository path", | ||
| HttpStatus.INTERNAL_SERVER_ERROR, | ||
| lastFailure); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry only the transient failure and let the remaining paths finish.
Two concerns in the new retry loop:
Files.deleteIfExiststhrowsDirectoryNotEmptyExceptionwhen a child deletion failed earlier. That subtype is anIOException, so the loop retries it four times and sleeps 1 s in total. A retry cannot empty the directory, so the delay produces no benefit. Restrict the retry to the transient case and fail fast otherwise.deletePaththrows on the final attempt.deleteRepositoryinvokes it insideforEach, so the first hard failure aborts the walk and leaves every remaining file and directory on disk. Collect the failures and throw once after the walk completes, so a single locked file does not strand the whole workspace.
Note also that the interrupt path sets the thread interrupt flag and then throws. CodebaseService.process catches the exception and only logs a warning, so the worker thread continues to run with the interrupt flag set. Confirm that this is the intended behavior for the import worker.
♻️ Proposed fix to skip non-transient retries
private void deletePath(Path path) {
IOException lastFailure = null;
for (int attempt = 1; attempt <= CLEANUP_ATTEMPTS; attempt++) {
try {
Files.deleteIfExists(path);
return;
} catch (IOException exception) {
lastFailure = exception;
- if (attempt == CLEANUP_ATTEMPTS) {
+ if (attempt == CLEANUP_ATTEMPTS || exception instanceof DirectoryNotEmptyException) {
break;
}🤖 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 `@server/src/main/java/com/meet/server/feature/codebase/GitService.java` around
lines 232 - 258, Update deletePath to retry only the explicitly transient
deletion failure, while immediately propagating non-transient IOException types
such as DirectoryNotEmptyException. Change deleteRepository to continue deleting
all paths, collect failures from deletePath, and throw one aggregate
CodebaseException after traversal completes. Align the InterruptedException
handling in deletePath with CodebaseService.process so the import worker does
not continue with a stale interrupt flag; preserve interruption status only if
the caller handles it appropriately.
| level: | ||
| com.meet.server.feature.retriver.CodeRetriever: DEBUG | ||
|
|
||
| - user:emails |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== application.yaml context =="
if [ -f server/src/main/resources/application.yaml ]; then
nl -ba server/src/main/resources/application.yaml | sed -n '30,50p'
else
echo "application.yaml not found"
fi
echo
echo "== GitHub scope references in repo =="
rg -n "user:emails|user:email|scopes:" server/src/main/resources/application.yaml 2>/dev/null || true
echo
echo "== OAuth app config files =="
fd -a -i '(^application\.ya?ml$|oauth|github|client)' server/src/main/resources . | sed 's#^\./##' | head -100Repository: Meet-08/CodeCompass
Length of output: 1927
🌐 Web query:
GitHub OAuth scopes user:email user:emails documentation
💡 Result:
GitHub provides the user:email scope to manage access to a user's email addresses [1][2]. Key details regarding this scope: - Purpose: The user:email scope grants read access to a user's private email addresses [1][3]. - Hierarchy: The user scope is a broader scope that includes user:email and user:follow access [1][4]. Because of this hierarchical relationship, if your application is granted the user scope, it effectively has access to the user's email addresses even if user:email is not explicitly listed on the token [3]. - Normalization: When requesting multiple scopes, GitHub normalizes them by discarding redundant scopes [1][4]. For example, if you request both user and user:email, the resulting token will only contain the user scope because it already encompasses the privileges of user:email [1][4]. - API usage: To list email addresses for an authenticated user, you must have the user:email scope [2]. However, more advanced operations, such as adding an email address to an account, require the broader user scope [2]. - Verification: Because users may modify requested scopes during the authorization flow, applications should verify which scopes were actually granted by checking the response or the X-OAuth-Scopes header [1][3]. When checking if you have email access, it is best practice to verify if the token possesses either the user:email scope OR the broader user scope [3].
Citations:
- 1: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps
- 2: https://docs.github.com/en/rest/users/emails?apiVersion=2022-11-28
- 3: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authenticating-to-the-rest-api-with-an-oauth-app
- 4: https://docs.github.com/en/enterprise-server@3.19/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== application.yaml context =="
if [ -f server/src/main/resources/application.yaml ]; then
nl -ba server/src/main/resources/application.yaml | sed -n '30,50p'
else
echo "application.yaml not found"
fi
echo
echo "== GitHub scope references in repo =="
rg -n "user:emails|user:email|scopes:" server/src/main/resources/application.yaml 2>/dev/null || true
echo
echo "== OAuth app config files =="
fd -a -i '(^application\.ya?ml$|oauth|github|client)' server/src/main/resources . | sed 's#^\./##' | head -100Repository: Meet-08/CodeCompass
Length of output: 1927
Use the valid GitHub email scope.
GitHub documents the OAuth scope as user:email, not user:emails. Replace the user:emails entry so the authorization flow requests the intended email access.
Proposed fix
- - user:emails
+ - user:email📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - user:emails | |
| - user:email |
🤖 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 `@server/src/main/resources/application.yaml` at line 42, Replace the invalid
user:emails OAuth scope entry in the application configuration with the valid
user:email scope, preserving the surrounding scope configuration.
1 participant
This pull request primarily focuses on improving documentation consistency and code quality in the frontend project. The most significant changes include formatting and style corrections in the
frontend/README.mdandfrontend/specs/auth.md, a minor ESLint configuration update, the addition of a new package dependency, and a small tweak to the Prettier config.Documentation improvements:
frontend/specs/auth.md. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]frontend/README.md. [1] [2] [3] [4] [5] [6] [7]Code quality and configuration:
.output/**directory infrontend/eslint.config.js.frontend/prettier.config.jsfor consistency.Dependencies:
@microsoft/fetch-event-sourceas a new dependency infrontend/package.json.