chore(ci): faster localdev iteration#3767
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR enables hot-reload development workflows by establishing Docker Compose infrastructure for incremental service rebuilds and adding frontend routing configuration. The 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🧹 Nitpick comments (1)
docker-compose.yml (1)
414-414: ⚡ Quick winPin the Rust image used by
rust-builder(and otherrust:latestDockerfiles) to a fixed tag instead ofrust:latest.
docker-compose.ymlusesrust:latestfor the persistentrust-buildercontainer, andjust dev-rebuildrunscargo buildinside that long-lived container—so a moving tag can make dev rebuilds non-deterministic across time/machines.rust/cloud-storage/**also contains multipleFROM rust:latest ...Dockerfiles.rust-builder: image: rust:latestAlign the tag with
rust/rust-toolchain.toml(channel = "1.94.0"), e.g.rust:1.94.0(or the repo’s preferred variant).🤖 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 `@docker-compose.yml` at line 414, Replace the floating rust:latest image references with a fixed Rust toolchain tag to ensure deterministic builds: update the docker-compose service named rust-builder to use rust:1.94.0 (or the exact channel in rust/rust-toolchain.toml) and update any Dockerfiles under rust/cloud-storage that use FROM rust:latest to the same pinned tag; ensure the chosen tag matches the channel value in rust/rust-toolchain.toml so both the long-lived rust-builder container and all Dockerfiles use the identical pinned Rust image.
🤖 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 `@js/app/packages/app/vite.base.ts`:
- Around line 189-192: The proxy target for the root route '^/$' is hardcoded to
http://localhost:3000 while server.port is derived from Number(process.env.PORT
|| 3000); update the proxy config so it uses the same port value instead of
3000. Replace the literal target string in the proxy entry (the '^/$' mapping)
to construct the host using the server port value (from process.env.PORT or the
server.port variable) so that requests forwarded by the proxy go to the actual
configured port.
---
Nitpick comments:
In `@docker-compose.yml`:
- Line 414: Replace the floating rust:latest image references with a fixed Rust
toolchain tag to ensure deterministic builds: update the docker-compose service
named rust-builder to use rust:1.94.0 (or the exact channel in
rust/rust-toolchain.toml) and update any Dockerfiles under rust/cloud-storage
that use FROM rust:latest to the same pinned tag; ensure the chosen tag matches
the channel value in rust/rust-toolchain.toml so both the long-lived
rust-builder container and all Dockerfiles use the identical pinned Rust image.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a167fd09-01b6-4016-a0f4-0312639463f9
📒 Files selected for processing (3)
docker-compose.ymljs/app/packages/app/vite.base.tsjustfile
| proxy: { | ||
| // always redirect root to /app | ||
| '^/$': { target: 'http://localhost:3000', rewrite: () => '/app' }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/app/vite.base.ts"
echo "== File exists? =="
ls -la "$FILE"
echo
echo "== Relevant line range (around 160-220) =="
nl -ba "$FILE" | sed -n '160,220p'
echo
echo "== Find PORT usage in file =="
rg -n "PORT" "$FILE" || true
echo
echo "== Find proxy configuration in file =="
rg -n "proxy|rewrite|target" "$FILE"Repository: macro-inc/macro
Length of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/app/vite.base.ts"
echo "== Relevant line range (160-220) =="
cat -n "$FILE" | sed -n '160,220p'
echo
echo "== PORT usage in file =="
rg -n "PORT" "$FILE" || true
echo
echo "== proxy/rewrite/target usage in file =="
rg -n "(proxy|rewrite|target)" "$FILE"Repository: macro-inc/macro
Length of output: 2457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/app/vite.base.ts"
echo "== Lines around NO_MINIFY =="
rg -n "NO_MINIFY" "$FILE"
echo
# show a window around first occurrence
LINE=$(rg -n "NO_MINIFY" "$FILE" | head -n1 | cut -d: -f1)
START=$((LINE-20))
END=$((LINE+20))
sed -n "${START},${END}p" "$FILE" | cat -n
echo
echo "== Show proxy block context =="
# show around the proxy '^/$' line
PROXYLINE=$(rg -n "'\\^/\\$':|\\^/\\$" "$FILE" | head -n1 | cut -d: -f1)
START=$((PROXYLINE-15))
END=$((PROXYLINE+15))
sed -n "${START},${END}p" "$FILE" | cat -nRepository: macro-inc/macro
Length of output: 2739
Root proxy hardcodes 3000 despite PORT override
js/app/packages/app/vite.base.ts sets server.port from Number(process.env.PORT || 3000), but the '^/$' proxy target is hardcoded to http://localhost:3000, so / forwards to the wrong port when PORT != 3000.
Suggested fix
@@
- const NO_MINIFY = process.env.NO_MINIFY === 'true';
+ const NO_MINIFY = process.env.NO_MINIFY === 'true';
+ const DEV_PORT = Number(process.env.PORT || 3000);
@@
- port: Number(process.env.PORT || 3000),
+ port: DEV_PORT,
@@
- '^/$': { target: 'http://localhost:3000', rewrite: () => '/app' },
+ '^/$': { target: `http://localhost:${DEV_PORT}`, rewrite: () => '/app' },🤖 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 `@js/app/packages/app/vite.base.ts` around lines 189 - 192, The proxy target
for the root route '^/$' is hardcoded to http://localhost:3000 while server.port
is derived from Number(process.env.PORT || 3000); update the proxy config so it
uses the same port value instead of 3000. Replace the literal target string in
the proxy entry (the '^/$' mapping) to construct the host using the server port
value (from process.env.PORT or the server.port variable) so that requests
forwarded by the proxy go to the actual configured port.
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| container="macro-{{ service }}-1" |
There was a problem hiding this comment.
Is this always the container name or will this depend on what worktree/workspace you are in?
| auth: | ||
| external: true | ||
|
|
||
| volumes: |
There was a problem hiding this comment.
should we make this volume external so it can be used across workspaces/worktrees to improve shared caching
2 participants
Add a just command to rebuild + swap the binary so you don't have to totally rebuild the container. Also make vite / redirect to /app
Note that we are doing all the builds in a "builder" container so that we know that they can run in our containers (e.g. I use nix, my nix binaries will not run in our not nix containers)