From d7bfd4779783eb62cb6470e3919c640d92ee1acf Mon Sep 17 00:00:00 2001 From: synAdmin Date: Wed, 9 Sep 2026 09:30:36 -0500 Subject: [PATCH 1/2] chore(ci): a manually-run workflow to delete old Actions runs (#324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea 1.27.3 expires a run's logs and artifacts but never the run record itself, so the Actions list grows without limit and fills with entries whose logs are already gone. Clearing it meant 455 API calls from a scratch file on one machine, which is the wrong home for something that has to happen again every few weeks. Dry run unless apply is typed as true. The operation cannot be undone and there is no confirmation once it starts, so the harmless answer has to be the default rather than the one you get by leaving a box alone. Manual only, deliberately. The list is an annoyance rather than a problem, and a cron that quietly deletes history deserves to be a decision taken on its own rather than one that arrives bundled with the tool. A run that is not completed is never a candidate, which is also what stops the cleanup deleting the run it is executing in. Ages a run by started_at, falling back to completed_at. That fallback is the whole reason this is worth committing rather than repeating from memory: a run cancelled before it ever started reports an epoch started_at while carrying a real completed_at, so reading only the first makes every cancelled run look undateable. The manual pass did exactly that and left nineteen runs from three weeks earlier in a list that was supposed to hold seven days. Neither timestamp usable still means keep — an epoch read as 1969 would delete the runs that have not happened yet. Uses a dedicated ACTIONS_CLEANUP_TOKEN secret rather than the automatic per-job token, since deleting a run may be beyond what that token permits. If it turns out to be enough, the secret and the env line both go. Host and repository come from the run's own context, so the file carries no hostname and survives the move #313 may yet make. No npm install: the script uses node's own https module, so there is nothing to fetch and nothing to break when a dependency moves. Verified by running the script against the live instance: a dry run reported 19 stale cancelled runs the earlier pass had missed, and applying it removed them with no failures. Closes #324 Co-Authored-By: Claude Opus 5 --- .gitea/workflows/cleanup-actions.yml | 53 ++++++++ scripts/cleanup-workflow-runs.js | 176 +++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 .gitea/workflows/cleanup-actions.yml create mode 100644 scripts/cleanup-workflow-runs.js diff --git a/.gitea/workflows/cleanup-actions.yml b/.gitea/workflows/cleanup-actions.yml new file mode 100644 index 0000000..3f45a9c --- /dev/null +++ b/.gitea/workflows/cleanup-actions.yml @@ -0,0 +1,53 @@ +name: Clean up old workflow runs + +# Manual only, and dry run by default (#324). +# +# Gitea 1.27.3 expires a run's logs and artifacts but never the run record +# itself, so the Actions list grows without limit and fills with entries whose +# logs are already gone. This removes those entries. +# +# Deliberately not on a schedule. Deleting a run cannot be undone, the list is +# an annoyance rather than a problem, and a cron that quietly removes history +# should be a decision taken on its own rather than the default that arrives +# with the tool. + +on: + workflow_dispatch: + inputs: + keep_days: + description: 'Keep runs newer than this many days' + required: false + default: '7' + apply: + description: 'Type true to actually delete. Anything else reports only.' + required: false + default: 'false' + +jobs: + cleanup: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + + # No npm install: the script uses only node's own https module, so there + # is nothing to fetch and nothing that can break when a dependency moves. + - name: Delete old runs + env: + # A dedicated secret rather than the automatic per-job token, because + # deleting a run may be beyond what that token is allowed to do. If it + # turns out to be sufficient, this and the secret can both go. + GITEA_ACCESS_TOKEN: ${{ secrets.ACTIONS_CLEANUP_TOKEN }} + # Taken from the run's own context so this file carries no hostname + # and works unchanged if the instance ever moves — which #313 may yet + # make happen. + GITEA_HOST: ${{ github.server_url }} + GITEA_REPO: ${{ github.repository }} + KEEP_DAYS: ${{ github.event.inputs.keep_days }} + APPLY: ${{ github.event.inputs.apply }} + run: node scripts/cleanup-workflow-runs.js diff --git a/scripts/cleanup-workflow-runs.js b/scripts/cleanup-workflow-runs.js new file mode 100644 index 0000000..b58b228 --- /dev/null +++ b/scripts/cleanup-workflow-runs.js @@ -0,0 +1,176 @@ +/** + * Deletes completed Actions runs older than a given age. + * + * Gitea 1.27.3 has retention for a run's logs (`LOG_RETENTION_DAYS`) and its + * artifacts, but none for the run record itself — so the Actions list grows + * without limit while the entries on it become empty shells once their logs + * expire. This removes the shells. See #324. + * + * Dry run unless APPLY is exactly "true". Deleting a run cannot be undone and + * there is no confirmation step once this starts, so the default has to be the + * harmless one. + * + * Environment: + * GITEA_HOST origin of the instance, e.g. https://gitea.example.com + * GITEA_REPO "owner/name" + * GITEA_ACCESS_TOKEN token permitted to delete runs + * KEEP_DAYS keep runs newer than this many days (default 7) + * APPLY "true" to delete; anything else reports only + */ + +const https = require('https'); +const { URL } = require('url'); + +const HOST = process.env.GITEA_HOST; +const REPO = process.env.GITEA_REPO; +const TOKEN = process.env.GITEA_ACCESS_TOKEN; +const KEEP_DAYS = Number(process.env.KEEP_DAYS || 7); +const APPLY = process.env.APPLY === 'true'; + +for (const [name, value] of Object.entries({ GITEA_HOST: HOST, GITEA_REPO: REPO, GITEA_ACCESS_TOKEN: TOKEN })) { + if (!value) { + console.error(`${name} is required`); + process.exit(1); + } +} +if (!Number.isFinite(KEEP_DAYS) || KEEP_DAYS < 0) { + console.error(`KEEP_DAYS must be a non-negative number, got ${process.env.KEEP_DAYS}`); + process.exit(1); +} + +const origin = new URL(HOST); +const BASE = `/api/v1/repos/${REPO}/actions/runs`; + +function call(method, path) { + return new Promise((resolve, reject) => { + const req = https.request( + { hostname: origin.hostname, port: origin.port || 443, path, method, headers: { Authorization: `token ${TOKEN}` } }, + (res) => { + let body = ''; + res.on('data', (d) => (body += d)); + res.on('end', () => resolve({ status: res.statusCode, body })); + } + ); + req.on('error', reject); + req.end(); + }); +} + +async function listAllRuns() { + const runs = []; + // Bounded rather than `while (true)`: a paging bug against an API that keeps + // answering would otherwise loop until the job times out. + for (let page = 1; page <= 200; page++) { + const res = await call('GET', `${BASE}?page=${page}&limit=50`); + if (res.status >= 400) throw new Error(`listing runs failed: ${res.status} ${res.body.slice(0, 200)}`); + const batch = JSON.parse(res.body).workflow_runs || []; + if (batch.length === 0) break; + runs.push(...batch); + if (batch.length < 50) break; + } + return runs; +} + +const EPOCH_GUARD = new Date('2000-01-01').getTime(); + +/** + * When a run happened, from whichever timestamp it actually has. + * + * `started_at` is preferred and is usually right, but a run **cancelled before + * it ever started** reports an epoch value there while carrying a real + * `completed_at`. Reading only `started_at` therefore made every cancelled run + * look undateable, and a first pass at this left 19 of them — from three weeks + * earlier — sitting in a list that was supposed to hold seven days. + * + * Returns null when neither timestamp is usable, which is the case that must + * stay conservative: an epoch date treated as "1969, therefore old" would + * delete precisely the runs that have not happened yet. + */ +function runTimeMs(run) { + for (const stamp of [run.started_at, run.completed_at]) { + const ms = stamp ? new Date(stamp).getTime() : NaN; + if (Number.isFinite(ms) && ms >= EPOCH_GUARD) return ms; + } + return null; +} + +/** + * Runs old enough to remove, and the reasons the others were left. + * + * A run that is not completed is never a candidate — which is also what stops + * this deleting the very run it is executing in. + */ +function selectDoomed(runs, cutoffMs) { + const doomed = []; + const kept = { recent: 0, unfinished: 0, undated: 0 }; + + for (const run of runs) { + if (run.status !== 'completed') { + kept.unfinished++; + continue; + } + const ms = runTimeMs(run); + if (ms === null) { + kept.undated++; + continue; + } + if (ms >= cutoffMs) { + kept.recent++; + continue; + } + doomed.push({ id: run.id, startedAt: run.started_at, when: new Date(ms).toISOString() }); + } + + doomed.sort((a, b) => a.id - b.id); + return { doomed, kept }; +} + +(async () => { + const runs = await listAllRuns(); + const cutoffMs = Date.now() - KEEP_DAYS * 86400000; + const { doomed, kept } = selectDoomed(runs, cutoffMs); + + console.log(`repository : ${REPO}`); + console.log(`total runs : ${runs.length}`); + console.log(`keeping : ${kept.recent} newer than ${KEEP_DAYS}d, ${kept.unfinished} unfinished, ${kept.undated} undated`); + console.log(`to delete : ${doomed.length}`); + if (doomed.length > 0) { + console.log(` oldest : run ${doomed[0].id} (${doomed[0].when})`); + console.log(` newest : run ${doomed[doomed.length - 1].id} (${doomed[doomed.length - 1].when})`); + } + console.log(''); + + if (!APPLY) { + console.log('DRY RUN — nothing deleted. Re-run with apply set to "true".'); + return; + } + if (doomed.length === 0) { + console.log('Nothing to delete.'); + return; + } + + let deleted = 0; + const failures = []; + for (const run of doomed) { + const res = await call('DELETE', `${BASE}/${run.id}`); + if (res.status >= 200 && res.status < 300) deleted++; + else failures.push(`${run.id}:${res.status}`); + + const done = deleted + failures.length; + if (done % 50 === 0) console.log(` ...${done}/${doomed.length}`); + } + + console.log(''); + console.log(`deleted : ${deleted}`); + console.log(`failed : ${failures.length}`); + if (failures.length > 0) { + console.log(` ${failures.slice(0, 20).join(' ')}${failures.length > 20 ? ' …' : ''}`); + // A partial delete is not a success. Most likely the token cannot delete + // runs, and reporting green here would hide that behind a job that + // appeared to work. + process.exitCode = 1; + } +})().catch((err) => { + console.error(`FAILED: ${err.message}`); + process.exit(1); +}); From 2bc9440b3828f3d24f0a66fde67ae559926c9a23 Mon Sep 17 00:00:00 2001 From: synAdmin Date: Wed, 9 Sep 2026 10:35:18 -0500 Subject: [PATCH 2/2] test(ci): record which Postgres actually answered (#154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds the measurement #154 has needed twice and never had. It does not fix the failure and does not guess at it: the issue has been wrong twice from reasoning ahead of evidence, and the point here is to make the next occurrence answer the question rather than reopen it. The observation that rules out every explanation so far is that the schema comes back. A suite fails because orders does not exist, and a later suite truncating that same table passes. A dropped database does not un-drop itself, so this was never one database losing its schema. More than one server answering to one name produces exactly this, and Docker embedded DNS round-robins every container sharing an alias, so a leftover service container from an earlier run fits every observation including the empty dmesg that killed the OOM theory. globalSetup now logs every address the database host resolves to. More than one is the answer outright. One address means this reading is wrong too, and the next suspect is a single container restarted with a fresh data directory. Alongside it, both globalSetup and a failing assertSchemaPresent record which server actually answered. pg_postmaster_start_time is what settles that and needs no special rights: two Postgres instances cannot share one, so differing values within a single run are proof, where a differing inet_server_addr alone could be argued to be one container that moved. The failure message now says to compare the two rather than leaving the reader to know that is the interesting comparison. Logged on a passing run as well as a failing one, deliberately. A failing run's addresses mean nothing without a passing run's to compare them against, and this issue has twice suffered from having only the failure to look at. Neither can throw. A diagnostic that fails the run it was added to explain is worse than no diagnostic, so both are wrapped and both degrade to a printed reason. The failure path costs one extra round trip, taken only when the schema is already known to be missing. assertSchemaPresent is not on the hot path — resetDb calls it only when its TRUNCATE has already failed. Verified: tsc clean, typecheck:tests clean, lint 0 errors with no new warnings, and the four message patterns schemaLoss.integration.test.ts asserts on are all still present. The probe reads only pg_catalog functions, so it still answers against the dropped schema that suite creates. Refs #154 Co-Authored-By: Claude Opus 5 --- .../tests/integration/setup/globalSetup.ts | 45 +++++++++++++++- backend/tests/integration/setup/testDb.ts | 53 +++++++++++++++++-- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/backend/tests/integration/setup/globalSetup.ts b/backend/tests/integration/setup/globalSetup.ts index 83ff035..fbdd4c6 100755 --- a/backend/tests/integration/setup/globalSetup.ts +++ b/backend/tests/integration/setup/globalSetup.ts @@ -23,9 +23,52 @@ async function waitForDb(retries = 20): Promise { ); } +/** + * Every address the database host resolves to, and which server answered (#154). + * + * This is the measurement that issue has needed twice and never had. The + * schema in a failing run comes back — a suite fails on a missing `orders`, + * and a later suite truncating that same table passes — which a database + * losing its schema cannot do. More than one server answering to one name can, + * and Docker's embedded DNS round-robins every container sharing an alias, so + * a leftover service container from an earlier run fits every observation + * including the empty `dmesg`. + * + * **More than one address printed here is the answer outright.** One address, + * and this reading is wrong too — the next suspect is a single container being + * restarted with a fresh data directory, which the postmaster start time + * recorded alongside will show. + * + * Logged unconditionally rather than only on failure: a passing run's addresses + * are the control, and without them a failing run's have nothing to be compared + * against. Never throws — a diagnostic that can fail the run it was added to + * explain is worse than none. + */ +async function reportDatabaseIdentity(): Promise { + const host = process.env.TEST_PGHOST || 'localhost'; + try { + const { lookup } = await import('dns/promises'); + const addresses = await lookup(host, { all: true }); + const rendered = addresses.map((a) => a.address).join(', '); + console.info( + `[#154] "${host}" resolves to ${addresses.length} address(es): ${rendered}` + + (addresses.length > 1 ? ' <-- more than one server can answer; this is the bug' : '') + ); + } catch (err) { + console.info(`[#154] could not resolve "${host}": ${err instanceof Error ? err.message : String(err)}`); + } +} + export default async function globalSetup(): Promise { await waitForDb(); - const { migrate, closeDb, assertSchemaPresent } = await import('./testDb'); + const { migrate, closeDb, assertSchemaPresent, describeBackend } = await import('./testDb'); + + await reportDatabaseIdentity(); + // Recorded before migrating so the run's baseline is in the log even if the + // migration is the thing that fails. A suite that later reports a different + // postmaster start time is talking to a different instance. + console.info(`[#154] ${await describeBackend('globalSetup reached')}`); + await migrate(); // Cheap, once, and it establishes the fact the rest of the run depends on: diff --git a/backend/tests/integration/setup/testDb.ts b/backend/tests/integration/setup/testDb.ts index 98018a5..bf30b09 100755 --- a/backend/tests/integration/setup/testDb.ts +++ b/backend/tests/integration/setup/testDb.ts @@ -75,21 +75,68 @@ async function missingTables(): Promise { * can be fixed from here: whatever the cause, the next occurrence should read as * "the database lost its schema" on the first line. */ +/** + * Which Postgres actually answered, rather than which one we asked for (#154). + * + * The reason this exists: the schema in a failing run **comes back**. A suite + * fails because `orders` does not exist, and a later suite truncating the same + * table passes. A dropped database does not un-drop itself, so more than one + * server must be answering to the same name — Docker's embedded DNS round-robins + * every container sharing an alias, so a leftover service container from an + * earlier run would produce exactly this. + * + * `pg_postmaster_start_time()` is what settles it and needs no special rights: + * two Postgres instances cannot share one. Differing values across a single run + * are proof outright, where a differing `inet_server_addr` alone could be argued + * to be one container that moved. + * + * Never throws. This is a diagnostic, and a diagnostic that can fail a run it + * was added to explain is worse than no diagnostic. + */ +export async function describeBackend(label: string): Promise { + try { + const { rows } = await testPool.query<{ + addr: string | null; + started: string; + pid: number; + db: string; + }>( + `SELECT inet_server_addr()::text AS addr, + pg_postmaster_start_time()::text AS started, + pg_backend_pid() AS pid, + current_database() AS db` + ); + const row = rows[0]; + if (!row) return `${label}: no row returned`; + return `${label}: db=${row.db} addr=${row.addr ?? 'local'} postmaster_start=${row.started} pid=${row.pid}`; + } catch (err) { + return `${label}: could not be identified (${err instanceof Error ? err.message : String(err)})`; + } +} + export async function assertSchemaPresent(context: string): Promise { const missing = await missingTables(); if (missing.length === 0) return; + // Gathered only on the failure path, which is the one worth paying for, and + // is where #154 has repeatedly lacked the one fact that would identify it. + const backend = await describeBackend('Answering server'); + throw new Error( `The test database has no schema (${context}). ` + `Missing ${missing.length} of ${REQUIRED_TABLES.length} tables: ${missing.join(', ')}. +` + + `${backend} + ` + `Migrations ran at the start of this run, so the schema existed and has since gone. ` + - `Nothing in this suite drops tables — TRUNCATE does not — so the database itself was ` + - `replaced or restarted underneath the run. On CI the likeliest cause is the Postgres ` + - `service container being recreated, which comes back with an empty data directory. ` + + `Nothing in this suite drops tables — TRUNCATE does not. Compare the postmaster start ` + + `time above against the one globalSetup logged: if they differ, this is a different ` + + `Postgres instance answering to the same name rather than one database losing its ` + + `schema, and the schema was never lost at all. ` + `See #154.` ); }