fix(databases): sync non-indexed attributes on worker-restart schema reload (RE-7) (#1183)

* fix(databases): sync non-indexed attributes in initStores() on resetDatabases() (RE-7)

`initStores()` only merged indexed attributes back into `table.attributes`
when updating an existing table (e.g. after a hot-reload–triggered
`resetDatabases()`). Non-indexed, non-primary-key fields (e.g. `name: String`,
`breed: String`) were written to `attributesDbi` by `table()` in the
restarted worker and signalled via ITC, but the main thread's
`resetDatabases()` → `initStores()` path skipped them — leaving
`describe_database` returning only the old attribute list until a full
kill+restart.

Fix: extend the existing-table update loop in `initStores()` to add/replace
non-indexed attributes as well as indexed ones, and remove them when they
are no longer present in the persisted schema.

Adds two regression tests in schemaMigrationFragility.test.js (RE-7 suite).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(databases): fix RE-7 regression test to actually exercise the INSERT path

The previous setup called table() with all four attributes, which populated
Table.attributes before resetDatabases() ran. initStores() always found
existingIdx >= 0, so the INSERT path (existingIdx < 0) was never reached
and the test passed with or without the fix.

Now simulates the true RE-7 mismatch: after table() writes all four attrs to
attributesDbi, reset in-memory Table.attributes back to [id] to mirror the
stale main-thread state that exists after a worker-only restart. resetDatabases()
must now re-sync from attributesDbi, exercising the INSERT path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(databases): address PR review — use attribute.attribute and fix removal test

- resources/databases.ts: use `attribute.attribute` instead of `attribute.name`
  in the non-indexed attribute findIndex (per cb1kenobi's suggestion; `attribute.name`
  can be falsey for attributes that arrive with only the `attribute` field set).
- unitTests/resources/schemaMigrationFragility.test.js: fix removal regression test —
  after `table()` updates in-memory Table.attributes, re-create the stale main-thread
  state (still holding breed/age) before calling resetDatabases(), so the removal loop
  in initStores() is actually exercised.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: run prettier on schemaMigrationFragility test

* fix(databases): preserve runtime-only relationship attrs on resetDatabases() (RE-7)

The removal loop in initStores() was dropping runtime-only attributes — like
relationship attrs added via GraphQL `@relationship` — because table()'s
persistence loop intentionally `continue`s past them (databases.ts:1138), so
they appear in `existingAttributes` (table.attributes) but not in the
`attributes` list rebuilt from attributesDbi. updatedAttributes() then stripped
the resolver/search support, breaking nested GraphQL queries after any
resetDatabases() / hot-reload — the symptom that showed up in CI as 8 failing
`handles query by nested attribute` assertions in graphql-querying-test and the
404/501 RESTProperties failures.

Also fixes a splice-while-iterating bug in the same loop: splicing the array
being walked by `for...of` skipped the next element, so two adjacent removals
(e.g. `breed` and `age` in the regression test) only dropped one. The loop now
collects into `toRemove` and applies the splices after iteration.

Adds a unit test that injects a relationship attr and asserts it survives a
resetDatabases() call, and extends the removal test's comment to explain why
the two-adjacent-removal pattern is intentional.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(databases): set attributesUpdated on non-indexed attribute update path

When an existing non-indexed attribute was already present in
existingAttributes (existingIdx >= 0) and got replaced by the splice,
attributesUpdated was never set to true.  As a result the schemaVersion
bump and updatedAttributes() call at line 682 were skipped, leaving
downstream code with stale property resolvers when a plain field's
type/nullability changed via hot-reload.

The add path (existingIdx < 0) was already correct; this brings the
update path into parity.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: kriszyp

resources/databases.ts

@@ -620,11 +620,29 @@ function initStores(
 					if (existingAttribute) existingAttributes.splice(existingAttributes.indexOf(existingAttribute), 1, attribute);
 					else existingAttributes.push(attribute);
 					attributesUpdated = true;
+				} else if (!attribute.isPrimaryKey) {
+					// Non-indexed, non-primary-key attributes (e.g. plain schema fields like `name: String`)
+					// must also be kept in sync so that describe_database reflects schema changes after a
+					// hot-reload / worker restart. Without this, resetDatabases() re-reads these attributes
+					// from attributesDbi but never merges them back into table.attributes — causing stale
+					// schema metadata until a full kill+restart. (RE-7)
+					const existingIdx = existingAttributes.findIndex((ea) => ea.name === attribute.attribute);
+					if (existingIdx >= 0) {
+						existingAttributes.splice(existingIdx, 1, attribute);
+						attributesUpdated = true;
+					} else {
+						existingAttributes.push(attribute);
+						attributesUpdated = true;
+					}
 				}
 			} catch (error) {
 				logger.error(`Error trying to update attribute`, attribute, existingAttributes, indices, error);
 			}
 		}
+		// Collect removals first; splicing while iterating `existingAttributes` skips adjacent
+		// elements, which would silently leave stale fields behind when two or more were dropped
+		// in the same reload.
+		const toRemove = [];
 		for (const existingAttribute of existingAttributes) {
 			const attribute = attributes.find((attribute) => attribute.name === existingAttribute.name);
 			if (!attribute) {
@@ -645,11 +663,22 @@ function initStores(
 				}
 				if (existingAttribute.indexed) {
 					// we only remove attributes if they were indexed, in order to support dropAttribute that removes dynamic indexed attributes
-					existingAttributes.splice(existingAttributes.indexOf(existingAttribute), 1);
-					attributesUpdated = true;
+					toRemove.push(existingAttribute);
+				} else if (!existingAttribute.isPrimaryKey) {
+					// Skip runtime-only attributes (e.g. relationship attrs — table()'s persistence loop
+					// `continue`s past them at line 1138). They are present in `existingAttributes` but
+					// never in the `attributes` list rebuilt from attributesDbi; removing them would drop
+					// the resolver/search support added by updatedAttributes(). Computed attrs ARE
+					// persisted, so only `relationship` is excluded here.
+					if (existingAttribute.relationship) continue;
+					toRemove.push(existingAttribute);
 				}
 			}
 		}
+		for (const existingAttribute of toRemove) {
+			existingAttributes.splice(existingAttributes.indexOf(existingAttribute), 1);
+			attributesUpdated = true;
+		}
 		if (table && !recreateForEngineChange) {
 			if (attributesUpdated) {
 				table.schemaVersion++;

unitTests/resources/schemaMigrationFragility.test.js

@@ -382,6 +382,123 @@ describe('schema-migration fragility: stale `changed` reused after re-fetch unde
 	});
 });
 
+describe('schema-migration fragility: non-indexed attributes missing from table.attributes after resetDatabases() (RE-7)', () => {
+	if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') return;
+
+	const DB = 're7NonIndexedAttrs';
+	const TABLE = 'Pet';
+	const testRoot = path.resolve(__dirname, '../envDir/re7NonIndexedAttrs');
+	const dbDir = path.join(testRoot, terms.DATABASES_DIR_NAME);
+
+	before(async () => {
+		setMainIsWorker(true);
+		await fs.remove(testRoot);
+		await fs.mkdirp(dbDir);
+		env.setProperty(terms.HDB_SETTINGS_NAMES.HDB_ROOT_KEY, testRoot);
+		env.setProperty(terms.CONFIG_PARAMS.ROOTPATH, testRoot);
+		env.setProperty(terms.CONFIG_PARAMS.STORAGE_PATH, dbDir);
+		env.setProperty(terms.CONFIG_PARAMS.DATABASES, {});
+
+		resetDatabases();
+		// Initial schema: only the primary key — simulates the main thread's stale view
+		// before a worker restart expands the schema.
+		table({
+			table: TABLE,
+			database: DB,
+			attributes: [{ name: 'id', isPrimaryKey: true }],
+		});
+		// Simulate a worker restart writing the expanded schema to attributesDbi.
+		// table() updates BOTH in-memory Table.attributes AND attributesDbi, so after
+		// this call attributesDbi has all four attributes.
+		table({
+			table: TABLE,
+			database: DB,
+			attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }, { name: 'breed' }, { name: 'age' }],
+		});
+		// Re-create the stale main-thread state: reset in-memory Table.attributes back to
+		// [id] only, while attributesDbi still has all four.  This is exactly the mismatch
+		// that exists on the main thread after a worker restarts and writes a new schema —
+		// the main thread's Table.attributes hasn't been updated yet.
+		const staleTable = getDatabases()[DB]?.[TABLE];
+		staleTable.attributes.splice(0, staleTable.attributes.length, { name: 'id', isPrimaryKey: true });
+		// Simulate the ITC schema-change handler calling resetDatabases() in the main thread
+		// (the path that describe_database goes through).  Without the fix, initStores()
+		// only re-syncs indexed/pk attributes and the non-indexed fields stay missing.
+		resetDatabases();
+	});
+
+	after(async () => {
+		await fs.remove(testRoot);
+	});
+
+	it('table.attributes includes all non-indexed fields after resetDatabases()', () => {
+		const tbl = getDatabases()[DB]?.[TABLE];
+		assert.ok(tbl, `${DB}.${TABLE} should be registered after resetDatabases()`);
+		const attrNames = tbl.attributes.map((a) => a.name);
+		assert.ok(attrNames.includes('name'), `expected "name" in attributes, got: ${attrNames}`);
+		assert.ok(attrNames.includes('breed'), `expected "breed" in attributes, got: ${attrNames}`);
+		assert.ok(attrNames.includes('age'), `expected "age" in attributes, got: ${attrNames}`);
+	});
+
+	it('removes non-indexed attributes dropped from the schema after resetDatabases()', () => {
+		// Simulate schema shrinking (field removal), then another resetDatabases().
+		table({
+			table: TABLE,
+			database: DB,
+			attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }],
+		});
+		// table() updates in-memory Table.attributes directly (databases.ts:997), so after the
+		// call above the in-memory state is already [id, name].  Re-create the stale main-thread
+		// view — still holding the old [id, name, breed, age] — so that the removal loop in
+		// initStores() actually needs to drop breed and age.  Two adjacent removals (breed AND
+		// age) also guard against a regression of the splice-while-iterating bug: splicing the
+		// array being iterated by `for...of` skipped the next element, so the loop must collect
+		// removals first and apply them after the iteration.
+		const tblForRemoval = getDatabases()[DB]?.[TABLE];
+		tblForRemoval.attributes.splice(
+			0,
+			tblForRemoval.attributes.length,
+			{ name: 'id', isPrimaryKey: true },
+			{ name: 'name' },
+			{ name: 'breed' },
+			{ name: 'age' }
+		);
+		resetDatabases();
+		const tbl = getDatabases()[DB]?.[TABLE];
+		const attrNames = tbl.attributes.map((a) => a.name);
+		assert.ok(attrNames.includes('name'), `expected "name" in attributes`);
+		assert.ok(!attrNames.includes('breed'), `"breed" should be removed, got: ${attrNames}`);
+		assert.ok(!attrNames.includes('age'), `"age" should be removed, got: ${attrNames}`);
+	});
+
+	it('preserves runtime-only relationship attributes across resetDatabases()', () => {
+		// Relationship attrs are runtime-only — table()'s persistence loop skips them
+		// (databases.ts:1138, `if (attribute.relationship) continue`), so they are present in
+		// table.attributes but never written to attributesDbi.  After resetDatabases(),
+		// initStores() rebuilds `attributes` from attributesDbi only — so the relationship attr
+		// won't appear there.  The removal loop must not drop it; otherwise updatedAttributes()
+		// would strip the resolver/search support and downstream GraphQL nested queries return
+		// undefined (the integration symptom: graphql-querying-test "handles query by nested
+		// attribute" assertions).
+		table({
+			table: TABLE,
+			database: DB,
+			attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }],
+		});
+		const tblWithRel = getDatabases()[DB]?.[TABLE];
+		// Inject a runtime-only relationship attribute, mirroring what graphql.ts does after
+		// parsing a `@relationship` directive — these never round-trip through attributesDbi.
+		tblWithRel.attributes.push({ name: 'related', relationship: { from: 'relatedId' } });
+		resetDatabases();
+		const tbl = getDatabases()[DB]?.[TABLE];
+		const attrNames = tbl.attributes.map((a) => a.name);
+		assert.ok(
+			attrNames.includes('related'),
+			`relationship attribute "related" should survive resetDatabases() but was dropped — got: ${attrNames}`
+		);
+	});
+});
+
 describe('schema-migration fragility: stale store reused after LMDB to RocksDB engine migration (F4)', () => {
 	if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') return;