fix: git sync deadlock + in-process task timeout + puppeteer version mismatch

.devcontainer/Dockerfile

@@ -24,20 +24,10 @@ RUN apt-get update && apt-get install -qy \
     unzip \
     wget
 
-# Chromium, for the Puppeteer extension. Rendering a page on the server means driving a headless
+# The Puppeteer extension installs its own browser, in app-init.sh: the one Chrome release it is built
-# browser, and this is the copy Puppeteer is pointed at rather than one it downloads for itself:
+# against, where a distro Chromium moves on with every update and eventually stops being one Puppeteer
-# the distro keeps it patched, and it exists for arm64 as well as amd64.
+# can drive. Only the headless shell, which is all the renderer uses.
-#
+ENV PUPPETEER_CHROME_SKIP_DOWNLOAD=true
-# Its own RUN, and without recommends, because Chromium recommends a desktop stack -- an X server, a
-# terminal emulator, usbmuxd -- that takes the package count from 61 to 177 and that nothing headless
-# ever touches. The apt lists from the install above are still around, so no second update is needed.
-RUN apt-get install -qy --no-install-recommends \
-    chromium \
-    fonts-liberation
-
-# Where Puppeteer looks for a browser, and that it must not fetch a second one on install
-ENV PUPPETEER_SKIP_DOWNLOAD=true
-ENV PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium
 
 # avoid million NPM install messages
 ENV npm_config_loglevel=warn

.devcontainer/app-init.sh

@@ -20,10 +20,19 @@ npm install
 # and a plain source checkout should not have to fetch it to install the backend.
 #
 # `--no-save` leaves package.json alone, which also means a later reinstall can prune it and turn
-# server-side rendering back off -- run this line again if the admin area says it is missing. The
+# server-side rendering back off -- run this block again if the admin area says it is missing. The
-# browser itself is in the image, so this fetches no Chromium (see PUPPETEER_* in the Dockerfile).
+# version is the extension definition's, as in the production image.
+#
+# Then the browser, which `browsers install` fetches if the postinstall did not, and the system
+# libraries it lists for itself in `deb.deps` -- the `apt-get satisfy` that `--install-deps` runs, but
+# under sudo alone rather than with Puppeteer running as root.
 echo "Installing the Puppeteer extension..."
-npm install --no-save puppeteer@25.4.0
+PUPPETEER_VERSION="$(sed -n 's/^installVersion: *//p' modules/extensions/puppeteer/definition.yml)"
+npm install --no-save "puppeteer@${PUPPETEER_VERSION}"
+PUPPETEER_BROWSER="$(npx puppeteer browsers install chrome-headless-shell --format '{{path}}')"
+sudo apt-get update -qq
+sudo DEBIAN_FRONTEND=noninteractive apt-get satisfy -qy --no-install-recommends \
+  "$(paste -sd, "$(dirname "$PUPPETEER_BROWSER")/deb.deps")"
 
 cd ../frontend
 npm install

CLAUDE.md

@@ -84,8 +84,13 @@ path in silence.
   `modules/authentication/local/`. `modules/storage/*` ships `db` and `disk` — see
   [Storage targets](#storage-targets). `modules/analytics/*` is the odd one out: a pair of YAML files
   and no implementation at all — see [Analytics](#analytics).
-- `tasks/simple/` — jobs run in-process by the scheduler; each exports `task()`. File name is
+- `tasks/simple/` — jobs run in-process by the scheduler; each exports `task(payload, { signal })`.
-  kebab-case, the task key is its camelCase form.
+  File name is kebab-case, the task key is its camelCase form. `scheduler.taskTimeout` applies here
+  as it does to workers, but a promise cannot be killed: at the timeout `signal` is aborted, and a
+  task that works in steps checks it between them and stops (`renderPages` hands the rest of its
+  queue to a fresh job, so a long re-render runs in slices). One that has not settled a minute later
+  is abandoned — failed, left running, and no second copy of that task starts on the instance until
+  it ends (`executeInProcess` in `core/scheduler.ts`).
 - `tasks/workers/` — CPU-bound jobs run in a worker thread via `worker.ts`, which boots a minimal
   `WIKI` global (config + logger + lazy `ensureDb()`) and dynamically imports the task.
 - `base.yml` — system defaults for every config key. Do not edit as a user-facing config; it defines
@@ -270,8 +275,8 @@ Match the check to the size of the change. `npm run build`, `npx oxlint` and `np
 seconds each and are the right check for nearly everything.
 
 **Do not stand up a throwaway instance and drive a headless browser to look at a small change.** That
-means booting a backend against a scratch database, seeding it, and screenshotting through
+means booting a backend against a scratch database, seeding it, and screenshotting through a
-`/usr/bin/chromium` — a good ten minutes of setup that a moved border, a colour, a spacing tweak or a
+headless browser — a good ten minutes of setup that a moved border, a colour, a spacing tweak or a
 renamed label does not earn. Read the rule you wrote, trust the build, and say what you changed.
 
 It is worth the setup for a **new** piece of UI whose markup has to meet a stylesheet written
@@ -296,9 +301,12 @@ Then `CONFIG_FILE=config.test.yml node --no-experimental-webstorage backend` **f
 `CONFIG_FILE` is resolved against `WIKI.ROOTPATH` (`core/config.ts`), so it is a path relative to the
 root and not to `backend/`. It seeds itself and takes ~25s to reach listening.
 
-**Puppeteer is not installed in any workspace, and must not be added to one for a screenshot.** Install
+**Puppeteer must not be added to a workspace's `package.json` for a screenshot.** Install
-`puppeteer-core` into a scratch directory instead and drive the browser already on the box:
+`puppeteer-core` into a scratch directory instead and drive the browser already on the box: the
-`executablePath: '/usr/bin/chromium'`, `args: ['--no-sandbox']`. It pulls ~25 packages and downloads no
+headless shell the dev container's Puppeteer extension installed, whose path
+`npx puppeteer browsers install chrome-headless-shell --format '{{path}}'` prints from `backend/`
+(it downloads nothing when the browser is there). Launch with that as `executablePath`,
+`headless: 'shell'` and `args: ['--no-sandbox']`; `puppeteer-core` pulls ~25 packages and downloads no
 browser of its own.
 
 **Scripting the API rather than the browser**, which is the quicker way to get a page and a history in
@@ -880,6 +888,13 @@ Everything else follows from that:
   dispatches the new bytes to every write target, since the copy a reader is served is usually the
   database's. Imported content is rendered with **no script or style permission** whoever ran the
   import, since the file need not have been written by them.
+- **An import is never written back to the target it reads from.** What it adopts is saved like any
+  edit and so copied to every target holding it, and `git` and `sftp` serialize every operation per
+  target — with the import itself being one, a copy back to the source waits on the import that is
+  waiting on it. `importTree` runs under `storage.importingFrom(target)`, an `AsyncLocalStorage`
+  scope that `putAsset`, `eachPageTarget` and `eachAssetTarget` skip that target in (counting it,
+  for `putAsset`, as somewhere the file is held). Anything else a module does to the wiki while
+  holding its own queue — git's removals in `applyIncoming` — needs the same scope.
 - **On import a file is a page if its extension is reserved, or if it declares an `editor`** in its
   front matter. A text page is front matter plus the source; a **JSON** page — a redirection today —
   is one JSON document with the metadata at its top level and the source under `content`.

backend/base.yml

@@ -45,9 +45,11 @@ defaults:
       maxRetries: 2
       retryBackoff: 60
       historyExpiration: 90000
-      # How long, in seconds, a job running in a worker thread may take before the scheduler stops
+      # How long, in seconds, a job may take before the scheduler stops waiting for it and counts it
-      # waiting for it and counts it as failed. A safety net rather than a schedule: a worker thread
+      # as failed. A safety net rather than a schedule: a worker thread that dies mid-task never
-      # that dies mid-task never answers at all, and without a ceiling the scheduler waits forever.
+      # answers at all, and an in-process task stuck on a lock or a dead browser never returns, and
+      # without a ceiling the scheduler waits forever. A worker is aborted at this point; an
+      # in-process task is asked to stop and, if it has not a minute later, abandoned.
       taskTimeout: 300
       # How long, in seconds, a job may sit marked as running before it is assumed that whatever was
       # running it is gone, and it is requeued. Deliberately generous: it has to outlast the longest

backend/core/scheduler.ts

@@ -17,8 +17,18 @@ import {
 import { and, eq, inArray, lt, sql } from 'drizzle-orm'
 import type { PoolClient } from 'pg'
 
+/** What the scheduler hands an in-process task besides its payload. */
+export interface TaskContext {
+  /**
+   * Aborted when the task has run for `scheduler.taskTimeout`. A task cannot be stopped from outside
+   * the way a worker thread can, so this is a request: one that does its work in steps should check
+   * it between them and stop, and one that ignores it is abandoned — see `executeInProcess`.
+   */
+  signal: AbortSignal
+}
+
 /** An in-process task, loaded from `tasks/simple/`. */
-export type SimpleTask = (payload?: any) => Promise<void> | void
+export type SimpleTask = (payload: any, context: TaskContext) => Promise<void> | void
 
 /** Fallback for `scheduler.taskTimeout`, in seconds, when nothing is configured. */
 const DEFAULT_TASK_TIMEOUT = 300
@@ -35,6 +45,25 @@ const DEFAULT_STALE_JOB_TIMEOUT = 3600
  */
 const TASK_TIMEOUT_GRACE = 5000
 
+/**
+ * How long an in-process task that has been asked to stop gets to do so before it is abandoned.
+ *
+ * Far longer than the worker's grace, because stopping here is cooperative: a task stops at the end
+ * of the step it is in — the page it is rendering, the target it is syncing — rather than the moment
+ * the signal fires, and one of those steps may itself take half a minute.
+ */
+const IN_PROCESS_STOP_GRACE = 60_000
+
+/**
+ * In-process tasks that timed out without stopping, and when they were abandoned.
+ *
+ * Such a task is still running and nothing can end it, so no second copy of it is started on this
+ * instance until it settles. A task hangs on something — a lock, a connection, a dead browser — and a
+ * second copy almost always hangs on the same thing, each one taking a slot for the length of the
+ * timeout; a task scheduled every minute would fill the pool within a few of them.
+ */
+const abandonedTasks = new Map<string, number>()
+
 /**
  * Sends the scheduler's cross-instance notifications, one at a time.
  *
@@ -267,6 +296,67 @@ export default {
     }
   },
 
+  /**
+   * Run a job in this process, and stop waiting for it if it does not come back.
+   *
+   * The counterpart of `executeOnWorker`, and without it one task that never settles is fatal rather
+   * than slow: its slot is never given back, its history row says `active` for ever, and a task that
+   * is scheduled keeps queuing copies that wait on the same thing.
+   *
+   * There is no thread to abort, so the two ceilings mean something different here. At the timeout the
+   * task's signal is aborted, which a task that works in steps answers by stopping at the end of the
+   * current one. If it has not settled a grace period later it is abandoned: recorded as failed like a
+   * worker that never answered, and left running, since nothing in JavaScript can stop a promise.
+   */
+  async executeInProcess(job: { task: string; payload?: any }): Promise<void> {
+    const abandonedAt = abandonedTasks.get(job.task)
+    if (abandonedAt !== undefined) {
+      throw new Error(
+        `An earlier run of ${job.task} timed out ${Math.round((Temporal.Now.instant().epochMilliseconds - abandonedAt) / 1000)}s ago and is still running, so another is not being started alongside it.`
+      )
+    }
+
+    const timeoutSeconds = WIKI.config.scheduler.taskTimeout ?? DEFAULT_TASK_TIMEOUT
+    const controller = new AbortController()
+    const stopTimer = setTimeout(() => {
+      controller.abort(
+        new Error(`The task did not finish within ${timeoutSeconds}s and was asked to stop.`)
+      )
+    }, timeoutSeconds * 1000)
+    let giveUpTimer: NodeJS.Timeout | undefined
+
+    const run = Promise.resolve().then(() =>
+      this.tasks![job.task](job.payload, { signal: controller.signal })
+    )
+    const abandon = () => {
+      abandonedTasks.set(job.task, Temporal.Now.instant().epochMilliseconds)
+      // -> Also what keeps a late rejection from surfacing as an unhandled one
+      void run
+        .then(
+          () => WIKI.logger.warn(`The abandoned run of ${job.task} has finished after all.`),
+          (err: any) => WIKI.logger.warn(`The abandoned run of ${job.task} failed: ${err.message}`)
+        )
+        .finally(() => abandonedTasks.delete(job.task))
+      return new Error(
+        `The task did not finish within ${timeoutSeconds}s, nor stop within ${IN_PROCESS_STOP_GRACE / 1000}s of being asked to. It is still running and cannot be stopped from here; no other run of it will start on this instance until it ends.`
+      )
+    }
+    try {
+      await Promise.race([
+        run,
+        new Promise<never>((_resolve, reject) => {
+          giveUpTimer = setTimeout(
+            () => reject(abandon()),
+            timeoutSeconds * 1000 + IN_PROCESS_STOP_GRACE
+          )
+        })
+      ])
+    } finally {
+      clearTimeout(stopTimer)
+      clearTimeout(giveUpTimer)
+    }
+  },
+
   /**
    * Take a batch of due jobs and run them.
    *
@@ -359,7 +449,7 @@ export default {
       if (job.useWorker) {
         await this.executeOnWorker(job)
       } else {
-        await this.tasks![job.task](job.payload)
+        await this.executeInProcess(job)
       }
       await WIKI.db
         .update(jobHistoryTable)

backend/helpers/storageFiles.ts

@@ -400,21 +400,30 @@ export interface ImportSummary {
  * @param readFile How to read one, for a tree that is not on this machine — the SFTP target hands in
  *   its own and everything else about the walk is the same
  */
-export async function importTree({
+export function importTree(options: ImportTreeOptions): Promise<ImportSummary | null> {
-  target,
+  // -> What is adopted here is saved like any edit, and so copied to every target holding it. Not to
-  root,
+  //    this one: the file is already there, and a module that serializes its operations would have
-  actorId,
+  //    the copy wait for this import to finish while the import waits for the copy.
-  overwrite,
+  return WIKI.models.storage.importingFrom(options.target, () => adoptTree(options))
-  files,
+}
-  readFile = (filePath) => fs.readFile(filePath)
+
-}: {
+interface ImportTreeOptions {
   target: StorageTarget
   root: string
   actorId: string
   overwrite: boolean
   files?: StoredFile[] | null
   readFile?: (filePath: string) => Promise<Buffer>
-}): Promise<ImportSummary | null> {
+}
+
+async function adoptTree({
+  target,
+  root,
+  actorId,
+  overwrite,
+  files,
+  readFile = (filePath) => fs.readFile(filePath)
+}: ImportTreeOptions): Promise<ImportSummary | null> {
   const found = files === undefined ? await walkStored(root) : files
   if (!found) {
     return null

backend/models/extensions.ts

@@ -226,11 +226,12 @@ class Extensions {
    *
    * Puppeteer is not declared anywhere, so this is a genuine first install, and the bulk of it is the
    * browser. Nothing has to be arranged for that: Puppeteer's own postinstall fetches one into its
-   * cache, which is the ordinary case and the one an install straight onto Linux takes. A server that
+   * cache, which is the ordinary case, and the Docker image does the same at build time into a fixed
-   * already has a browser opts out with `PUPPETEER_SKIP_DOWNLOAD` and points at it with
+   * `PUPPETEER_CACHE_DIR`. A server that already has a browser can opt out with
-   * `PUPPETEER_EXECUTABLE_PATH` — what the Docker image does with the Chromium it takes from the
+   * `PUPPETEER_SKIP_DOWNLOAD` and point at it with `PUPPETEER_EXECUTABLE_PATH`, but it has to be the
-   * distro. Neither is required, and neither is set here: npm inherits this process's environment, so
+   * Chrome release this Puppeteer was built for, which a distro package does not stay. None of these
-   * an install from the admin area sees exactly what the operator set for the server and nothing else.
+   * is set here: npm inherits this process's environment, so an install from the admin area sees
+   * exactly what the operator set for the server and nothing else.
    *
    * Hence the flags:
    *

backend/models/rendering.ts

@@ -1018,8 +1018,13 @@ class Rendering {
    * browser. It asks the one already going to look again before it stops, which is what stops a page
    * queued in the moment between the last claim and the end of the drain from waiting for the next
    * request to come along.
+   *
+   * **A drain runs in slices of the scheduler's task timeout.** Re-rendering a whole wiki can take far
+   * longer than that, which is not the drain being stuck, so `signal` is answered by stopping after
+   * the page in hand and handing what is still queued to a fresh job — queued once this drain has let
+   * go of the flag, or the new one would join it just as it was finishing.
    */
-  async drainQueue(): Promise<void> {
+  async drainQueue(signal?: AbortSignal): Promise<void> {
     if (this.draining) {
       this.drainRequested = true
       return
@@ -1028,11 +1033,15 @@ class Rendering {
     try {
       do {
         this.drainRequested = false
-        await this.renderQueuedPages()
+        await this.renderQueuedPages(signal)
-      } while (this.drainRequested)
+      } while (this.drainRequested && !signal?.aborted)
     } finally {
       this.draining = false
     }
+    if (signal?.aborted) {
+      // -> Whether or not anything is left: a job that finds the queue empty costs one query
+      await WIKI.scheduler.addJob({ task: DRAIN_TASK, maxRetries: 0 })
+    }
   }
 
   /** True while `drainQueue` is working, so that a second call joins it instead of duplicating it. */
@@ -1053,7 +1062,7 @@ class Rendering {
    * out of time, which leaves a page wedged in whatever loop it was in, and the pages behind it in the
    * queue have done nothing to deserve that.
    */
-  private async renderQueuedPages(): Promise<void> {
+  private async renderQueuedPages(signal?: AbortSignal): Promise<void> {
     // -> Asked before anything else so that the common drain — a spare job for a batch already swept —
     //    costs one query and says nothing
     const waiting = await WIKI.db
@@ -1072,7 +1081,7 @@ class Rendering {
 
     let renderer: PageRenderer | null = null
     try {
-      while (true) {
+      while (!signal?.aborted) {
         /*
           Deliberately outside the per-page catch below, and ahead of the claim: a browser that will
           not open is not this page's fault and will not be the next one's either. Letting that throw
@@ -1176,9 +1185,14 @@ class Rendering {
     }
 
     const browser = await puppeteer.launch({
-      headless: true,
+      // -> The headless shell: all a renderer needs, and the build the Docker image installs
+      headless: 'shell',
       args: ['--no-sandbox', '--disable-dev-shm-usage']
     })
+    const mismatch = await describeVersionMismatch(browser)
+    if (mismatch) {
+      WIKI.logger.warn(mismatch)
+    }
     try {
       const page = await browser.newPage()
       // -> A shell page whose only job is to load the frontend's renderer bundle. It is served by this
@@ -1242,9 +1256,44 @@ class Rendering {
       try {
         await browser.close()
       } catch {}
+      // -> A protocol mismatch fails as whichever step trips over it first, in words that name
+      //    neither version — `Requesting main frame too early!` from the navigation, typically
+      if (mismatch) {
+        err.message = `${err.message} - ${mismatch}`
+      }
       throw err
     }
   }
 }
 
+/**
+ * Why the browser Puppeteer launched may not be one it can drive, or null when nothing says so.
+ *
+ * Puppeteer speaks the DevTools protocol of the one Chrome release it was built against, and a browser
+ * some way from it fails without saying why. Puppeteer never picks a mismatched browser by itself: the
+ * way to get one is `PUPPETEER_EXECUTABLE_PATH` pointing at a system Chromium that has since moved on.
+ *
+ * Major versions only, since a patch release either side is routine. The expected version is read from
+ * Puppeteer's internal revisions module, as its own `browsers install` command reads it, because there
+ * is no public API for it — so if that has moved, the check is skipped rather than failing a render.
+ */
+async function describeVersionMismatch(browser: any): Promise<string | null> {
+  try {
+    const specifier = 'puppeteer-core/internal/revisions.js'
+    const { PUPPETEER_REVISIONS } = await import(specifier)
+    const expected: string = PUPPETEER_REVISIONS['chrome-headless-shell']
+    // -> `HeadlessChrome/154.0.8037.57`, or `Chrome/…` for a full browser
+    const actual: string = (await browser.version()).split('/').pop()
+    if (!expected || !actual || expected.split('.')[0] === actual.split('.')[0]) {
+      return null
+    }
+    const source = process.env.PUPPETEER_EXECUTABLE_PATH
+      ? ` It was launched from PUPPETEER_EXECUTABLE_PATH (${process.env.PUPPETEER_EXECUTABLE_PATH}): unset that to use the browser Puppeteer installs for itself, or install the Puppeteer release built for this Chrome.`
+      : ''
+    return `Puppeteer is built for Chrome ${expected}, but the browser it launched is ${actual}, and may not be one it can drive.${source}`
+  } catch {
+    return null
+  }
+}
+
 export const rendering = new Rendering()

backend/models/storage.ts

@@ -1,3 +1,4 @@
+import { AsyncLocalStorage } from 'node:async_hooks'
 import fs from 'node:fs/promises'
 import path from 'node:path'
 import { load } from 'js-yaml'
@@ -25,6 +26,9 @@ export type ContentType = (typeof CONTENT_TYPES)[number]
  */
 const DB_MODULE = 'db'
 
+/** The target an import in progress is reading from. See `Storage.importingFrom`. */
+const importSource = new AsyncLocalStorage<string>()
+
 /** Which content type an asset falls into when it is not large enough to count as a large file. */
 const CONTENT_TYPE_BY_KIND: Record<AssetKind, ContentType> = {
   image: 'images',
@@ -1330,6 +1334,28 @@ class Storage {
     return source ? [source, ...targets.filter((t) => t.id !== source.id)] : targets
   }
 
+  /**
+   * Run an import from this target without writing what it takes in back to it.
+   *
+   * Everything an import adopts is saved the way an editor saves it, and a save is copied to every
+   * target holding that kind of content — the one it was read from included. That copy is worse than
+   * redundant: the git and SFTP modules serialize every operation on a target, and an import runs as
+   * one of those operations, so a write back to the same target waits for the import to finish while
+   * the import waits for the write. The file is already there, which is where it was read from.
+   *
+   * Carried by `AsyncLocalStorage` rather than a parameter because the save that dispatches is several
+   * models away from the import that caused it, through `createPage`, `updatePage`, `deletePage` and
+   * the asset model, none of which should have to know an import is happening.
+   */
+  importingFrom<T>(target: StorageTarget, work: () => Promise<T>): Promise<T> {
+    return importSource.run(target.id, work)
+  }
+
+  /** Whether this target is the one an import in progress is reading from. */
+  private isImportSource(target: StorageTarget): boolean {
+    return importSource.getStore() === target.id
+  }
+
   /**
    * Write an asset's bytes to every target that holds its kind, and that has somewhere to put them.
    *
@@ -1372,6 +1398,9 @@ class Storage {
     if (accepting.length < 1) {
       throw this.unstorableAssetError(ref, contentType, declining)
     }
+    // -> Counted above as somewhere the file can be held, which it is: it was read from there. It
+    //    is only not written to again — see `importingFrom`.
+    const writing = accepting.filter(({ target }) => !this.isImportSource(target))
     if (declining.length > 0) {
       // -> Not a warning on the target: it did what the site configured it to do
       WIKI.logger.debug(
@@ -1379,7 +1408,7 @@ class Storage {
       )
     }
 
-    for (const { target, mod } of accepting) {
+    for (const { target, mod } of writing) {
       try {
         await mod.putAsset(target, ref, data)
       } catch (err: any) {
@@ -1596,6 +1625,9 @@ class Storage {
     run: (mod: StorageModule, target: StorageTarget) => Promise<void>
   ): Promise<void> {
     for (const target of await this.writeTargetsFor(ref.siteId, ref.kind, ref.fileSize)) {
+      if (this.isImportSource(target)) {
+        continue
+      }
       const mod = await this.ensureModule(target.module)
       if (!mod) {
         continue
@@ -1654,6 +1686,9 @@ class Storage {
     run: (mod: StorageModule, target: StorageTarget) => Promise<void>
   ): Promise<void> {
     for (const target of await this.pageTargets(siteId)) {
+      if (this.isImportSource(target)) {
+        continue
+      }
       const mod = await this.ensureModule(target.module)
       if (!mod) {
         continue

backend/modules/extensions/puppeteer/definition.yml

@@ -3,7 +3,8 @@ title: Puppeteer
 description: >-
   Headless Chromium browser. Required to re-render a page from its source on the server: the markdown
   renderer lives in the browser, so the server drives one. Installing it downloads a Chromium build of
-  a few hundred megabytes, unless the server already provides one through PUPPETEER_EXECUTABLE_PATH.
+  a few hundred megabytes, unless the server already provides one through PUPPETEER_EXECUTABLE_PATH,
+  which has to be the Chrome release this version of Puppeteer is built for.
 website: 'https://pptr.dev'
 detect:
   type: module
@@ -17,4 +18,8 @@ architectures:
 isInstallable: true
 # The version installed, here rather than in `package.json` for the reason above. The Docker image
 # reads this same line, so an image and a hand-installed instance agree on what Puppeteer is.
-installVersion: 25.4.0
+#
+# It also decides which browser: each release is built against one Chrome, and installs that build and
+# no other. Chrome for Testing has native linux-arm64 builds only from Chrome 153, and a release pinned
+# to an older one installs the x64 build on arm64 without complaint, so this must not go below 25.12.0.
+installVersion: 25.12.0

backend/modules/storage/git/storage.ts

@@ -34,12 +34,13 @@ interface Repo {
   fingerprint: string
   /** Whether the remote has been contacted since this entry was made. See `ensureRemote`. */
   remoteReady: boolean
-  /** Serializes work on this repository. See `withRepo`. */
-  queue: Promise<unknown>
 }
 
 const repos = new Map<string, Repo>()
 
+/** The last operation queued against each target. See `withRepo`. */
+const queues = new Map<string, Promise<unknown>>()
+
 /** The working copy for this target, as an absolute path. */
 function repoDir(target: StorageTarget): string {
   return resolveRoot(target.config.localRepoPath, DEFAULT_REPO_PATH)
@@ -207,8 +208,7 @@ async function prepareRepo(target: StorageTarget): Promise<Repo> {
     git,
     root,
     fingerprint: configFingerprint(target),
-    remoteReady: false,
+    remoteReady: false
-    queue: Promise.resolve()
   }
 }
 
@@ -220,19 +220,25 @@ async function prepareRepo(target: StorageTarget): Promise<Repo> {
  * safe, and the queue is per target because two targets are two working copies.
  */
 async function withRepo<T>(target: StorageTarget, run: (repo: Repo) => Promise<T>): Promise<T> {
+  const turn = async () => {
     let repo = repos.get(target.id)
     if (!repo || repo.fingerprint !== configFingerprint(target)) {
       repo = await prepareRepo(target)
       repos.set(target.id, repo)
     }
-  const entry = repo
+    return run(repo)
-  const result = entry.queue.then(
+  }
-    () => run(entry),
+  // -> The repository is prepared inside the turn, and the queue is kept apart from it. A queue that
-    () => run(entry)
+  //    lived on the prepared entry would give two operations arriving before there was one an entry
-  )
+  //    each, and a changed setting a new entry beside whatever was still running on the old one — two
+  //    queues over one working copy, and the index lock this exists to avoid.
+  const result = (queues.get(target.id) ?? Promise.resolve()).then(turn, turn)
   // -> The queue holds the settled outcome rather than the result, so one failed operation does not
   //    reject every operation queued behind it
-  entry.queue = result.catch(() => {})
+  queues.set(
+    target.id,
+    result.catch(() => {})
+  )
   return result
 }
 
@@ -1110,7 +1116,11 @@ async function applyIncoming(
   let deletedAssets = 0
   for (const segments of removals) {
     try {
-      const removed = await removeFromWiki(target, segments, actorId)
+      // -> The file is already gone from this working copy, and deleting it again would queue behind
+      //    the sync that is running this — see `storage.importingFrom`
+      const removed = await WIKI.models.storage.importingFrom(target, () =>
+        removeFromWiki(target, segments, actorId)
+      )
       if (removed === 'page') {
         deletedPages++
       } else if (removed === 'asset') {

backend/tasks/simple/render-pages.ts

@@ -1,3 +1,5 @@
+import type { TaskContext } from '../../core/scheduler.ts'
+
 /**
  * Render every page waiting in the render queue.
  *
@@ -7,6 +9,6 @@
  * there is one browser and it renders one page at a time. A run that finds the queue empty (a second
  * job for a batch this one already swept) returns without launching anything.
  */
-export async function task(): Promise<void> {
+export async function task(_payload: unknown, { signal }: TaskContext): Promise<void> {
-  await WIKI.models.rendering.drainQueue()
+  await WIKI.models.rendering.drainQueue(signal)
 }

backend/tasks/simple/sync-storage-targets.ts

@@ -1,3 +1,5 @@
+import type { TaskContext } from '../../core/scheduler.ts'
+
 /**
  * Sync every storage target that has a remote to keep in step with, and whose site is due.
  *
@@ -23,7 +25,7 @@
  * working copy is synced. Every instance in a high-availability set keeps its own, so they each fall
  * in step at their own turn rather than fighting over one repository.
  */
-export async function task(): Promise<void> {
+export async function task(_payload: unknown, { signal }: TaskContext): Promise<void> {
   const syncable = await WIKI.models.storage.syncableTargets()
   if (syncable.length < 1) {
     return
@@ -53,7 +55,14 @@ export async function task(): Promise<void> {
 
   WIKI.logger.info(`Syncing ${targets.length} storage target(s)...`)
   let failed = 0
-  for (const target of targets) {
+  for (const [index, target] of targets.entries()) {
+    // -> A sync cannot be interrupted halfway, but the next one need not be started. The targets left
+    //    over sync on their next due tick.
+    if (signal.aborted) {
+      throw new Error(
+        `Stopped after ${index} of ${targets.length} storage target(s): ${signal.reason?.message}`
+      )
+    }
     try {
       const message = await WIKI.models.storage.executeAction(target, 'sync', actorId)
       WIKI.logger.info(`Synced ${target.title} for site ${target.siteId}: ${message ?? 'done'}`)

dev/build/Dockerfile

@@ -4,7 +4,6 @@ LABEL maintainer="requarks.io"
 RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -qy --no-install-recommends \
     bash \
     build-essential \
-    chromium \
     curl \
     fonts-liberation \
     git \
@@ -30,11 +29,16 @@ USER node
 
 ENV NODE_ENV=production
 
-# The browser the Puppeteer extension drives, installed above rather than downloaded by Puppeteer: the
+# The browser the Puppeteer extension drives is the one Puppeteer installs for itself, not the distro's:
-# distro keeps it patched, it exists for arm64 as well as amd64, and the image does not carry two copies
+# Puppeteer speaks the protocol of the single Chrome release it was built against, and a distro package
-# of Chromium. `PUPPETEER_SKIP_DOWNLOAD` has to be set before the install below for that to hold.
+# moves on with every security update while Puppeteer stays pinned, so each rebuild widened the gap until
-ENV PUPPETEER_SKIP_DOWNLOAD=true
+# every render failed. Installed by Puppeteer, the pin below decides both halves of the pair.
-ENV PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium
+#
+# Only the headless shell, which is all a renderer needs; the full browser is skipped.
+# The cache is a fixed path rather than under $HOME so that it is found whatever user the container is
+# run as -- an arbitrary UID on OpenShift has no home directory of its own.
+ENV PUPPETEER_CACHE_DIR=/wiki/.cache/puppeteer
+ENV PUPPETEER_CHROME_SKIP_DOWNLOAD=true
 
 WORKDIR /wiki/backend
 RUN npm ci --omit=dev
@@ -48,9 +52,37 @@ RUN npm ci --omit=dev
 # operator adds Puppeteer to an instance by hand: one place to bump, and an image that cannot drift from
 # what a hand-installed instance gets. An empty read fails the build rather than quietly installing
 # whatever is newest.
+#
+# Puppeteer's postinstall downloads the browser, and `browsers install` makes sure of it: npm may be
+# configured not to run install scripts, and when the browser is already there it only prints its path.
 RUN PUPPETEER_VERSION="$(sed -n 's/^installVersion: *//p' modules/extensions/puppeteer/definition.yml)" && \
     test -n "$PUPPETEER_VERSION" && \
-    npm install --no-save "puppeteer@${PUPPETEER_VERSION}"
+    npm install --no-save "puppeteer@${PUPPETEER_VERSION}" && \
+    npx puppeteer browsers install chrome-headless-shell
+
+# The system libraries that browser needs, which are no longer arriving as the dependencies of a distro
+# package. Each Chrome for Testing build lists its own in `deb.deps`, per architecture, and this is the
+# `apt-get satisfy` that `browsers install --install-deps` would run -- done here as root, rather than
+# running Puppeteer as root, so that nothing in the cache ends up owned by root.
+USER root
+RUN DEPS="$(find "$PUPPETEER_CACHE_DIR" -name deb.deps)" && \
+    test "$(printf '%s\n' "$DEPS" | grep -c .)" -eq 1 && \
+    apt-get update && \
+    DEBIAN_FRONTEND=noninteractive apt-get satisfy -qy --no-install-recommends "$(paste -sd, "$DEPS")" && \
+    rm -rf /var/lib/apt/lists/*
+USER node
+
+# Launch the browser the way the renderer does and navigate once, which is the step a mismatched or
+# half-installed browser fails at. A pair that cannot render fails the image build, not every render
+# on every instance running the image.
+RUN node --input-type=module -e " \
+    import puppeteer from 'puppeteer'; \
+    const browser = await puppeteer.launch({ headless: 'shell', args: ['--no-sandbox', '--disable-dev-shm-usage'] }); \
+    const page = await browser.newPage(); \
+    await page.goto('data:text/html,<p>ok</p>'); \
+    if ((await page.\$eval('p', (e) => e.textContent)) !== 'ok') throw new Error('The browser did not render.'); \
+    console.log('Puppeteer drives ' + (await browser.version())); \
+    await browser.close();"
 
 WORKDIR /wiki