fix(tree): validate tree index key selection

[Craig Macomber (Microsoft) (CraigMacomber)]

Description

Clarifies and validates createTreeIndex key selection so selectors use Simple Tree property keys, which are translated to stored keys internally.

Selected fields are validated eagerly as required, single-valued leaf fields before the index subscribes to forest updates. Index construction or update failures now break the forest and preserve the original error, preventing continued use of an apparently valid checkout.

This also adds explicit key-finder dependency scopes, uses immutable invalidation for identifier indexes, skips schemas with ambiguous identifier fields, and expands executable examples and regression coverage.

Breaking Changes

createTreeIndex when indexing over a field which had an explicit stored key now will interpret the provided keys as property keys, consistent with out other APIs. Code which worked around this bug by specifying the stored key will need to be updated.

createTreeIndex when indexing nodes with multiple identifier fields will no longer index them, instead of picking whatever identifier field comes first.

createTreeIndex in cases where valid in schema data could cause it to crash later when updating the index now give proper usages errors when constructing the index, so in the edge cases where such invalid indexes simply happened to not error (for example due to the tree always having none of the type being indexes), the code making them may need to be updated.

Reviewer Guidance

The review process is outlined in the pull request guidelines.

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (962 lines, 15 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain around broken-index reads and cursor cleanup.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR updates Simple Tree indexing to validate property-key selectors, translate stored keys, and improve failure handling.

Changes:

• Added eager key validation and property-to-stored-key translation.
• Improved dependency scopes, breaker integration, and identifier indexing.
• Expanded tests, examples, exports, and changesets.

File summaries

File	Description
packages/dds/tree/src/test/simple-tree/simpleTreeIndex.spec.ts	Key validation and failure regression coverage
packages/dds/tree/src/test/simple-tree/identifierIndex.spec.ts	Identifier regression coverage
packages/dds/tree/src/test/feature-libraries/indexing/treeIndex.spec.ts	Index construction and breaker coverage
packages/dds/tree/src/simple-tree/api/simpleTreeIndex.ts	Selector validation and index construction
packages/dds/tree/src/simple-tree/api/identifierIndex.ts	Identifier discovery and immutable indexing
packages/dds/tree/src/feature-libraries/indexing/index.ts	Indexing exports
packages/dds/tree/src/feature-libraries/indexing/anchorTreeIndex.ts	Dependency scopes and failure handling
packages/dds/tree/src/feature-libraries/index.ts	Feature-library exports
packages/dds/tree/src/entrypoints/legacy.ts	Export ordering
packages/dds/tree/src/entrypoints/beta.ts	Export ordering
packages/dds/tree/src/entrypoints/alpha.ts	Export ordering
packages/dds/tree/src/core/forest/forest.ts	Forest breaker contract
packages/dds/tree/src/core/forest/editableForest.ts	Breaker contract updates
.changeset/clear-index-property-keys.md	Property-key behavior changeset
.changeset/calm-identifiers-index.md	Identifier behavior changeset

Review details

Suppressed comments (4)

packages/dds/tree/src/feature-libraries/indexing/anchorTreeIndex.ts:129

• If indexField throws while indexing an existing node, forest.breaker.run rethrows before this cursor.free() executes. The forest contract requires outstanding cursors to be freed or cleared when invalidating, so a failed index construction leaves this subscription cursor live on the broken forest. Move the cleanup into a finally block.

				this.indexField(cursor);
			});
			cursor.free();
		});

packages/dds/tree/src/feature-libraries/indexing/anchorTreeIndex.ts:159

• When a newly created node has a value rejected by isKeyValid, this.indexField throws inside forest.breaker.run and the following detachedCursor.free() is skipped. That leaves an outstanding cursor after the forest is invalidated; use try/finally around the indexing work so update failures do not leak cursor state.

					this.indexField(detachedCursor);
					detachedCursor.free();
				}),

packages/dds/tree/src/feature-libraries/indexing/anchorTreeIndex.ts:352

• Both indexField and indexSpine can throw when an update produces an invalid key. In that path this cursor.clear() is skipped, leaving a current subscription cursor while forest.breaker.run marks the forest broken. Put cursor cleanup in a finally block to preserve the forest's invalidation contract.

		this.indexField(cursor);
		if (this.keyFinderDependencyScope === KeyFinderDependencyScope.Subtree) {
			this.indexSpine(cursor);
		}
		cursor.clear();

packages/dds/tree/src/test/simple-tree/simpleTreeIndex.spec.ts:404

• This test verifies the construction error but not the new failure-isolation contract: initial indexing is now run through forest.breaker.run, so a rejected existing value should leave the checkout broken and preserve the original error as its cause. Add a follow-up assertion that using the view after this assert.throws fails with a UsageError whose cause is the construction error, matching the update-failure regression test.

			assert.throws(
				() =>
					createTreeIndex(
						view,
						(schema) => (schema === NumericName ? "name" : undefined),
						(nodes) => nodes,
						isStringKey,
					),
				(error: Error) =>
					error instanceof UsageError &&
					error.message.includes('The value in key field "name" selected for schema') &&
					error.message.endsWith("was rejected by isKeyValid."),
			);

• Files reviewed: 15/15 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Bundle size comparison

Base commit: 400b2f1828ff7a5c671e85214dfa28bc20387fdd
Head commit: fec167d5a62ed1ad5cf28f935a80177c3c014c1d

Pending — Build - client packages is running. Results will appear here when the build completes.

[github-actions]

Bundle size comparison

Base commit: 400b2f1828ff7a5c671e85214dfa28bc20387fdd
Head commit: fec167d5a62ed1ad5cf28f935a80177c3c014c1d

Notable changes

• 🔴 fluidFrameworkAllAlpha.js: parsed 805988 → 807213 (+1225), gzip 221392 → 221932 (+540)

Per-bundle deltas

@fluid-example/bundle-size-tests

• 🔴 fluidFrameworkAllAlpha.js: parsed 805988 → 807213 (+1225), gzip 221392 → 221932 (+540)
• azureClient.js: parsed 634336 → 634331 (-5), gzip 169989 → 170074 (+85)
• odspClient.js: parsed 606600 → 606711 (+111), gzip 163052 → 163197 (+145)
• aqueduct.js: parsed 538752 → 538763 (+11), gzip 144632 → 144679 (+47)
• fluidFramework.js: parsed 415378 → 415411 (+33), gzip 117810 → 117845 (+35)
• sharedTree.js: parsed 404757 → 404783 (+26), gzip 115252 → 115271 (+19)
• containerRuntime.js: parsed 315027 → 315009 (-18), gzip 86438 → 86439 (+1)
• sharedString.js: parsed 175732 → 175739 (+7), gzip 49785 → 49795 (+10)
• experimentalSharedTree.js: parsed 161846 → 161846 (0), gzip 46722 → 46722 (0)
• matrix.js: parsed 153720 → 153727 (+7), gzip 44381 → 44388 (+7)
• loader.js: parsed 147328 → 147344 (+16), gzip 40038 → 40049 (+11)
• odspDriver.js: parsed 106695 → 106753 (+58), gzip 33227 → 33293 (+66)
• directory.js: parsed 65669 → 65676 (+7), gzip 18493 → 18502 (+9)
• 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
• odspPrefetchSnapshot.js: parsed 46463 → 46444 (-19), gzip 15512 → 15522 (+10)
• map.js: parsed 45820 → 45827 (+7), gzip 14120 → 14127 (+7)
• 252.js: parsed 44384 → 44384 (0), gzip 13741 → 13741 (0)
• summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
• socketModule.js: parsed 27108 → 27078 (-30), gzip 8069 → 8103 (+34)
• createNewModule.js: parsed 12464 → 12464 (0), gzip 4792 → 4805 (+13)
• summaryModule.js: parsed 3888 → 3888 (0), gzip 1874 → 1874 (0)
• connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
• sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 496 → 505 (+9)
• debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
• FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)