Coding rules

These are the rules a review checks. The full list, with a wrong and a right version for each, is AGENTS.md in DevScripts. This page covers the ones that get pull requests sent back most often.

The examples come from ct/journiv.sh and install/journiv-install.sh in ProxmoxVE unless another script is named. AGENTS.md names Journiv as its reference script.

Scripts stay linearCopy Anchor Link

A ct/ script defines one function, update_script. An install script defines none. Commands are written out in order, top to bottom. No install_deps(), no wrapper around a helper, not even for something that is called twice.

msg_info "Initializing Database"
set -a
source /opt/journiv.env
set +a
$STD /opt/journiv/.venv/bin/python -c "from alembic.config import main; main(['upgrade', 'head'])"
SKIP_DATA_SEEDING=false $STD /opt/journiv/.venv/bin/python -c "from app.core.database import seed_initial_data; seed_initial_data()"
msg_ok "Initialized Database"

The update path in ct/journiv.sh runs the same migration command again instead of sharing a function with the install script.

There is one exception. A script that supports more than one OS family defines the functions the engine dispatches to, such as setup_deb_based and setup_alpine in the install script (called through run_os_setup) or update_deb_based and update_alpine inside update_script (called through run_os_update). install/adguard-install.sh is an example.

If you think a helper is missing, propose it for core instead of defining it in the script.

Use core helpers, not your own downloadsCopy Anchor Link

Downloading, unpacking, version tracking, runtimes and databases all have helpers. A script calls them and adds only what is specific to the application.

setup_ffmpeg
PG_VERSION="17" setup_postgresql
PG_DB_NAME="journiv" PG_DB_USER="journiv" setup_postgresql_db

fetch_and_deploy_gh_release "journiv" "journiv/journiv-app" "tarball"
PYTHON_VERSION="3.12" UV_PROJECT_DIR="/opt/journiv" setup_uv

Rules that follow from this:

• No curl/wget plus tar for a release, no git clone or git pull. Use fetch_and_deploy_gh_release with an explicit mode (tarball, binary, prebuild, singlefile), or the GitLab, Codeberg, tag, branch and plain-URL variants.

• No own version comparison. check_for_gh_release decides whether an update exists.

• No hardcoded architecture in an asset name. ct/temporal.sh in DevScripts resolves it inline:

  Copy Text

  CLEAN_INSTALL=1 fetch_and_deploy_gh_release "temporal" "temporalio/temporal" "prebuild" "latest" "/opt/temporal" "temporal_*_linux_$(arch_resolve).tar.gz"

• No msg_info/msg_ok around setup_*, fetch_and_deploy_* or check_for_*. They print their own messages.

• No Docker in ct/ and install/ scripts.

• Python goes through setup_uv and uv, not pip or python3 -m venv.

Helper functions lists what exists.

$STD in front of everything noisyCopy Anchor Link

$STD runs a command through the engine's silent wrapper: the output goes to the install log instead of the terminal (verbose mode shows it as well), and a failed command stops the install. Put it in front of every package, build and migration command.

msg_info "Installing Dependencies"
$STD apt install -y \
  build-essential \
  libffi-dev \
  libpq-dev \
  libmagic1 \
  libheif1 \
  libde265-0 \
  libpango-1.0-0 \
  libpangoft2-1.0-0 \
  redis-server
systemctl enable -q --now redis-server
msg_ok "Installed Dependencies"

Use apt, not apt-get. Do not list packages the container already has (curl, sudo, wget, gnupg, ca-certificates, jq, mc).

Messages come in pairsCopy Anchor Link

Every msg_info is closed by exactly one msg_ok with the same subject in the past tense. msg_info starts a spinner; an unclosed one keeps spinning over whatever comes next.

msg_info "Stopping Services"
systemctl stop journiv journiv-worker journiv-beat
msg_ok "Stopped Services"

• Name the application, not a placeholder: msg_info "Configuring Journiv", not "Configuring ${APP}".
• A prompt comes before msg_info, never inside an open block.
• A notice that has no matching completion is msg_warn, not msg_info.
• End a successful update with msg_ok "Updated successfully!". The automated test looks for that text to decide whether an update happened.

No hardcoded credentialsCopy Anchor Link

A password or secret that is the same on every installation is a finding. Generate it during the install and write it into the application's own configuration.

cat <<EOF >/opt/journiv.env
ENVIRONMENT=production
SECRET_KEY=$(openssl rand -hex 32)
DB_DRIVER=postgres
DATABASE_URL=postgresql://journiv:${PG_DB_PASS}@localhost:5432/journiv

PG_DB_PASS is set by setup_postgresql_db; the script never chooses a database password. The file gets chmod 600 a few lines later.

For a password the user has to type, use random_password. From install/affine-install.sh:

ADMIN_PASS=$(random_password 16)

random_password returns exactly the requested number of alphanumeric characters (24 without an argument). Do not build one from openssl rand -base64 piped through tr or cut.

In the JSON, default_credentials stays null when the password is generated; a note says where to find it.

Version files belong to the helpersCopy Anchor Link

fetch_and_deploy_gh_release "journiv" ... writes the installed version to ~/.journiv. check_for_gh_release "journiv" ... reads that file and returns success when a newer release exists. The first argument is the link between the two, so it has to be identical in the install script and in update_script.

if check_for_gh_release "journiv" "journiv/journiv-app"; then
  msg_info "Stopping Services"
  systemctl stop journiv journiv-worker journiv-beat
  msg_ok "Stopped Services"

  CLEAN_INSTALL=1 fetch_and_deploy_gh_release "journiv" "journiv/journiv-app" "tarball"

• Do not write, read or compare a version file yourself, and do not keep a second one such as /opt/<app>_version.txt.
• An application with several components uses one name per component. ct/temporal.sh tracks temporal, temporal-ui and temporal-cli separately.
• The automated test sets these files back and runs update. If the update reports success and a file is unchanged, the test fails; if a second update updates again, it adds a note.

CLEAN_INSTALL=1 empties the target directory before unpacking. Keep user data outside it: Journiv's media, logs and exports live in /opt/journiv_data and its configuration in /opt/journiv.env, so the update needs no backup step. When data cannot be moved out, use CLEAN_INSTALL_KEEP for directories and create_backup/restore_backup for files, as ct/airtrail.sh does:

create_backup /opt/airtrail/.env

CLEAN_INSTALL=1 CLEAN_INSTALL_KEEP="uploads" fetch_and_deploy_gh_release "airtrail" "johanohly/AirTrail" "tarball" "latest" "/opt/airtrail"

restore_backup

Never back up to /tmp.

Comments are the exceptionCopy Anchor Link

A script carries the header block and otherwise no comments. No banner lines, no comment that repeats the next command. The msg_info blocks already divide the script.

One short comment is allowed where a line would look like a mistake without it. install/journiv-install.sh has exactly one:

# Journiv refuses to start on plain HTTP without this.
ALLOW_INSECURE_COOKIE_AUTH_OVER_HTTP=true

The reason for a design decision goes into the pull request description, once.

Prompts can be answered up frontCopy Anchor Link

An install script that always asks cannot be deployed unattended, and the automated test cannot build it. Read a var_ variable first and ask only when it is empty. From install/forgejo-runner-install.sh:

if [[ -z "${var_forgejo_instance:-}" ]]; then
  read -r -p "${TAB3}Forgejo Instance URL (e.g. https://codeberg.org): " var_forgejo_instance || true
  var_forgejo_instance="${var_forgejo_instance:-https://codeberg.org}"
fi

The variable only reaches the container when the ct/ script exports it. From ct/forgejo-runner.sh:

export var_forgejo_instance="${var_forgejo_instance:-}"

Declare the same name in app_vars of the JSON so the website can offer it as a field. New scripts can use prompt_input_required, prompt_input, prompt_confirm and prompt_select from core instead of a raw read; they do the same check.

Smaller rulesCopy Anchor Link