docs(#363 review F2): update AGENTS.md migration-ordering to the new tolerant behavior

The "Migration ordering" section still described the OLD crash-loop-at-boot
behavior this PR removes ("Kysely refuses to start … rejected at boot"). Rewrote
it to the new two-layer model: the CI migration-order gate is the primary defense
(rename to a current timestamp), and the runtime now sets allowUnorderedMigrations
so the app applies a back-dated migration instead of crash-looping (with the note
that #ensureNoMissingMigrations still guards a removed applied migration, and that
migrations must stay independent since apply order can differ across instances).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

.github/workflows/develop.yml

@@ -18,12 +18,48 @@ env:
   IMAGE: ghcr.io/vvzvlad/gitmost
 
 jobs:
-  # Run the reusable test suite first so a failing test blocks the image build.
+  # Run the reusable test suite. Together with the e2e jobs below it gates the
+  # publish job (the image push), not the build itself — build runs in parallel.
   test:
     uses: ./.github/workflows/test.yml
 
+  # Runs in parallel with the test/e2e jobs and only warms the buildx cache
+  # (GHA cache, scope develop-amd64). No push happens here — the publish job
+  # below is the only one that pushes the image.
   build:
-    needs: test
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+
+      - name: Resolve version
+        id: version
+        run: echo "value=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
+
+      - name: Build develop image (warm cache, no push)
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          platforms: linux/amd64
+          build-args: |
+            APP_VERSION=${{ steps.version.outputs.value }}
+            AI_AGENT_ROLES_CATALOG_URL=https://raw.githubusercontent.com/vvzvlad/gitmost/develop/agent-roles-catalog
+          push: false
+          cache-from: type=gha,scope=develop-amd64
+          cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
+
+  # The gate: rebuilds from the cache the build job just wrote (near-instant on
+  # a cache hit; worst case — cache eviction — a full rebuild, which matches the
+  # old sequential timing) and pushes :develop only when unit tests AND both
+  # e2e suites AND the build are green.
+  publish:
+    needs: [test, e2e-server, e2e-mcp, build]
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
@@ -57,13 +93,10 @@ jobs:
           push: true
           tags: ${{ env.IMAGE }}:develop
           cache-from: type=gha,scope=develop-amd64
-          cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
 
-  # e2e jobs run on every develop push but DO NOT gate the build/publish above:
+  # e2e jobs gate the publish (image push), not the build: the :develop image
-  # `build` stays `needs: test` only, so the :develop image still ships even if
+  # is pushed only when unit tests AND both e2e suites pass (publish.needs
-  # e2e fails. A failing e2e job turns the run red and triggers GitHub's email
+  # lists them all).
-  # to the pusher — that red run + email is the intended notification, not a
-  # deploy block.
   e2e-server:
     runs-on: ubuntu-latest
     # Hard cap: the full-AppModule e2e leaks open handles and hung jest to the 6h max.
@@ -124,9 +157,7 @@ jobs:
       - name: Run server e2e
         run: pnpm --filter ./apps/server test:e2e
 
-  # Same rationale as e2e-server: this job is intentionally NOT in
+  # Gates the publish too — see the comment above e2e-server.
-  # `build.needs`. Deploy of the :develop image must not be blocked by e2e;
-  # a red run plus GitHub's email to the pusher is the notification mechanism.
   e2e-mcp:
     runs-on: ubuntu-latest
     timeout-minutes: 20

.github/workflows/test.yml

@@ -13,6 +13,49 @@ permissions:
   contents: read
 
 jobs:
+  # Guard against a long-lived branch adding a migration whose timestamped
+  # filename sorts BEFORE migrations already applied on the target branch (and
+  # thus in prod). The Kysely startup migrator rejects that as "corrupted
+  # migrations" and crash-loops the app on boot (incident #361). This gate fails
+  # the PR so the migration is renamed to a current timestamp before merge. Only
+  # runs for pull_request events (needs a base branch to diff against).
+  migration-order:
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: Checkout (full history for the base-branch diff)
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Added migrations must sort after the newest on the base branch
+        env:
+          TARGET_BRANCH: ${{ github.base_ref }}
+        run: |
+          set -euo pipefail
+          MIG_DIR="apps/server/src/database/migrations"
+          # checkout above already did fetch-depth:0 (full history). Fetch the base
+          # WITHOUT --depth (a shallow graft would truncate the base history and
+          # break the merge-base when the base has moved ahead of the PR merge —
+          # exactly the long-branch-vs-moving-base case this gate guards, #361).
+          git fetch --no-tags origin "$TARGET_BRANCH"
+          newest_on_target=$(git ls-tree -r --name-only "origin/${TARGET_BRANCH}" "$MIG_DIR" | sort | tail -1)
+          # NO `|| true`: a diff failure (e.g. an unresolved merge-base) must fail
+          # the job CLOSED — a gate whose job is to BLOCK must never pass on error.
+          # `set -e` above already aborts on a non-zero diff exit.
+          added=$(git diff --diff-filter=A --name-only "origin/${TARGET_BRANCH}...HEAD" -- "$MIG_DIR")
+          bad=0
+          for f in $added; do
+            if [[ "$f" < "$newest_on_target" || "$f" == "$newest_on_target" ]]; then
+              echo "::error::Migration $f sorts at or before the newest on ${TARGET_BRANCH} ($newest_on_target) — rename it with a CURRENT timestamp before merge (do not change its contents). See incident #361."
+              bad=1
+            fi
+          done
+          if [ "$bad" -eq 0 ]; then
+            echo "Migration order OK (added migrations all sort after $newest_on_target)."
+          fi
+          exit $bad
+
   test:
     runs-on: ubuntu-latest
     timeout-minutes: 20

AGENTS.md

@@ -250,7 +250,10 @@ pnpm --filter server migration:codegen                   # regenerate src/databa
 ```
 Migration files live in `apps/server/src/database/migrations/` and are named `YYYYMMDDThhmmss-description.ts`. Fork-specific migrations only **add** tables (`page_embeddings`, `ai_chats`, `ai_chat_messages`, `ai_provider_credentials`, `ai_mcp_servers`, `page_template_references`) and columns (e.g. `pages.is_template`, a `NOT NULL DEFAULT false` boolean) — never drop/rewrite Docmost data.
 
-**Migration ordering — always check when merging branches/features.** Kysely runs migrations in **alphabetical (= timestamp) order** and refuses to start if a *new* migration sorts **before** one already applied to the DB (`corrupted migrations: ... must always have a name that comes alphabetically after the last executed migration`). When you merge a branch or land a feature, verify your migration's timestamp still sorts **after every migration that may already be applied on the target** (`/bin/ls -1 apps/server/src/database/migrations | sort | tail`). Branches developed in parallel routinely break this: a feature branch adds `…T130000-…`, `main` meanwhile ships and deploys `…T150000-…`, and after the merge the older-timestamped file is rejected at boot. **Fix = rename your migration to a timestamp after the latest one already in the target** (content unchanged — the filename is the ordering key), then rebuild so the compiled `dist/database/migrations/` picks up the new name.
+**Migration ordering — always check when merging branches/features.** Kysely runs migrations in **alphabetical (= timestamp) order**. A *new* migration that sorts **before** one already applied to the DB is a "back-dated" migration, which branches developed in parallel routinely produce: a feature branch adds `…T130000-…`, `develop` meanwhile ships and deploys `…T150000-…`, and after the merge the older-timestamped file has been skipped. Two layers guard this (both added for incident #361, where a back-dated migration crash-looped prod for ~11 min):
+
+- **CI gate (primary):** the `migration-order` job in `.github/workflows/test.yml` fails a PR whose added migration sorts at/before the newest on the base branch. **So the fix is to rename your migration to a timestamp after the latest one already in the target** (`/bin/ls -1 apps/server/src/database/migrations | sort | tail`; content unchanged — the filename is the ordering key), then rebuild so the compiled `dist/database/migrations/` picks up the new name.
+- **Runtime safety net:** both Migrators (`migration.service.ts` startup auto-migrate + `migrate.ts` CLI) set `allowUnorderedMigrations: true`, so the app does **not** refuse to start on an out-of-order migration — it applies the skipped older one instead of crash-looping. Kysely's `#ensureNoMissingMigrations` guard is still on (a *removed* applied migration is still an error). Because apply order can then differ from lexicographic across instances, migrations must stay **independent** (each creates its own objects) — the CI gate remains the primary line; this net only covers a gate bypass (manual push / hotfix branch).
 
 ## Architecture — the big picture
 

Dockerfile

@@ -5,6 +5,13 @@ RUN npm install -g pnpm@10.4.0
 
 FROM base AS builder
 
+# re2 (packages/mcp) always compiles from source under pnpm (the prebuilt-binary
+# download cannot identify the GitHub repo), so node-gyp needs python3/make/g++.
+# This stage is discarded, so the toolchain can stay installed.
+RUN apt-get update \
+  && apt-get install -y --no-install-recommends python3 make g++ \
+  && rm -rf /var/lib/apt/lists/*
+
 WORKDIR /app
 
 COPY . .
@@ -57,9 +64,16 @@ COPY --from=builder /app/patches /app/patches
 
 RUN chown -R node:node /app
 
-USER node
+# Toolchain is needed transiently to compile re2 during the prod install; install
+# and purge it in one layer to keep the final image slim. The install itself runs
+# as the node user via su to keep node_modules ownership without a costly chown layer.
+RUN apt-get update \
+  && apt-get install -y --no-install-recommends python3 make g++ \
+  && su node -c "pnpm install --frozen-lockfile --prod" \
+  && apt-get purge -y --auto-remove python3 make g++ \
+  && rm -rf /var/lib/apt/lists/*
 
-RUN pnpm install --frozen-lockfile --prod
+USER node
 
 RUN mkdir -p /app/data/storage
 

apps/server/src/database/migrate.ts

@@ -24,6 +24,10 @@ const migrator = new Migrator({
     path,
     migrationFolder,
   }),
+  // Match the startup auto-migrator (migration.service.ts): a back-dated
+  // migration from a long-lived branch must be applied, not rejected as
+  // "corrupted migrations" (incident #361). See that file for the full rationale.
+  allowUnorderedMigrations: true,
 });
 
 run(db, migrator, migrationFolder);

apps/server/src/database/services/migration.service.ts

@@ -19,6 +19,16 @@ export class MigrationService {
         path,
         migrationFolder: path.join(__dirname, '..', 'migrations'),
       }),
+      // A long-lived branch can add a migration whose timestamped filename sorts
+      // BEFORE migrations already applied in prod (e.g. #234's 20260627 landing
+      // after 20260704 was live). With the default (ordered) setting the startup
+      // migrator then sees "corrupted migrations" — the applied set is no longer a
+      // prefix of the sorted list — throws, and the app crash-loops on boot
+      // (incident #361: 502s for ~11 min). allowUnorderedMigrations runs any
+      // not-yet-applied migration regardless of filename order, so a back-dated
+      // migration is applied instead of bricking startup. A CI order-gate still
+      // discourages back-dating; this is the runtime safety net.
+      allowUnorderedMigrations: true,
     });
 
     const { error, results } = await migrator.migrateToLatest();

packages/mcp/test-e2e.mjs

@@ -450,7 +450,7 @@ async function main() {
     // 8. get_page markdown round-trip sanity (table separator present)
     const md = await client.getPage(pageId);
     check("get_page md: table separator emitted", md.data.content.includes("| --- |"), "");
-    check("get_page md: callout exported as :::", md.data.content.includes(":::info"));
+    check("get_page md: callout exported as Obsidian '> [!info]'", md.data.content.includes("> [!info]"));
 
     // 9. comments: create / list / reply / update / check_new / delete
     const beforeComments = new Date(Date.now() - 1000).toISOString();