From 3fd447f5e1e1c4432ca8b9cb1658dcdfb9bf8c94 Mon Sep 17 00:00:00 2001 From: Erik Hjelm Fjeldheim Date: Sat, 10 Oct 2026 13:32:32 -0600 Subject: [PATCH 1/4] feat: #44 deploy the server build to where crate-api runs it The workflow built server/ in the runner's own workspace and restarted crate-api, but nothing ever put that build where the unit actually runs it. crate-api.service has WorkingDirectory=~/T19-Project-2/server and ExecStart=.../node dist/index.js, so a restart just brought back whatever had been installed on the machine by hand. Reported by rachelks on #42. The deploy step now: 1. checks out $GITHUB_SHA in ~/T19-Project-2, which is a git clone of main as of this issue rather than a hand copy 2. installs and builds server/ there 3. restarts crate-api and polls until /graphql answers 4. runs a smoke suite through the same Apache proxy the browser uses Three things this fixes that were wrong before: - The unit check is [ -f ... ], not `systemctl list-unit-files | grep`. grep -q closes the pipe at the first match, systemctl dies of SIGPIPE, and with pipefail the pipeline reports failure. The if took the else branch and skipped a service that was definitely there. - The restart needs a NOPASSWD sudoers rule, because the runner has no TTY. Scoped to three systemctl commands. - No sleep. Type=simple reports active before Node binds the port, and a measured 1850 ms startup made `sleep 2` produce a 503 through Apache. It polls instead. Verified on the VM: ~/T19-Project-2 is a real clone at 0fbd54a, git fetch works with the deploy key added for that machine, `checkout --force` leaves .gitignore's .env untouched (it holds DATABASE_URL), and sudo is passwordless for the three commands. The skip guard for `server/` stays, because #42 has not merged yet. Once it does, the next push to main will deploy the API end to end. Co-Authored-By: opencode --- .github/workflows/ci.yml | 79 ++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d8c3df..fe2c7fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,44 +114,68 @@ jobs: chmod -R a+rX /var/www/html/project2 curl -fsS -o /dev/null -w 'GET /project2/ : %{http_code}\n' http://localhost/project2/ - # Restarting the GraphQL API is M2 work. Until `server/` exists in this - # repository there is nothing to deploy, and restarting the unit would - # just bring back whatever was last installed on the VM by hand. + # Deploy the API. The unit runs `node dist/index.js` out of + # ~/T19-Project-2/server, so that directory has to hold the build of the + # commit we just tested. It is a git clone of main (see #44), so point it + # at $GITHUB_SHA rather than copying files around, then install and build + # there. # - # Two things here are deliberately not what the first version did: + # Deliberately not what earlier versions did: # # - The unit check is `[ -f ... ]`, not `systemctl list-unit-files | grep`. # `grep -q` exits on the first match and closes the pipe, `systemctl` # gets SIGPIPE and dies, and with `pipefail` the whole pipeline reports # failure. So the `if` took the else branch and skipped a service that - # was definitely there. The log showed "Failed to print table: Broken + # was definitely there. The log said "Failed to print table: Broken # pipe". Reported by rachelks. # - # - The restart needs `sudo`, and the runner has no TTY. That needs a + # - The restart needs `sudo` and the runner has no TTY. That needs a # NOPASSWD rule scoped to exactly these commands. Without it the step # dies with "sudo: A terminal is required to authenticate". - - name: Restart the GraphQL API if the server is deployed + # + # - No `sleep`. `Type=simple` reports active before Node binds the port. + # Measured at about 1850 ms, so a fixed `sleep 2` has already produced a + # 503 through Apache. Poll instead. + - name: Deploy the API and restart crate-api run: | set -euo pipefail + DEPLOY_ROOT="${HOME}/T19-Project-2" + if [ ! -f /etc/systemd/system/crate-api.service ]; then - echo "crate-api.service is not installed. Skipping API restart." + echo "crate-api.service is not installed. Skipping API deploy." exit 0 fi if [ ! -d server ]; then - echo "No server/ in this repository yet (arrives in M2). Skipping API restart." + echo "No server/ in this repository yet (arrives with #42). Skipping API deploy." exit 0 fi - if [ ! -f server/dist/index.js ]; then - echo "::error::server/ exists but server/dist/index.js does not. Was 'pnpm build' run?" + if [ ! -d "${DEPLOY_ROOT}/.git" ]; then + echo "::error::${DEPLOY_ROOT} is not a git clone. See #44." + exit 1 + fi + + echo "Deploying ${GITHUB_SHA} to ${DEPLOY_ROOT}" + git -C "${DEPLOY_ROOT}" fetch --quiet origin + # --force so a dirty tree cannot block the deploy. .env is gitignored + # and so survives the checkout; it holds DATABASE_URL. + git -C "${DEPLOY_ROOT}" checkout --force "${GITHUB_SHA}" + echo " at $(git -C "${DEPLOY_ROOT}" rev-parse --short HEAD)" + + if [ ! -f "${DEPLOY_ROOT}/.env" ]; then + echo "::error::${DEPLOY_ROOT}/.env is missing. crate-api will not start without DATABASE_URL." + exit 1 + fi + + pnpm install --dir "${DEPLOY_ROOT}" --frozen-lockfile + pnpm --dir "${DEPLOY_ROOT}" --filter @t19/server build + + if [ ! -f "${DEPLOY_ROOT}/server/dist/index.js" ]; then + echo "::error::server built but server/dist/index.js is missing." exit 1 fi + sudo /usr/bin/systemctl restart crate-api - # `Type=simple` makes systemd report active as soon as the process is - # spawned, well before Node has bound the port. Measured on the VM, - # the API answers again after about 1850 ms, so a fixed `sleep 2` - # leaves a margin of roughly 120 ms. That was enough to return 503 - # through the Apache proxy on at least one run. Poll instead. printf 'Waiting for the API' for _ in $(seq 1 30); do code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 2 \ @@ -161,11 +185,28 @@ jobs: if [ "$code" = "200" ]; then echo echo "POST /graphql : 200" - exit 0 + break fi printf '.' sleep 1 done + if [ "${code:-000}" != "200" ]; then + echo + echo "::error::crate-api restarted but /graphql never returned 200 (last code ${code})." + exit 1 + fi + + # Smoke suite, against the real port and through the same Apache + # proxy the browser uses. This is the acceptance line of #6. echo - echo "::error::crate-api restarted but /graphql never returned 200 (last code ${code})." - exit 1 + echo "Smoke suite:" + curl -fsS -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d '{"query":"{ __typename }"}' \ + -o /dev/null -w ' { __typename } %{http_code}\n' + curl -fsS -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d '{"query":"{ searchAlbums(input:{query:\"\"},first:1){totalCount pageInfo{hasNextPage}} }"}' \ + -o /dev/null -w ' searchAlbums %{http_code}\n' + curl -fsS -o /dev/null -w ' GET /project2/ %{http_code}\n' \ + http://localhost/project2/ From 67baedf92fcfeb9585ebac774a29ef24c0d1df90 Mon Sep 17 00:00:00 2001 From: Erik Hjelm Fjeldheim Date: Mon, 12 Oct 2026 13:42:20 -0600 Subject: [PATCH 2/4] fix: #44 make the smoke suite read the GraphQL body, not just the status rachelks: GraphQL returns HTTP 200 even when the payload carries errors, so a status check proves nothing. She is right and it was demonstrable immediately: querying searchAlbums against the API currently running on the VM returns 200 with {"errors":[{"message":"Unknown argument \"input\" on field \"Query.searchAlbums\".","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}]} and the old check reported 200 and moved on. The smoke suite now POSTs, splits the status line off the body, and rejects the response if it is not 200 or if the body contains an "errors" key. Verified against the live API: __typename passes, searchAlbums is correctly rejected. Branch also rebased onto main so it carries the source-map-js fix from #43. Reported by rachelks. Co-Authored-By: opencode --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe2c7fa..8dead36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,15 +198,36 @@ jobs: # Smoke suite, against the real port and through the same Apache # proxy the browser uses. This is the acceptance line of #6. + # + # GraphQL answers HTTP 200 even when the payload carries errors, so + # the status code alone proves nothing. Every check has to read the + # body and reject an "errors" key. Pointed out by rachelks on #45. echo echo "Smoke suite:" - curl -fsS -X POST http://localhost/graphql \ - -H 'content-type: application/json' \ - -d '{"query":"{ __typename }"}' \ - -o /dev/null -w ' { __typename } %{http_code}\n' - curl -fsS -X POST http://localhost/graphql \ - -H 'content-type: application/json' \ - -d '{"query":"{ searchAlbums(input:{query:\"\"},first:1){totalCount pageInfo{hasNextPage}} }"}' \ - -o /dev/null -w ' searchAlbums %{http_code}\n' - curl -fsS -o /dev/null -w ' GET /project2/ %{http_code}\n' \ - http://localhost/project2/ + check() { + local label="$1" body="$2" resp code json + resp=$(curl -s -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d "${body}" -w '\n%{http_code}') + code="${resp##*$'\n'}" + json="${resp%$'\n'*}" + if [ "${code}" != "200" ]; then + echo "::error::${label}: HTTP ${code} body: ${json}" + return 1 + fi + if printf '%s' "${json}" | grep -q '"errors"'; then + echo "::error::${label}: GraphQL errors body: ${json}" + return 1 + fi + printf ' %-22s %s ok\n' "${label}" "${code}" + } + + check '{ __typename }' '{"query":"{ __typename }"}' + check 'searchAlbums' '{"query":"{ searchAlbums(input:{query:\"\"},first:1){totalCount pageInfo{hasNextPage}} }"}' + + if curl -fsS -o /dev/null http://localhost/project2/; then + echo " GET /project2/ 200 ok" + else + echo "::error::GET /project2/ failed" + exit 1 + fi From cd40e66967779e841235cd6f5822eab42d352ca7 Mon Sep 17 00:00:00 2001 From: Erik Hjelm Fjeldheim Date: Wed, 14 Oct 2026 22:26:26 -0600 Subject: [PATCH 3/4] fix: #44 address review: required sort field, hard-fail deploy, JSON checks Four points from rachelks on #45. 1. The searchAlbums smoke query omitted `sort`, which is `AlbumSort!` in AlbumSearchInput, so the query failed validation and the check could never pass. Now sends `sort: TITLE_ASC`. 2. The skip guards for crate-api.service and server/ now fail instead of exiting 0. #42 has merged so both are expected, and a deploy job that goes green without deploying anything is the wrong failure mode. 3. The body check parses the JSON rather than grepping for the word "errors". Grep would also fire on an album called "Errors", and it did not verify `data` was present at all. Now rejects a response with errors, with no data, or that is not JSON. Added a totalCount assertion as well, so the check is on the data and not only on its presence. 4. The python validators live in shell variables on one line. A multi-line `python3 -c` heredoc cannot be used inside a YAML block scalar: the block ends at the first unindented line and the workflow stops parsing. Learned by breaking it. Also rebased onto main now that #42 and #43 have merged. Validated the two python expressions against five payloads: a good response, GraphQL errors, null data, a valid totalCount, and two bad ones. Each rejected case prints what was wrong. Co-Authored-By: opencode --- .github/workflows/ci.yml | 47 +++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8dead36..66d41b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,13 +141,17 @@ jobs: set -euo pipefail DEPLOY_ROOT="${HOME}/T19-Project-2" + # #42 has landed, so `server/` is expected and the unit is expected + # to exist. These used to skip, which meant the deploy job could go + # green without deploying anything. rachelks on #45: that is the + # wrong failure mode. if [ ! -f /etc/systemd/system/crate-api.service ]; then - echo "crate-api.service is not installed. Skipping API deploy." - exit 0 + echo "::error::crate-api.service is not installed on this machine." + exit 1 fi if [ ! -d server ]; then - echo "No server/ in this repository yet (arrives with #42). Skipping API deploy." - exit 0 + echo "::error::server/ is missing from the checkout." + exit 1 fi if [ ! -d "${DEPLOY_ROOT}/.git" ]; then echo "::error::${DEPLOY_ROOT} is not a git clone. See #44." @@ -200,10 +204,17 @@ jobs: # proxy the browser uses. This is the acceptance line of #6. # # GraphQL answers HTTP 200 even when the payload carries errors, so - # the status code alone proves nothing. Every check has to read the - # body and reject an "errors" key. Pointed out by rachelks on #45. + # the status code alone proves nothing (rachelks on #45). The body is + # parsed as JSON and rejected unless it has a non-null `data` and no + # `errors`, rather than grepping for the word "errors", which would + # also fire on an album called "Errors". + # + # The validators live in shell variables on one line. A multi-line + # python -c heredoc cannot be used here: the YAML block scalar would + # end at the first unindented line and the workflow would not parse. echo echo "Smoke suite:" + CHECK_BODY='import json,sys; p=json.load(sys.stdin); e=p.get("errors"); n=p.get("data"); print(f"errors: {e}" if e else (f"no data: {p}" if n is None else "")); sys.exit(1 if (e or n is None) else 0)' check() { local label="$1" body="$2" resp code json resp=$(curl -s -X POST http://localhost/graphql \ @@ -215,15 +226,31 @@ jobs: echo "::error::${label}: HTTP ${code} body: ${json}" return 1 fi - if printf '%s' "${json}" | grep -q '"errors"'; then - echo "::error::${label}: GraphQL errors body: ${json}" + detail=$(printf '%s' "${json}" | python3 -c "${CHECK_BODY}" 2>&1) || { + echo "::error::${label}: ${detail:-body check failed} body: ${json}" return 1 - fi + } printf ' %-22s %s ok\n' "${label}" "${code}" } check '{ __typename }' '{"query":"{ __typename }"}' - check 'searchAlbums' '{"query":"{ searchAlbums(input:{query:\"\"},first:1){totalCount pageInfo{hasNextPage}} }"}' + # `sort` is `AlbumSort!` in AlbumSearchInput, so leaving it out makes + # the query fail validation. That was a real bug in the first version + # of this check. + check 'searchAlbums' '{"query":"{ searchAlbums(input:{query:\"\",sort:TITLE_ASC},first:1){totalCount pageInfo{hasNextPage}} }"}' + + # One assertion on the data itself, not just its presence. + CHECK_TOTAL='import json,sys; p=json.load(sys.stdin); t=p["data"]["searchAlbums"]["totalCount"]; print(t) if isinstance(t,int) and t>=0 else sys.exit(f"totalCount is not a non-negative integer: {t!r}")' + totals=$(curl -s -X POST http://localhost/graphql \ + -H 'content-type: application/json' \ + -d '{"query":"{ searchAlbums(input:{query:\"\",sort:TITLE_ASC},first:1){totalCount} }"}') + if total=$(printf '%s' "${totals}" | python3 -c "${CHECK_TOTAL}" 2>&1); then + echo " totalCount ${total} albums" + else + echo "::error::totalCount assertion failed: ${total} body: ${totals}" + exit 1 + fi + if curl -fsS -o /dev/null http://localhost/project2/; then echo " GET /project2/ 200 ok" From 4c1ab0df49f388f51349395338c5a7ff921c5629 Mon Sep 17 00:00:00 2001 From: Erik Hjelm Fjeldheim Date: Thu, 15 Oct 2026 13:07:48 -0600 Subject: [PATCH 4/4] fix: #44 apply database migrations on deploy #46 landed migration 0001 for tracks, simulated users, reviews and crate items, and pointed out that this deploy never runs db:migrate. It was right. A deploy that lands new migration files and does not apply them is the same gap as #44 one layer down: the code is current, the schema is not. Migrate runs before the restart, so the new build is never pointed at an old database. It runs on every deploy because db:migrate is idempotent (verified on #46), not only when the migration list changes. DATABASE_URL is exported from the same .env the systemd unit loads. The server package scripts look for `server/.env` instead, which may not exist in the deploy root, so it is passed explicitly rather than assuming the two agree. Node's --env-file does not override a variable already in the environment, so this wins even when server/.env is present. Verified that a URL-encoded password survives the sed extraction and the handoff to the child process byte for byte. Also rebased onto main, which brings #46 along. Co-Authored-By: opencode --- .github/workflows/ci.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66d41b2..8a94485 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -178,6 +178,29 @@ jobs: exit 1 fi + # #5 landed migrations for tracks, simulated users, reviews and crate + # items. A deploy that lands new migration files and never applies + # them is the same gap as #44, one layer down: the code is current + # and the schema is not. Migrate before the restart so the new build + # is never pointed at an old database. + # + # `db:migrate` is idempotent (verified on #46), so this runs on every + # deploy and not only when the migration list changes. + # + # DATABASE_URL comes from the same .env the systemd unit loads. The + # server package scripts look for `server/.env` instead, which may + # not exist here, so export it explicitly rather than assuming the + # two agree. Node's --env-file does not override a variable already + # in the environment, so this wins even if server/.env is present. + echo + echo "Applying database migrations" + db_url="$(sed -n 's/^DATABASE_URL=//p' "${DEPLOY_ROOT}/.env" | tail -n 1)" + if [ -z "${db_url}" ]; then + echo "::error::DATABASE_URL is not set in ${DEPLOY_ROOT}/.env, so migrations cannot run." + exit 1 + fi + DATABASE_URL="${db_url}" pnpm --dir "${DEPLOY_ROOT}" --filter @t19/server db:migrate + sudo /usr/bin/systemctl restart crate-api printf 'Waiting for the API'