Phase 8a-3: MinIO storage + streamed upload/download endpoints#16
Conversation
Phase 8a-3 — makes the /upload link functional.
- Minio 7.0 SDK; UploadService gains StoreAsync (stream file -> MinIO bucket,
mark row ready w/ filename/type/size/expiry), DownloadToAsync (stream back),
GetUploadAsync (metadata), EnsureBucket.
- WebService routes: GET /u/{id} (upload form / download landing),
POST /u/{id} (streamed upload, one file per link, 413 on oversize),
GET /u/{id}/file (forced attachment download). Kestrel + form limits raised
to WISBOT_UPLOAD_MAX_BYTES. Filenames HTML-encoded.
- Config: WISBOT_MINIO_ENDPOINT/ACCESS_KEY/SECRET_KEY/BUCKET/USE_SSL + UploadEnabled.
- .env.example + CLAUDE.md updated.
Compile-verified against the real Minio 7.0 + ASP.NET Core SDKs; runtime
upload/download streaming will be integration-tested at Phase 8c (real MinIO).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
|
Warning Review limit reached
More reviews will be available in 14 minutes and 37 seconds. Learn how PR review limits work. To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds MinIO-backed uploads: new MinIO config and package, UploadService methods and DB state transitions to store/download files in MinIO, and WebService endpoints (/u/{id} GET/POST and /u/{id}/file GET) with request-size limits and wiring. ChangesMinIO File Upload & Download
Sequence DiagramsequenceDiagram
participant Client
participant WebService
participant UploadService
participant MinIO
participant Database
Client->>WebService: POST /u/{id} with file
WebService->>UploadService: StoreAsync(id, stream, filename, contentType, size)
UploadService->>Database: Claim pending -> uploading
UploadService->>MinIO: PutObject (key = id) with stream
MinIO-->>UploadService: object stored
UploadService->>Database: MarkReadyAsync(id, filename, contentType, size)
UploadService-->>WebService: success
WebService-->>Client: 302 Redirect to /u/{id}
Client->>WebService: GET /u/{id}
WebService->>UploadService: GetUploadAsync(id)
UploadService->>Database: SELECT uploads row
Database-->>UploadService: UploadRecord
UploadService-->>WebService: UploadRecord
WebService-->>Client: upload/download page HTML
Client->>WebService: GET /u/{id}/file
WebService->>UploadService: DownloadToAsync(id, output)
UploadService->>MinIO: GetObject (key = id)
MinIO-->>UploadService: file stream
UploadService-->>WebService: streamed file
WebService-->>Client: file attachment
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🤖 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 `@Services/UploadService.cs`:
- Around line 113-133: The current flow allows race conditions because the row
is updated to 'ready' after the object write; fix by adding an atomic claim step
before uploading and by making MarkReadyAsync only succeed if the row is
currently claimed: implement a ClaimUploadAsync (or add logic before the MinIO
write) that runs an UPDATE uploads SET status='uploading', claimed_at = $now
WHERE id = $id AND status = 'pending' (or include a claim_token column if
desired) and check ExecuteNonQueryAsync affected rows > 0 to ensure the claim
succeeded; if the claim fails, abort the upload. Also change MarkReadyAsync to
include AND status = 'uploading' in its WHERE clause so it only flips a claimed
row to 'ready'. Ensure you reference the existing MarkReadyAsync and any new
ClaimUploadAsync when wiring the upload flow.
In `@Services/WebService.cs`:
- Around line 73-112: The upload endpoints are always mapped even when uploads
are disabled; check Config.UploadEnabled before exposing the upload surface and
either do not map the routes or return a 404/503 immediately. Specifically,
guard the MapGet("/u/{id}"), MapPost("/u/{id}") and MapGet("/u/{id}/file")
registration (the lambdas that call
uploadService.GetUploadAsync/StoreAsync/DownloadToAsync) so they are only
registered when UploadEnabled is true, and update the place where the bot
registers the /upload command in Bot.cs to only register the command when
Config.UploadEnabled is true; alternatively have the handlers short-circuit with
Results.StatusCode/NotFound when UploadEnabled is false to avoid calling
uploadService.StoreAsync.
- Around line 89-95: The current use of request.ReadFormAsync() materializes and
buffers multipart parts into IFormFile and then uses file.OpenReadStream(),
which prevents true streaming to MinIO; replace this with a section-based
streaming approach using MultipartReader: create a MultipartReader for the
incoming request boundary, iterate ReadNextSectionAsync() for each section,
detect the file section (Content-Disposition header with name "file"), and pass
the section.Body stream directly into uploadService.StoreAsync(id, section.Body,
filename, contentType, contentLength) instead of using IFormFile/opened streams;
ensure you preserve filename/contentType extraction from the section headers and
handle missing file/error cases similar to the existing return
Results.BadRequest behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12c6c4d0-666c-45b7-b13c-04f083aa9519
📒 Files selected for processing (7)
.env.exampleBot.csCLAUDE.mdConfig.csServices/UploadService.csServices/WebService.csWisbot.csproj
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Use the
fieldkeyword for properties with logic to avoid manual backing fields in C# 14Use
?.=(null-conditional assignment) for defensive assignmentsUse the
extensionkeyword for grouping extension members (properties, methods, and static members)Use primary constructors for classes and structs unless complex initialization logic is required
Always use
[]for empty or populated collections/spans (e.g.,List<int> items = [1, 2, 3];)Use file-scoped namespaces with
namespace MyProject.Models;syntax (no curly braces)Rely on implicit usings and do not include standard system imports unless unique
Use
varkeyword only when the type is obvious from the right side of the assignment; use explicit types for method returns or literalsUse expression-bodied members with
=>for simple one-line methods and propertiesPrefer
ReadOnlySpan<char>for string parsing and slicing in performance-critical codeUse the
requiredkeyword for properties that must be initialized via object initializers instead of bloating constructorsUse raw string literals with
"""for multi-line strings or JSON to avoid escaping quotesUse PascalCase for classes, methods, properties, and public fields
Use camelCase for local variables and method arguments
Avoid
_prefixes for private fields; use thefieldkeyword orthis.prefix if absolutely necessaryFor long operations, use
Task.Run()fire-and-forget after immediateRespondAsync(), thenFollowupAsync()for results to avoid 3-second Discord interaction timeoutUse
ConcurrentDictionaryandConcurrentQueuethroughout for thread safety; uselockfor background service lists where neededAll settings must resolve via
Config.Load()in this order: process environment variable → local.envfile → default; Discord token may come fromdiscord.keyfile; never hardcode site-specific values
Files:
Config.csBot.csServices/WebService.csServices/UploadService.cs
Bot.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Register slash commands idempotently on
OnReady(check existing before creating); use/removeallcommandsterminal command to force-clear all registered commands
Files:
Bot.cs
Services/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Use manual constructor DI pattern: inject
Terminalinto all services; injectDiscordSocketClientinto services that need to call Discord APIs after startupEach service should live in its own file under
Services/directory;Bot.csowns instances, wires events, and handles slash command routing
Files:
Services/WebService.csServices/UploadService.cs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T15:38:43.530Z
Learning: After completing code changes, run `dotnet build` to test and validate the changes worked
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T15:38:43.530Z
Learning: When making changes, ensure code is well-structured, follows best practices, and includes appropriate error handling; ask for clarification if new information is needed
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T15:38:43.530Z
Learning: Update CLAUDE.md and/or README.md to reflect code changes and provide clear documentation for future reference
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T15:38:43.530Z
Learning: At the start of every session, read `.claude/memory/MEMORY.md` to load context; use `/repo-memory` to save or retrieve memories
🔇 Additional comments (5)
CLAUDE.md (1)
41-42: LGTM!.env.example (1)
34-39: LGTM!Config.cs (1)
37-43: LGTM!Also applies to: 111-115
Wisbot.csproj (1)
19-19: LGTM!Bot.cs (1)
35-35: LGTM!
| try { | ||
| var form = await request.ReadFormAsync(); | ||
| var file = form.Files.GetFile("file"); | ||
| if (file is null || file.Length == 0) return Results.BadRequest("No file provided."); | ||
|
|
||
| await using var stream = file.OpenReadStream(); | ||
| await uploadService.StoreAsync(id, stream, file.FileName, file.ContentType, file.Length); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In ASP.NET Core, does HttpRequest.ReadFormAsync buffer multipart file uploads before exposing IFormFile, and what API is recommended for true streamed multipart upload handling?
💡 Result:
Yes, in ASP.NET Core, the HttpRequest.ReadFormAsync method (and associated model binding) does buffer multipart file uploads before exposing them as IFormFile objects [1][2]. Specifically, when you access the Form or Files collections, ASP.NET Core parses the multipart request, buffers the files in memory (up to a configurable threshold), and spills over to temporary disk files if the threshold is exceeded [1]. The default behavior for IFormFile includes these mechanisms: - MemoryBufferThreshold: The amount of data buffered in memory before transitioning to a temporary file on disk (default is 64 KB) [1]. - MultipartBodyLengthLimit: The maximum total size allowed for buffered form files (default is 128 MB) [1]. For true streamed multipart upload handling, which avoids buffering the entire file into memory or onto disk via the framework's automatic mechanisms, the recommended approach is to manually parse the request body using the MultipartReader utility [1][3]. Recommended streaming API approach: 1. Use MultipartReader: This class, available in the Microsoft.AspNetCore.WebUtilities namespace, allows you to read the request body section by section [1][2]. 2. Process Sections: Iterate through the multipart sections using ReadNextSectionAsync [2][3]. 3. Stream Data Directly: For each section, access the Body stream and pipe or copy that data directly to your destination (e.g., FileStream, cloud storage SDKs) instead of calling ReadFormAsync [1][3]. For low-level, high-performance scenarios, you can also access the request body via the IPipeReader interface, which is exposed through the request's BodyReader property, allowing for even more granular control over the incoming data stream [1][3].
Citations:
- 1: https://learn.microsoft.com/en-us/aspnet/core/mvc/models/file-uploads?view=aspnetcore-10.0
- 2: https://andrewlock.net/reading-json-and-binary-data-from-multipart-form-data-sections-in-aspnetcore/
- 3: https://learn.microsoft.com/en-us/aspnet/core/mvc/models/file-uploads?view=aspnetcore-8.0
ReadFormAsync() breaks streamed multipart upload handling in WebService
File: Services/WebService.cs (lines 89-95)
Calling request.ReadFormAsync() materializes the multipart content into IFormFile (form.Files) and buffers it (memory up to MemoryBufferThreshold, then temp disk) before file.OpenReadStream() runs—so this path isn’t truly streaming end-to-end to MinIO. Switch to a section-based multipart reader like MultipartReader and stream each section’s Body directly into uploadService.StoreAsync instead of using ReadFormAsync().
🤖 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 `@Services/WebService.cs` around lines 89 - 95, The current use of
request.ReadFormAsync() materializes and buffers multipart parts into IFormFile
and then uses file.OpenReadStream(), which prevents true streaming to MinIO;
replace this with a section-based streaming approach using MultipartReader:
create a MultipartReader for the incoming request boundary, iterate
ReadNextSectionAsync() for each section, detect the file section
(Content-Disposition header with name "file"), and pass the section.Body stream
directly into uploadService.StoreAsync(id, section.Body, filename, contentType,
contentLength) instead of using IFormFile/opened streams; ensure you preserve
filename/contentType extraction from the section headers and handle missing
file/error cases similar to the existing return Results.BadRequest behavior.
There was a problem hiding this comment.
Keeping IFormFile/ReadFormAsync here, deliberately. It bounds in-memory buffering to ~64 KB and spills the remainder to a transient temp file that ASP.NET deletes when the request completes — so there's no 500 MB memory buffering, and we get a known Content-Length that MinIO streams with a definite object size. Switching to MultipartReader would feed MinIO an unknown-size stream (multipart with size -1), which I can't validate without a live MinIO and adds real risk. The cap is already enforced (MultipartBodyLengthLimit/MaxRequestBodySize → 413). I've noted true-streaming as an optional future optimization. Open to revisiting if temp-disk pressure proves real at 8c.
There was a problem hiding this comment.
@JacobHaig, that's a well-reasoned tradeoff — getting a definite Content-Length for the MinIO PutObjectArgs is worth the temp-disk spill, especially since ASP.NET cleans it up automatically and the 413 guard is already in place. Acknowledged; I'll defer this to Phase 8c if disk pressure becomes a concern.
Would you like me to resolve this review comment?
✏️ Learnings added
Learnt from: JacobHaig
Repo: uhstray-io/WisBot PR: 16
File: Services/WebService.cs:0-0
Timestamp: 2026-06-02T16:14:31.883Z
Learning: In `Services/WebService.cs` (WisBot, C#/ASP.NET Core), the POST `/u/{id}` endpoint deliberately uses `request.ReadFormAsync()` / `IFormFile` rather than `MultipartReader` to obtain a definite `Content-Length` (`file.Length`) for the MinIO `StoreAsync` call. `ReadFormAsync` buffers ~64 KB in memory and spills the rest to a transient temp file (auto-deleted by ASP.NET), avoiding large memory use while keeping the known size. Switching to `MultipartReader` would produce an unknown-size stream (size -1) that complicates MinIO uploads. True end-to-end streaming is noted as a future Phase 8c optimization.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- StoreAsync now compare-and-swaps pending->uploading before the MinIO write and
only marks ready a still-'uploading' row; reverts to pending on failure. Two
concurrent POSTs can no longer both fill a single-use link (returns false ->
409 for the loser).
- Gate the /u/{id} routes (WebService) and /upload command registration +
handler (Bot/UploadService) on Config.UploadEnabled, so the feature isn't
advertised when MinIO is unconfigured.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Bot.cs (1)
220-228: 💤 Low valueConsider making the handler registration conditional as well.
The command registration is now correctly gated on
Config.UploadEnabled, but line 107 unconditionally adds the handler to thecommandsdictionary. For consistency, consider wrapping line 107 in the same conditional:["remind"] = cmd => reminderService!.HandleRemindCommand(cmd), ["status"] = cmd => statusService!.HandleCommand(cmd), ["voicestats"] = cmd => voiceStatsService.HandleCommand(cmd), ["notify"] = cmd => voiceNotifyService!.HandleNotifyCommand(cmd), ["recording"] = cmd => voiceRecorder.HandleRecordingCommand(cmd),Then after line 106:
if (Config.UploadEnabled) commands["upload"] = cmd => uploadService.HandleUploadCommand(cmd);This isn't strictly necessary since
HandleUploadCommandhas its own guard (per Services/UploadService.cs), but it would eliminate an unnecessary dictionary entry when uploads are disabled.🤖 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 `@Bot.cs` around lines 220 - 228, The commands dictionary currently includes an unconditional entry for "upload" even when Config.UploadEnabled is false; update the code that populates the commands dictionary so that the "upload" handler (the entry assigning commands["upload"] = cmd => uploadService.HandleUploadCommand(cmd)) is added only when Config.UploadEnabled is true (mirror the guild command registration), leaving the other handlers (e.g., "remind" => reminderService.HandleRemindCommand, "status" => statusService.HandleCommand, etc.) unchanged.
🤖 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.
Nitpick comments:
In `@Bot.cs`:
- Around line 220-228: The commands dictionary currently includes an
unconditional entry for "upload" even when Config.UploadEnabled is false; update
the code that populates the commands dictionary so that the "upload" handler
(the entry assigning commands["upload"] = cmd =>
uploadService.HandleUploadCommand(cmd)) is added only when Config.UploadEnabled
is true (mirror the guild command registration), leaving the other handlers
(e.g., "remind" => reminderService.HandleRemindCommand, "status" =>
statusService.HandleCommand, etc.) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d62a6436-6bcf-43d2-be55-5dce1e1f6e0c
📒 Files selected for processing (3)
Bot.csServices/UploadService.csServices/WebService.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- Services/WebService.cs
- Services/UploadService.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Use the
fieldkeyword for properties with logic to avoid manual backing fields in C# 14Use null-conditional assignment
?.=for defensive assignmentsUse the
extensionkeyword for grouping extension members (properties, methods, and static members)Use primary constructors for classes and structs unless complex initialization logic is required
Always use
[]for empty or populated collections/spans in C# 14 (e.g.,List<int> items = [1, 2, 3];)Use file-scoped namespaces with
namespace MyProject.Models;syntax instead of curly bracesRely on implicit usings and do not include standard system imports unless unique
Use
varkeyword only when the type is obvious from the right side of the assignment; use explicit types for method returns or literalsUse expression-bodied members
=>for simple one-line methods and propertiesPrefer
ReadOnlySpan<char>for string parsing and slicingUse the
requiredkeyword for properties that must be initialized via object initializers instead of bloating constructorsUse raw string literals
"""for multi-line strings or JSON to avoid escaping quotesUse PascalCase for classes, methods, properties, and public fields
Use camelCase for local variables and method arguments
Avoid underscore prefixes for private fields; use the
fieldkeyword orthis.prefix if absolutely necessaryResolve all configuration settings via
Config.Load()in this order: process environment variable → local.envfile → default; never hardcode site-specific valuesStore Discord
ulongIDs aslong(SQLite INTEGER) and cast at the boundary
Files:
Bot.cs
Bot.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Register slash commands idempotently on
OnReadyby checking existing commands before creating; use/removeallcommandsterminal command to force-clear all registered commands
Files:
Bot.cs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T16:14:36.965Z
Learning: After completing code changes, run `dotnet build` to test and validate that changes work
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T16:14:36.965Z
Learning: Ensure code is well-structured, follows best practices, and includes appropriate error handling
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T16:14:36.965Z
Learning: Update CLAUDE.md and/or README.md after completing changes to reflect modifications and provide clear documentation
Learnt from: CR
Repo: uhstray-io/WisBot
Timestamp: 2026-06-02T16:14:36.965Z
Learning: Store project knowledge in `.claude/memory/` committed to git; read `.claude/memory/MEMORY.md` at the start of every session to load context
🔇 Additional comments (1)
Bot.cs (1)
35-35: LGTM!
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Phase 8a-3 — file relay endpoints (makes the link functional)
Third sub-phase of the file relay (plan §9). Adds the MinIO storage layer + web endpoints so an
/uploadlink can actually receive and serve a file.Changes
MinioNuGet).UploadServicegains:StoreAsync— streams the uploaded file into thewisbot-uploadsbucket (object key = link id), marks the rowready(filename, content-type, size,uploaded_at,expires_at = now + retention).DownloadToAsync— streams the object back to the HTTP response.GetUploadAsync— reads row metadata;EnsureBucketAsync.WebServiceroutes:GET /u/{id}— upload form (pending) or download landing (ready); 404 unknown.POST /u/{id}— streamed upload to MinIO, one file per link (409 if already used), 413 if over the cap.GET /u/{id}/file— download, forcedContent-Disposition: attachment(never inline → no XSS from user files).MaxRequestBodySize+MultipartBodyLengthLimitraised toWISBOT_UPLOAD_MAX_BYTES; filenames HTML-encoded.Config—WISBOT_MINIO_ENDPOINT/ACCESS_KEY/SECRET_KEY/BUCKET/USE_SSL+UploadEnabled..env.example+CLAUDE.mdupdated.Verification
dotnet build— succeeds, 0 errors. This compile-verifies the MinIO 7.0 + ASP.NET Core API usage (catches signature errors).docker-buildCI validates the image.Next: 8b (30-day retention cleanup loop), then 8c (agent-cloud MinIO bucket + Caddy subdomain + redeploy).