fix(metadata): derive the project_id-to-environment_id migration's table list from the declarations

.changeset/migration-affected-tables-declared-gate.md

@@ -0,0 +1,35 @@
+---
+"@objectstack/metadata": patch
+---
+
+fix(metadata): stop `migrateProjectIdToEnvironmentId` renaming into a column no declaration knows about (#13205)
+
+`AFFECTED_TABLES` was a hand-written list, and it outlived the declarations it
+described. The branch/project-removal amendment (M1) took `environment_id` out
+of `sys_metadata_history`'s declaration in `@objectstack/metadata-core`; the
+migration kept naming that table. Its only guard asks whether `project_id` is
+present **physically** (`_columnExists`) — which says nothing about the target
+column being **declared** — so against any database whose physical
+`sys_metadata_history` still carried the pre-v5 column, the migration renamed it
+to `environment_id`: a fresh orphan column that no declaration, no `syncSchema`
+and no reader knows about.
+
+The list is now **derived from the declarations** rather than restated beside
+them. A candidate table is migrated only if its current declaration carries the
+target column, so the two cannot drift apart again — the derivation and the
+declaration are the same fact. `@objectstack/metadata-core` was already a
+dependency of this package, so this adds no dependency edge.
+
+A candidate that does not declare the target column is now **reported** as
+`status: 'skipped_not_declared'` (an additive member of the result union)
+instead of silently vanishing from the result array: an operator reading the
+results can tell "considered and deliberately skipped" from "forgotten again",
+which is the state this defect started in.
+
+The sibling `migrateEnvIdToProjectId` is deliberately left alone: its target
+(`project_id`) is an intermediate column that no current declaration carries by
+design, so the "target must be declared" rule is sound only for the terminal
+migration in the chain.
+
+No behaviour changes for `sys_metadata`, whose declaration does carry
+`environment_id`: it is renamed exactly as before.

packages/metadata/src/migrations/migrate-project-id-to-environment-id.test.ts

@@ -0,0 +1,199 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #13205 — the migration's table list must never outlive the declarations.
+ *
+ * `migrateProjectIdToEnvironmentId` is the TERMINAL step of the tenancy-column
+ * chain, so its target column (`environment_id`) is by definition the current
+ * declared shape. A table on its list whose declaration no longer carries that
+ * column is therefore not a stale comment — it is a rename that MINTS AN ORPHAN
+ * COLUMN, on a real database, that no declaration knows about.
+ *
+ * `sys_metadata_history` was exactly that: the branch/project-removal amendment
+ * (M1) took `environment_id` out of its declaration and the hand-written
+ * `AFFECTED_TABLES` kept naming it. The old `_columnExists` guard could not see
+ * it — it gates on `project_id` being PHYSICALLY present, which says nothing
+ * about the target being DECLARED.
+ *
+ * So these are pins on the PROPERTY, not on today's answer:
+ * 1. every table the migration will touch declares the column it produces
+ * (quantified over `AFFECTED_TABLES`, so a hand-added entry is caught);
+ * 2. membership TRACKS the declaration in both directions (so the derivation
+ * cannot be replaced by a literal that happens to match today, and cannot
+ * quietly degrade to nothing);
+ * 3. the migration issues no SQL AT ALL against a table it must not rename,
+ * even when that table physically still carries `project_id` — which is
+ * the shape the defect actually took.
+ */
+
+import { describe, expect, it } from 'vitest';
+import {
+ SysMetadataObject,
+ SysMetadataHistoryObject,
+ SysMetadataAuditObject,
+ SysMetadataCommitObject,
+ SysViewDefinitionObject,
+} from '@objectstack/metadata-core';
+
+import {
+ AFFECTED_TABLES,
+ migrateProjectIdToEnvironmentId,
+} from './migrate-project-id-to-environment-id.js';
+
+/** The column the migration produces. Renaming into it is only legal if it is declared. */
+const TARGET_COLUMN = 'environment_id';
+/** The column the migration renames away from. */
+const SOURCE_COLUMN = 'project_id';
+
+type DeclaredObject = { name: string; fields?: Record<string, unknown> };
+
+/**
+ * Every metadata storage object declared by `@objectstack/metadata-core`. Used
+ * to RESOLVE a table named by the migration back to its declaration — a name
+ * with no declaration at all is itself drift.
+ */
+const DECLARED_OBJECTS: readonly DeclaredObject[] = [
+ SysMetadataObject,
+ SysMetadataHistoryObject,
+ SysMetadataAuditObject,
+ SysMetadataCommitObject,
+ SysViewDefinitionObject,
+] as unknown as readonly DeclaredObject[];
+
+/**
+ * The tables this migration may ever consider: the metadata storage tables that
+ * historically carried the tenancy column. This is the migration's UNIVERSE
+ * (fixed by history), not its answer — the answer is what the assertions below
+ * derive from the declarations.
+ */
+const CANDIDATE_TABLES = ['sys_metadata', 'sys_metadata_history'] as const;
+
+function declaration(table: string): DeclaredObject | undefined {
+ return DECLARED_OBJECTS.find((o) => o.name === table);
+}
+
+function declaresTarget(table: string): boolean {
+ const object = declaration(table);
+ return !!object && Object.prototype.hasOwnProperty.call(object.fields ?? {}, TARGET_COLUMN);
+}
+
+/** Records every statement the migration issues, and answers column probes. */
+function fakeDriver(physicalColumns: Record<string, readonly string[]>) {
+ const statements: string[] = [];
+ const raw = async (sql: string, _bindings?: unknown[]) => {
+ statements.push(sql);
+ const pragma = /^PRAGMA table_info\("(.+)"\)$/.exec(sql);
+ if (pragma) {
+ const columns = physicalColumns[pragma[1]!] ?? [];
+ return columns.map((name) => ({ name }));
+ }
+ if (sql.startsWith('SELECT column_name')) return [];
+ return [];
+ };
+ return { driver: { raw } as any, statements };
+}
+
+describe('migrateProjectIdToEnvironmentId — AFFECTED_TABLES is pinned to declared reality', () => {
+ it('every table it will rename DECLARES the column the rename produces', () => {
+ // The safety property, quantified over the list itself: whatever ends up
+ // in AFFECTED_TABLES — derived today, hand-written tomorrow — must be a
+ // table whose CURRENT declaration carries `environment_id`. This is the
+ // assertion that would have failed on `sys_metadata_history`.
+ const orphanMinting = AFFECTED_TABLES.filter((table) => !declaresTarget(table));
+ expect(orphanMinting).toEqual([]);
+ });
+
+ it('names only tables that have a declaration at all', () => {
+ const undeclared = AFFECTED_TABLES.filter((table) => declaration(table) === undefined);
+ expect(undeclared).toEqual([]);
+ });
+
+ it('membership TRACKS the declaration, in both directions', () => {
+ // Not `toEqual(['sys_metadata'])` — that would re-state today's answer and
+ // pin nothing. Each candidate's membership is compared against its own
+ // declaration, so this keeps holding (and keeps meaning something) if a
+ // declaration legitimately regains or loses `environment_id`.
+ for (const table of CANDIDATE_TABLES) {
+ expect(
+ AFFECTED_TABLES.includes(table),
+ `${table}: list membership must equal "declares ${TARGET_COLUMN}"`,
+ ).toBe(declaresTarget(table));
+ }
+ });
+
+ it('is not vacuous — the derivation still selects the tables that do declare it', () => {
+ // Guards the other failure direction: a derivation that silently degrades
+ // to an empty list would satisfy every assertion above.
+ const expected = CANDIDATE_TABLES.filter(declaresTarget);
+ expect(expected.length).toBeGreaterThan(0);
+ expect([...AFFECTED_TABLES].sort()).toEqual([...expected].sort());
+ });
+
+ it('considers no table outside the historical candidate set', () => {
+ expect(AFFECTED_TABLES.every((t) => (CANDIDATE_TABLES as readonly string[]).includes(t))).toBe(true);
+ });
+});
+
+describe('migrateProjectIdToEnvironmentId — behaviour against a physically-stale database', () => {
+ /**
+ * The defect's exact shape: BOTH physical tables still carry `project_id`.
+ * The pre-fix code renamed both, because it only asked whether `project_id`
+ * was physically there.
+ */
+ const stale = {
+ sys_metadata: ['id', 'name', 'type', SOURCE_COLUMN],
+ sys_metadata_history: ['id', 'name', 'type', SOURCE_COLUMN],
+ };
+
+ it('renames only the declared table and issues NO statement against the undeclared one', async () => {
+ const { driver, statements } = fakeDriver(stale);
+
+ const results = await migrateProjectIdToEnvironmentId(driver);
+
+ const renamed = results.filter((r) => r.status === 'renamed').map((r) => r.table);
+ expect(renamed).toEqual(CANDIDATE_TABLES.filter(declaresTarget));
+
+ const alters = statements.filter((s) => s.startsWith('ALTER TABLE'));
+ expect(alters).toEqual([
+ `ALTER TABLE "sys_metadata" RENAME COLUMN ${SOURCE_COLUMN} TO ${TARGET_COLUMN}`,
+ ]);
+
+ // Sharper than "no ALTER": a table the migration must not touch is never
+ // even probed. Pre-fix this list held a PRAGMA and an ALTER.
+ const undeclared = CANDIDATE_TABLES.filter((t) => !declaresTarget(t));
+ for (const table of undeclared) {
+ expect(statements.filter((s) => s.includes(table))).toEqual([]);
+ }
+ });
+
+ it('reports the skipped table instead of dropping it silently', async () => {
+ const { driver } = fakeDriver(stale);
+
+ const results = await migrateProjectIdToEnvironmentId(driver);
+
+ // One entry per candidate — an operator can tell "considered and skipped"
+ // from "forgotten again", which is the state this card started in.
+ expect(results.map((r) => r.table).sort()).toEqual([...CANDIDATE_TABLES].sort());
+ for (const table of CANDIDATE_TABLES.filter((t) => !declaresTarget(t))) {
+ expect(results.find((r) => r.table === table)?.status).toBe('skipped_not_declared');
+ }
+ });
+
+ it('is idempotent on an already-migrated database', async () => {
+ const { driver, statements } = fakeDriver({
+ sys_metadata: ['id', 'name', 'type', TARGET_COLUMN],
+ sys_metadata_history: ['id', 'name', 'type'],
+ });
+
+ const results = await migrateProjectIdToEnvironmentId(driver);
+
+ expect(results.find((r) => r.table === 'sys_metadata')?.status).toBe('already_done');
+ expect(statements.filter((s) => s.startsWith('ALTER TABLE'))).toEqual([]);
+ });
+
+ it('still refuses a driver without .raw()', async () => {
+ await expect(migrateProjectIdToEnvironmentId({} as any)).rejects.toThrow(
+ /must expose a \.raw\(sql, bindings\?\) method/,
+ );
+ });
+});

packages/metadata/src/migrations/migrate-project-id-to-environment-id.ts

@@ -3,10 +3,9 @@
 /**
  * Migration: project_id → environment_id
  *
- * Renames the `project_id` column to `environment_id` on the metadata
- * storage tables:
- * - sys_metadata
- * - sys_metadata_history
+ * Renames the `project_id` column to `environment_id` on the metadata storage
+ * tables — but only on the tables whose CURRENT declaration actually knows
+ * `environment_id`.
  *
  * Forward counterpart of {@link migrateEnvIdToProjectId} (which performed the
  * earlier `env_id → project_id` rename). Together they let an operator walk an
@@ -15,6 +14,35 @@
  * migrateEnvIdToProjectId(driver); // env_id → project_id (legacy)
  * migrateProjectIdToEnvironmentId(driver); // project_id → environment_id (v5)
  *
+ * ─────────────────────────────────────────────────────────────────────
+ * Why the table list is DERIVED and not written out (#13205)
+ *
+ * This migration is the terminal step of that chain: its target column is
+ * the CURRENT declared shape, so "should this table be renamed?" is not an
+ * independent fact — it is `does this object still declare environment_id?`.
+ * Written out by hand, the two drifted apart: `sys_metadata_history` stayed
+ * on the list after the branch/project-removal amendment (M1) removed
+ * `environment_id` from its declaration, so against a database whose
+ * physical `sys_metadata_history` still carried `project_id` this migration
+ * renamed it to a column NO declaration knows about — minting exactly the
+ * orphan column class the metadata drift audit exists to remove.
+ *
+ * The old guard could not catch it: the loop gates on `project_id` existing
+ * PHYSICALLY (`_columnExists`), which says nothing about the target column
+ * being DECLARED. So the list is now computed from the declarations in
+ * `@objectstack/metadata-core` (already a dependency of this package — no
+ * new edge), and a candidate that does not declare the target column is
+ * reported as `skipped_not_declared` rather than dropped silently: an
+ * operator reading the result sees the table was considered and why nothing
+ * happened, instead of having to guess whether it was forgotten again.
+ *
+ * ⚠️ The sibling `migrate-env-id-to-project-id.ts` is deliberately NOT
+ * changed this way. Its target (`project_id`) is an INTERMEDIATE column that
+ * no current declaration carries by design — gating it on today's
+ * declarations would disable the chain's first step entirely. The rule
+ * "target must be declared" is sound only for the terminal migration.
+ * ─────────────────────────────────────────────────────────────────────
+ *
  * (The per-type projection tables `sys_object` / `sys_view` / `sys_flow` /
  * `sys_agent` / `sys_tool` were removed in 2026-05 along with the projection
  * pipeline — see ADR 0005 addendum. They are intentionally not included.)
@@ -29,24 +57,59 @@
  */
 
 import type { IDataDriver } from '@objectstack/spec/contracts';
+import { SysMetadataObject, SysMetadataHistoryObject } from '@objectstack/metadata-core';
+
+/** The column this migration RENAMES AWAY FROM. */
+const SOURCE_COLUMN = 'project_id';
 
-const AFFECTED_TABLES = [
- 'sys_metadata',
- 'sys_metadata_history',
-] as const;
+/** The column this migration PRODUCES. Must be declared, or the rename mints an orphan. */
+const TARGET_COLUMN = 'environment_id';
+
+/**
+ * Every metadata storage table this migration considers. Membership here says
+ * "this table has, historically, carried the tenancy column" — whether the
+ * rename actually runs is decided by {@link AFFECTED_TABLES} below, from the
+ * declaration.
+ */
+const CANDIDATE_OBJECTS = [SysMetadataObject, SysMetadataHistoryObject] as const;
+
+function declaresColumn(object: { fields?: Record<string, unknown> }, column: string): boolean {
+ return Object.prototype.hasOwnProperty.call(object.fields ?? {}, column);
+}
+
+/** Candidate table names, in declaration order. */
+const CANDIDATE_TABLES: readonly string[] = CANDIDATE_OBJECTS.map((o) => o.name);
+
+/**
+ * The tables this migration will actually rename: the candidates whose CURRENT
+ * declaration carries {@link TARGET_COLUMN}.
+ *
+ * Exported for the pin in `migrate-project-id-to-environment-id.test.ts` (not
+ * re-exported from `./index.ts` — this is not package surface).
+ */
+export const AFFECTED_TABLES: readonly string[] = CANDIDATE_OBJECTS
+ .filter((o) => declaresColumn(o, TARGET_COLUMN))
+ .map((o) => o.name);
 
 export interface ProjectIdToEnvironmentIdResult {
  table: string;
- status: 'renamed' | 'already_done' | 'table_missing' | 'error';
+ /**
+ * `skipped_not_declared` — the table is a known metadata storage table, but
+ * its current declaration has no `environment_id`, so renaming into it
+ * would create a column nothing declares. Nothing was executed.
+ */
+ status: 'renamed' | 'already_done' | 'table_missing' | 'skipped_not_declared' | 'error';
  error?: string;
 }
 
 /**
- * Rename `project_id` → `environment_id` on all metadata tables.
+ * Rename `project_id` → `environment_id` on all metadata tables that still
+ * declare `environment_id`.
  *
  * @param driver An IDataDriver with access to the target database.
  * Must expose a raw query method: `driver.raw(sql, bindings?)`.
- * @returns Per-table migration results.
+ * @returns Per-table migration results — one entry per candidate table,
+ * including the ones skipped for lacking the declared target.
  */
 export async function migrateProjectIdToEnvironmentId(
  driver: IDataDriver,
@@ -63,10 +126,18 @@ export async function migrateProjectIdToEnvironmentId(
 
  const results: ProjectIdToEnvironmentIdResult[] = [];
 
- for (const table of AFFECTED_TABLES) {
+ for (const table of CANDIDATE_TABLES) {
+ // The declared-target gate, ahead of every physical probe: a table whose
+ // declaration lost `environment_id` must never be renamed INTO it, no
+ // matter what the physical schema still carries (#13205).
+ if (!AFFECTED_TABLES.includes(table)) {
+ results.push({ table, status: 'skipped_not_declared' });
+ continue;
+ }
+
  try {
- const hasColumn = await _columnExists(driverAny, table, 'project_id');
- const alreadyMigrated = await _columnExists(driverAny, table, 'environment_id');
+ const hasColumn = await _columnExists(driverAny, table, SOURCE_COLUMN);
+ const alreadyMigrated = await _columnExists(driverAny, table, TARGET_COLUMN);
 
  if (alreadyMigrated && !hasColumn) {
  results.push({ table, status: 'already_done' });
@@ -79,7 +150,7 @@ export async function migrateProjectIdToEnvironmentId(
  }
 
  await driverAny.raw(
- `ALTER TABLE "${table}" RENAME COLUMN project_id TO environment_id`,
+ `ALTER TABLE "${table}" RENAME COLUMN ${SOURCE_COLUMN} TO ${TARGET_COLUMN}`,
  );
 
  results.push({ table, status: 'renamed' });