Skip to content

Add REVIEW.md with automated PR review guidelines #157

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

Closed
andypotanin wants to merge 2 commits into from
Closed

Add REVIEW.md with automated PR review guidelines #157

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a664a35
Add REVIEW.md with automated PR review guidelines
andypotanin Aug 19, 2026
20c48ed
Address Copilot and Devin review feedback on REVIEW.md
andypotanin Aug 19, 2026
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: 26 additions & 0 deletions REVIEW.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Review Guidelines - worker (udx-worker base image)

Base container image for the entire UDX worker family (worker-nodejs, worker-php, worker-tooling and their children worker-site, worker-engine, docker-sftp, plus R2A). Ubuntu base, runs as non-root UID/GID 500. A regression here propagates to every downstream image and every tenant workload.

## Critical Areas (extra scrutiny)

- `bin/entrypoint.sh`: the ENTRYPOINT for the whole image family. Trace every change for child-image compatibility (children rely on its env handling, service startup, and exit behavior).
- `lib/` shared runtime library (`process_manager.sh`, `env_handler.sh`, `secrets.sh` + `lib/secrets/{aws,azure,gcp}.sh`, `worker_config.sh`, `runtime_output.sh`, `utils.sh`, `cli.sh` + `lib/cli/*`): function signature or output format changes are breaking changes for children; require a check of downstream usage.
- `src/configs/services.yaml` and `src/configs/worker.yaml`: default service and worker config schema consumed downstream.
- Dockerfile UID/GID 500 creation and the `chown -R` block: everything downstream assumes UID 500 with specific ownership. Changes to user, ownership, or directory permissions are the classic source of child-image breakage (log dirs, port binds, home paths). Flag ANY permissions change and require downstream verification.
- `worker.yml` secrets resolution (`gcp/...`, `aws/...`, `azure/...`, `bitwarden/...` refs): watch for changes that could log resolved secret values or weaken provider auth handling.

## Release Model

- Version comes from GitVersion (`ci/git-version.yml`): merge to `latest` cuts a Minor release automatically when a filtered path changed (`Dockerfile`, `bin/**`, `lib/**`, `src/**`, `etc/**`, `test/**`, `Makefile*`, `ci/**`). There is no changelog; the PR description is the release note - require it to state downstream impact (which child images need rebumps).
- Child images pin this image by version tag; a release is inert for children until each bumps its `FROM` pin. Findings about rollout should reference that pin chain.

## Conventions to Enforce

- Shell passes shellcheck with no new exclusions; Dockerfile passes hadolint; YAML passes yamllint (all run in CI on every push).
- Tool and package versions in the Dockerfile stay pinned; reject newly unpinned installs or removed checksum verification.
- `make test` and `make build` stay the canonical local verification commands.

## Security

- This image handles cloud credentials (GCP_CREDS, AWS_CREDS, AZURE_CREDS, BITWARDEN_CREDS) to resolve secrets. Flag anything that writes credentials to disk outside established paths, echoes them, or broadens their lifetime.
Loading