chore(infra): use per-service container WORKDIR#4253
Conversation
Containers were uniformly using /app as WORKDIR regardless of service type, which made multi-service compose stacks ambiguous and produced confusing log paths. This aligns the working directory with the service that owns the container: - api/oss, api/ee -> /api - web/oss, web/ee -> /web - services/oss, ee -> /services Updates Dockerfiles (dev + gh), docker-compose stacks (oss + ee, dev + gh + gh.local + gh.ssl), Helm charts (alembic config defaults, web and cron deployments), Railway Dockerfiles + scripts, alembic .ini files, env .example files, migration READMEs, and self-hosting docs. Also fixes alembic config fallback paths in api/oss/src/utils/env.py, the crontab path in api/oss/src/crons/queries.sh, and the ENTRYPOINT_DIR default in web/entrypoint.sh.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:L
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR reorganizes the application's directory layout: API code moves from ChangesDirectory Layout Reorganization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 54 seconds. Comment |
dosubot
Bot
added
devops
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
api/ee/databases/postgres/migrations/tracing/README copy.md (1)
16-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe tracing README still points at the core migrations directory.
Lines 16, 28, and 34 all use
/api/ee/databases/postgres/migrations/core, so anyone following this doc will generate/apply/downgrade against the wrong Alembic config. These commands should stay under the tracing directory.Suggested fix
-docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/core agenta-ee-dev-api-1 alembic -c alembic.ini revision --autogenerate -m "migration message" +docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/tracing agenta-ee-dev-api-1 alembic -c alembic.ini revision --autogenerate -m "migration message" -docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/core agenta-ee-dev-api-1 alembic -c alembic.ini upgrade head +docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/tracing agenta-ee-dev-api-1 alembic -c alembic.ini upgrade head -docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/core agenta-ee-dev-api-1 alembic -c alembic.ini downgrade head +docker exec -e PYTHONPATH=/api -w /api/ee/databases/postgres/migrations/tracing agenta-ee-dev-api-1 alembic -c alembic.ini downgrade head
🧹 Nitpick comments (1)
hosting/docker-compose/oss/docker-compose.dev.yml (1)
131-140: 💤 Low valueConsider adding SDK/client paths to watchmedo for consistency.
The
watchmedocommand only watches/api/for changes, which means SDK or client code changes under/sdks/pythonand/clients/pythonwon't trigger a worker restart. The api service handles this with explicit--reload-dirarguments for uvicorn.For consistency, you could extend the watchmedo command to watch additional directories:
watchmedo auto-restart --directory=/api/ --directory=/sdks/python --directory=/clients/python --pattern=*.py --recursive --This is optional since SDK changes are less frequent in typical development workflows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8756d0a3-3fec-46b5-83e1-6a6a14b36cfa
📒 Files selected for processing (51)
api/ee/databases/postgres/migrations/core/README.mdapi/ee/databases/postgres/migrations/core/alembic.iniapi/ee/databases/postgres/migrations/tracing/README copy.mdapi/ee/databases/postgres/migrations/tracing/alembic.iniapi/ee/docker/Dockerfile.devapi/ee/docker/Dockerfile.ghapi/oss/databases/postgres/migrations/core/README.mdapi/oss/databases/postgres/migrations/core/alembic.iniapi/oss/databases/postgres/migrations/tracing/README.mdapi/oss/databases/postgres/migrations/tracing/alembic.iniapi/oss/docker/Dockerfile.devapi/oss/docker/Dockerfile.ghapi/oss/src/crons/queries.shapi/oss/src/utils/env.pydocs/docs/self-host/01-quick-start.mdxdocs/docs/self-host/02-configuration.mdxdocs/docs/self-host/03-upgrading.mdxhosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.examplehosting/docker-compose/run.shhosting/helm/agenta-oss/templates/_helpers.tplhosting/helm/agenta-oss/templates/cron-deployment.yamlhosting/helm/agenta-oss/templates/web-deployment.yamlhosting/railway/oss/alembic/Dockerfilehosting/railway/oss/cron/Dockerfilehosting/railway/oss/scripts/build-and-push-images.shhosting/railway/oss/scripts/deploy-from-images.shhosting/railway/oss/web/Dockerfilehosting/railway/oss/web/railway.jsonhosting/railway/oss/worker-evaluations/Dockerfilehosting/railway/oss/worker-events/Dockerfilehosting/railway/oss/worker-tracing/Dockerfilehosting/railway/oss/worker-webhooks/Dockerfileservices/ee/docker/Dockerfile.devservices/ee/docker/Dockerfile.ghservices/oss/docker/Dockerfile.devservices/oss/docker/Dockerfile.ghweb/ee/docker/Dockerfile.devweb/ee/docker/Dockerfile.ghweb/entrypoint.shweb/oss/docker/Dockerfile.devweb/oss/docker/Dockerfile.gh
| | `ALEMBIC_CFG_PATH_CORE` | Core alembic path | `/api/oss/databases/postgres/migrations/core/alembic.ini` | | ||
| | `ALEMBIC_CFG_PATH_TRACING` | Tracing alembic path | `/api/oss/databases/postgres/migrations/tracing/alembic.ini` | |
There was a problem hiding this comment.
Document these Alembic defaults as license-specific.
These paths are correct for OSS, but this page also covers EE and the EE defaults live under /api/ee/.... As written, EE operators will copy the wrong values from the table.
Suggested doc tweak
-| `ALEMBIC_CFG_PATH_CORE` | Core alembic path | `/api/oss/databases/postgres/migrations/core/alembic.ini` |
-| `ALEMBIC_CFG_PATH_TRACING` | Tracing alembic path | `/api/oss/databases/postgres/migrations/tracing/alembic.ini` |
+| `ALEMBIC_CFG_PATH_CORE` | Core alembic path | `oss: /api/oss/databases/postgres/migrations/core/alembic.ini`, `ee: /api/ee/databases/postgres/migrations/core/alembic.ini` |
+| `ALEMBIC_CFG_PATH_TRACING` | Tracing alembic path | `oss: /api/oss/databases/postgres/migrations/tracing/alembic.ini`, `ee: /api/ee/databases/postgres/migrations/tracing/alembic.ini` |📝 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.
| | `ALEMBIC_CFG_PATH_CORE` | Core alembic path | `/api/oss/databases/postgres/migrations/core/alembic.ini` | | |
| | `ALEMBIC_CFG_PATH_TRACING` | Tracing alembic path | `/api/oss/databases/postgres/migrations/tracing/alembic.ini` | | |
| | `ALEMBIC_CFG_PATH_CORE` | Core alembic path | `oss: /api/oss/databases/postgres/migrations/core/alembic.ini`, `ee: /api/ee/databases/postgres/migrations/core/alembic.ini` | | |
| | `ALEMBIC_CFG_PATH_TRACING` | Tracing alembic path | `oss: /api/oss/databases/postgres/migrations/tracing/alembic.ini`, `ee: /api/ee/databases/postgres/migrations/tracing/alembic.ini` | |
| # === EXECUTION ============================================ # | ||
| init: true | ||
| command: supercronic /app/crontab | ||
| command: supercronic /api/crontab |
There was a problem hiding this comment.
Preserve cron startup compatibility for pinned EE image tags.
This compose change assumes the pulled API image already writes /api/crontab. Any deployment still pinned to an older AGENTA_API_IMAGE_TAG will break the cron service because those images still generate /app/crontab.
Suggested compatibility fallback
- command: supercronic /api/crontab
+ command: sh -c "if [ -f /api/crontab ]; then exec supercronic /api/crontab; else exec supercronic /app/crontab; fi"📝 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.
| command: supercronic /api/crontab | |
| command: sh -c "if [ -f /api/crontab ]; then exec supercronic /api/crontab; else exec supercronic /app/crontab; fi" |
| # === EXECUTION ============================================ # | ||
| init: true | ||
| command: supercronic /app/crontab | ||
| command: supercronic /api/crontab |
There was a problem hiding this comment.
Keep the GHCR cron command backward-compatible with older API image tags.
This compose file can be upgraded independently from the published image tag. If an operator keeps an older AGENTA_API_IMAGE_TAG, that image still exposes /app/crontab, so the cron service will fail to start after this change.
Suggested compatibility fallback
- command: supercronic /api/crontab
+ command: sh -c "if [ -f /api/crontab ]; then exec supercronic /api/crontab; else exec supercronic /app/crontab; fi"📝 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.
| command: supercronic /api/crontab | |
| command: sh -c "if [ -f /api/crontab ]; then exec supercronic /api/crontab; else exec supercronic /app/crontab; fi" |
| echo "Copying local SDK and client into build contexts..." | ||
| rm -rf api/sdks services/sdks api/clients services/clients | ||
| cp -r sdks api/sdks | ||
| cp -r sdks services/sdks | ||
| cp -r clients api/clients | ||
| cp -r clients services/clients | ||
| cleanup_sdk_copies() { rm -rf api/sdks services/sdks api/clients services/clients; } | ||
| trap cleanup_sdk_copies EXIT |
There was a problem hiding this comment.
Register cleanup trap before staging copies.
If a cp -r fails before Line 319, cleanup is never registered and partial api/{sdks,clients} or services/{sdks,clients} state can be left behind.
Suggested patch
if [[ "$STAGE" == "gh.local" ]]; then
+ cleanup_sdk_copies() { rm -rf api/sdks services/sdks api/clients services/clients; }
+ trap cleanup_sdk_copies EXIT
+
+ [[ -d sdks ]] || error_exit "Missing directory: sdks"
+ [[ -d clients ]] || error_exit "Missing directory: clients"
+
echo "Copying local SDK and client into build contexts..."
- rm -rf api/sdks services/sdks api/clients services/clients
+ cleanup_sdk_copies
cp -r sdks api/sdks
cp -r sdks services/sdks
cp -r clients api/clients
cp -r clients services/clients
- cleanup_sdk_copies() { rm -rf api/sdks services/sdks api/clients services/clients; }
- trap cleanup_sdk_copies EXIT
fi
|
Superseded by re-sliced PRs that fix CI failures. See new PR layout for details. |
1 participant
Summary
Containers were uniformly using
/appasWORKDIRregardless of whichservice they hosted, which made multi-service compose stacks ambiguous
(e.g.
/app/oss/could be either the API or the web app depending onwhich container you were in) and produced confusing log paths.
This aligns container
WORKDIRwith the service that owns it:api/oss,api/ee/apiweb/oss,web/ee/webservices/oss,services/ee/servicesWhat changed
Containers:
api/{oss,ee}/docker/Dockerfile.{dev,gh}web/{oss,ee}/docker/Dockerfile.{dev,gh}services/{oss,ee}/docker/Dockerfile.{dev,gh}Compose stacks: all
WORKDIR-relative volume mounts,watchmedoreload directories, and the cron command updated to use the new paths.
hosting/docker-compose/{oss,ee}/docker-compose.{dev,gh,gh.local,gh.ssl}.ymlhosting/docker-compose/run.shhosting/docker-compose/{oss,ee}/env.{oss,ee}.{dev,gh}.exampleHelm:
hosting/helm/agenta-oss/templates/_helpers.tpl— 4 Alembicconfig-path defaults
hosting/helm/agenta-oss/templates/web-deployment.yamlhosting/helm/agenta-oss/templates/cron-deployment.yamlRailway:
hosting/railway/oss/scripts/build-and-push-images.shhosting/railway/oss/scripts/deploy-from-images.shhosting/railway/oss/web/railway.jsonApplication code (path strings only):
api/oss/src/utils/env.py—AlembicConfighardcoded fallback pathsapi/oss/src/crons/queries.sh— crontab path in awk commandweb/entrypoint.sh—ENTRYPOINT_DIRdefaultAlembic:
alembic.inifiles —script_locationREADME.mdfiles — example commandsPublic docs:
docs/docs/self-host/01-quick-start.mdxdocs/docs/self-host/02-configuration.mdxdocs/docs/self-host/03-upgrading.mdxDiff
Risks
custom Dockerfile/compose override held outside this repo (e.g. a
customer's compose file that mounts host paths into
/app/...) willbreak. Internal customers should be notified before merge.
alembicCfgPathin their values file, those overrides reference theold
/app/...path and need to be updated. Defaults in the chart's_helpers.tplmove to the new path.manually against an old container gets a stale command from cached
docs — the upgrade flow includes both the container update and the
doc update so this is consistent at release time.
helm, alembic config, and env defaults move. The Python
agentapackage itself is not touched.
QA
builds and the dev compose stack)
docker compose -f hosting/docker-compose/oss/docker-compose.dev.yml upstarts cleanly; API responds on
/health; web loadsdocker-compose.ee.dev.ymlhelm templateagainstthe OSS chart shows the new
/apipath in env varsmigration runs from
/api/oss/databases/postgres/migrations/...kubectl logs <cron-pod>shows queries.shexecuting from
/api/Notes
chore/sdk/relocate-...,feat/sdk/python-fern-client,feat/sdk/typescript-fern-client)layer onto this base.