Skip to content

Pass --build to every docker compose run in AGENTS.md #1196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ThisIsMissEm wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Pass --build to every docker compose run in AGENTS.md #1196

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,11 @@ Client: http://localhost:3000 · Server: http://localhost:8080

Integration tests spin up services via docker compose. Unit tests run in-process.

Always pass `--build` to `docker compose run`. Compose only builds when no image exists yet, so without it your code changes are not in the container and the run silently reports on a stale image.

```bash
# Run all tests (via docker compose)
docker compose run --rm test
docker compose run --rm --build test

# Server unit tests (no Docker)
(cd server && npm test)
Expand All @@ -116,11 +118,11 @@ CI runs entirely via GitHub Actions (`.github/workflows/apply_pr_checks.yaml`).
```bash
npm ci && npm run prettier
npm ci && npm run generate && test -z "$(git status --porcelain)"
docker compose run --rm backend npm run lint
docker compose run --rm backend npm run build
docker compose run --rm client npm run lint
docker compose run --rm client npm run build
docker compose run --rm test
docker compose run --rm --build backend npm run lint
docker compose run --rm --build backend npm run build
docker compose run --rm --build client npm run lint
docker compose run --rm --build client npm run build
docker compose run --rm --build test
Comment on lines -119 to +125

@ThisIsMissEm ThisIsMissEm Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably this should be kept the same, though it is weird to build development tools into a baked docker image..

@juanmrad juanmrad Sep 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this as I was looking at the pr. And the --build is only needed because the compose services use COPY instead of volume mounts.

For local dev, mounting the source directories would keep the container in sync without rebuilding. That avoids the --build tax entirely and makes the feedback loop faster. I think we can update docker-compose.yaml to mount ./server and ./client into the respective services instead?

@ThisIsMissEm ThisIsMissEm Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that'd actually help here, due to how volume mounts end up working with the node_modules directories and with .dockerignore — really I don't think we should be running lint, build, or test within the container typically in developer workflows: we already have those tools all existing locally via npm install — with a volume mount we'd end up with macos deps running inside a linux container which would obviously break.

The chances of drift here between the CI and local copy is next to none, bar maybe platform specific issues, since the only additional software in the container is dumb-init. Platform specific issues would be things we'd need to get fixed anyway in order to make the project contributor friendly.

@juanmrad juanmrad Sep 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand the concern now. Mounting ./server over /app would overlay the image’s existing /app, including its Linux-installed node_modules. We could preserve those with a separate /app/node_modules volume or narrower source mounts, but that requires additional Compose design.

I don’t think .dockerignore affects runtime bind mounts. It only controls the Docker build context. Either way, with the current setup the source is baked into the images, so adding --build makes sense.

```

Individual checks:
Expand All @@ -129,11 +131,11 @@ Individual checks:
| ---------------------------------------- | ------------------------------------------------------------------- |
| `check_formatting` | `npm ci && npm run prettier` |
| `check_generated_graphql` | `npm ci && npm run generate && test -z "$(git status --porcelain)"` |
| `check_api_server` (lint) | `docker compose run --rm backend npm run lint` |
| `check_api_server` (build) | `docker compose run --rm backend npm run build` |
| `run_frontend_checks_if_changed` (lint) | `docker compose run --rm client npm run lint` |
| `run_frontend_checks_if_changed` (build) | `docker compose run --rm client npm run build` |
| `check_api_server` (test) | `docker compose run --rm test` |
| `check_api_server` (lint) | `docker compose run --rm --build backend npm run lint` |
| `check_api_server` (build) | `docker compose run --rm --build backend npm run build` |
| `run_frontend_checks_if_changed` (lint) | `docker compose run --rm --build client npm run lint` |
| `run_frontend_checks_if_changed` (build) | `docker compose run --rm --build client npm run build` |
| `check_api_server` (test) | `docker compose run --rm --build test` |

Tear down:

Expand Down Expand Up @@ -205,7 +207,7 @@ Two things differ from a local dev setup:

## ROOST guiding principles

- **Commands over prose.** Prefer `docker compose run --rm test` over descriptive paragraphs.
- **Commands over prose.** Prefer `docker compose run --rm --build test` over descriptive paragraphs.
- **Same review bar.** PRs authored with agent assistance are held to the same standards as any other PR.
- **Boundaries with alternatives.** When stating a restriction, provide the alternative path (e.g. don't edit `generated.ts` — regenerate via `npm run generate`).
- **Iterate over time.** Start minimal. When you give an agent the same instruction twice, add it to this file.
Expand Down
Loading