chore: Update infrastructure, dev scripts, and documentation#5
chore: Update infrastructure, dev scripts, and documentation#5nkissick-del wants to merge 3 commits intojmagar:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR reorganizes development infrastructure by relocating the dev script to a dedicated scripts directory, pinning the UV base image version to 0.5.24, adding Claude-specific permissions configuration, updating documentation with new paths, and enhancing the dev script with improved logging, process discovery, and server lifecycle management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="scripts/dev.sh">
<violation number="1" location="scripts/dev.sh:333">
P2: PID selection uses unconstrained `pgrep -f`, which can match unrelated/stale processes, causing the PID file to point at the wrong process and potentially killing the wrong server.</violation>
<violation number="2" location="scripts/dev.sh:418">
P2: PID selection uses unconstrained `pgrep -f`, which can match unrelated/stale processes, causing the PID file to point at the wrong process and potentially killing the wrong server.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
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)
scripts/dev.sh (1)
565-566:⚠️ Potential issue | 🟡 MinorInconsistent script path in interrupt handler.
The help text references
./dev.shbut should use./scripts/dev.shto match the updated location.Proposed fix
handle_log_interrupt() { echo "" log_info "📄 Stopped following logs. Server continues running in background." - log_info "💡 Use './dev.sh --status' to check server status" 1 - log_info "💡 Use './dev.sh --tail' to resume following logs" 1 + log_info "💡 Use './scripts/dev.sh --status' to check server status" 1 + log_info "💡 Use './scripts/dev.sh --tail' to resume following logs" 1 exit 0 }
🤖 Fix all issues with AI agents
In `@Dockerfile`:
- Line 8: The Dockerfile COPY instruction references an outdated pinned UV image
tag "ghcr.io/astral-sh/uv:0.5.24"; update that tag to a more recent stable
release (for example "0.9.29" or whichever latest tested version) in the COPY
--from=... /uv /uvx /bin/ line to keep reproducible builds while getting fixes,
then rebuild and smoke-test the image to ensure no breaking changes; keep the
explicit tag pinning rather than using a floating "latest" tag.
In `@scripts/dev.sh`:
- Line 300: Split the combined declaration+assignment to avoid masking the
command substitution exit status: replace occurrences like the declaration of
variable fsize where you currently do local fsize=$(stat -f%z "$LOG_FILE"
2>/dev/null || echo 0) with a separate local declaration (local fsize) followed
by an assignment (fsize=$(stat -f%z "$LOG_FILE" 2>/dev/null || echo 0)); apply
the same change for the second occurrence at the other location (the other local
fsize usage), and if you rely on the command's exit status, capture and check
the substitution's exit code after the assignment.
- Around line 298-304: The file-size check using macOS-only stat syntax (stat
-f%z) breaks Linux; update the rotation logic around LOG_FILE/fsize to use a
portable fallback: detect which stat flavor is available (BSD vs GNU) or fall
back to a POSIX-safe method (e.g., wc -c) to compute file size, then assign that
to fsize and compare to 10485760 as before; apply the same change to the other
occurrence noted (the block around lines 393-399) so both rotations work on
Linux and macOS.
- Around line 119-121: Replace the fragile regex array-membership check with a
simple helper function: add a function (e.g., contains_elem) that accepts a
value and an array and iterates over the array comparing elements to the value,
returning success if found; then change the membership test around the pids
array to call that helper (e.g., if ! contains_elem "$pid" "${pids[@]}"; then
pids+=("$pid") ) and apply the same replacement to the other occurrence
referenced (the block around lines 131-133). Ensure the helper uses exact string
comparison (not regex) and preserves exit codes for use in the if tests.
This has been actioned. |
This has been actioned |
Overview
This PR improves the project structure and developer experience without changing core application logic. It addresses some technical debt in the development scripts and Docker configuration.
Changes
scripts/directory and improved process management (PID tracking, log rotation).Dockerfileto ensure reproducible builds..gitignoreto properly exclude local environment configuration files.Verification
Summary by cubic
Streamlines dev tooling and Docker builds for consistency and reliability. Moves the dev script, pins uv, improves logging and process handling, and updates docs.
Refactors
Migration
Written for commit cc11180. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
list_notifications()parameter renamed for clarity.Chores