Merge branch 'main' into bugfix/327-inventory-stale-after-publish

.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

backend/tests/integration/setup/globalSetup.ts

@@ -23,9 +23,52 @@ async function waitForDb(retries = 20): Promise<void> {
   );
 }
 
+/**
+ * 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<void> {
+  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<void> {
   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:

backend/tests/integration/setup/testDb.ts

@@ -75,21 +75,68 @@ async function missingTables(): Promise<string[]> {
  * 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<string> {
+  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<void> {
   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 ` +
+      `Nothing in this suite drops tables — TRUNCATE does not. Compare the postmaster start ` +
-      `replaced or restarted underneath the run. On CI the likeliest cause is the Postgres ` +
+      `time above against the one globalSetup logged: if they differ, this is a different ` +
-      `service container being recreated, which comes back with an empty data directory. ` +
+      `Postgres instance answering to the same name rather than one database losing its ` +
+      `schema, and the schema was never lost at all. ` +
       `See #154.`
   );
 }

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);
+});