Add staged optional-to-required SharedTree field migration API (sf.stagedRequired)

[Noah Encke (noencke)]

Scenario

Applications that shipped a field as sf.optional(T) often later discover the field is conceptually mandatory, but they cannot simply switch to sf.required(T): doing so is a schema narrowing change, so old clients that are still running (and documents they created with the field empty) would immediately break. Today the only options are a coordinated flag-day deployment or leaving the field permanently optional and hand-checking for undefined everywhere.

This adds an alpha SharedTree schema API for a staged optional-to-required field migration, so an application can stop writing empty values first, and tighten the stored schema later once old clients are gone — the same rollout shape as the existing SchemaFactoryAlpha.stagedOptional and SchemaStaticsBeta.staged, but for the opposite direction.

Rollout contract

1. Version N uses sf.optional(T).
2. Version N+1 uses sf.stagedRequired(T) (stagedRequiredRecursive for recursive schema). The stored schema stays Optional, so version N clients are unaffected and their documents remain viewable. Because such a document may contain a node where the field is empty, the field is also Optional in the view schema during this phase — reads are typed T | undefined. What this step changes is that a version N+1 client never creates an empty value: constructing a node without a value, assigning/inserting undefined, and delete all throw a UsageError at runtime. TreeView.upgradeSchema treats this staged change as a no-op.
3. Once version N clients are extinct, the application explicitly opts in via the staged-upgrade token/policy mechanism (the optional StagedSchemaUpgradePolicy.includeStagedRequired, also reachable through extractPersistedSchema), which tightens the stored field kind from Optional to Required. Note that this member is optional on the @input StagedSchemaUpgradePolicy interface and defaults to never applying the tightening, so existing policy implementations keep compiling and keep their current behavior.
4. Version N+2 uses sf.required(T) and drops the staged marker. Only at this point does the field become non-optional in the TypeScript types.

class Point extends sf.objectAlpha("Point", {
	x: sf.number,
	y: sf.stagedRequired(sf.number),
}) {}

The upgrade is monotonic: the new computeUpgradeSchemas(viewSchema, stored, stagedSchemaUpgrades?) preserves staged-required fields that the stored schema has already tightened, so a staged client cannot propose an "upgrade" that reverts the tightening back to Optional. It never applies a staged tightening on its own, since that is a narrowing change that must always be opted into explicitly. It composes with the caller's StagedSchemaUpgradePolicy, and is used by both checkSchemaCompatibility and TreeView.upgradeSchema.

Because step 3 is the one upgrade that legitimately narrows the stored schema, computeUpgradeSchemas returns two projections of the same view schema, differing only in staged-required field kinds:

• wideningOnly — only the staged-required upgrades already applied in the stored schema. TreeView.upgradeSchema requires this to be a superset of the stored schema, which preserves the "upgrades never narrow" guarantee for everything else.
• target — additionally the ones explicitly opted into. This is what is actually written, using the pre-existing TreeCheckout.updateSchema(schema, allowNonSupersetSchema) escape hatch when it is not a superset.

checkSchemaCompatibility derives canUpgrade from wideningOnly and isEquivalent from target. When nothing is opted in the two projections are the same object, so behavior is unchanged.

TreeView.upgradeSchema does not scan the document: nothing verifies that every node already has a value for the field. Ensuring that is the application's responsibility, and this is called out in the API docs and the changeset.

Why the view field is Optional during the staged phase

This mirrors how stagedOptional and SchemaStaticsBeta.staged already work: during the staged phase the view schema describes the looser of the two states. A document created by a version N client may legitimately have the field empty, so T | undefined is the only read type that honestly describes every document a version N+1 client can open. Typing the read as non-undefined and throwing on read would make the TypeScript type a lie.

The target state is enforced on the write side instead:

• Assigning or inserting undefined, and delete, throw a UsageError (objectNode.ts setField / applyFieldChange, and the root setter via setField).
• Constructing a node without a value for the field throws a UsageError (objectNode.ts objectToFlexContent).
• Building content from a cursor or from insertable data with the field empty throws a UsageError, so TreeAlpha.create, TreeAlpha.importVerbose, TreeAlpha.importCompressed, TreeBeta.clone and TreeView.initialize cannot reintroduce an empty value (simple-tree/api/create.ts, simple-tree/unhydratedFlexTreeFromInsertable.ts, shared-tree/treeAlpha.ts).

These are runtime rather than compile-time errors because a TypeScript mapped type cannot make the write type of a property required while its read type is optional — the same limitation stagedOptional works around.

Nothing is scanned or materialized when opening a document or creating a view, no value is synthesized, and nothing is written during reads. For object fields, TreeAlpha.child(node, key) remains available as a proxy-bypassing presence check.

Compatibility caveat

Enabling step 3 assumes version N clients have been phased out; it is an operational precondition, not something the API can enforce. A stagedRequired client refuses to clear the field itself (assignment of undefined and delete both throw), which reduces the remaining race to concurrent clients two rollout generations behind. This is deliberately not a claim of absolute safety against arbitrarily old concurrent clients, and that caveat is stated in the API docs and the changeset.

Compatibility behavior outside this explicit staged case is unchanged: the discrepancy tolerance in discrepancies.ts is scoped to exactly a staged-required view field (Optional) over an already-tightened Required stored field, so schema narrowing is still not generally permitted.

Validation

Rebased onto main (c29323d9ca). All commands run from packages/dds/tree unless noted. The table below was produced at db18adef61 (the view-kind redesign); the write-enforcement follow-up dcc52965d5 is validated separately beneath it.

Command	Result
npm run build (tree — 211 fluid-build tasks: tsc ESM+CJS, api-extractor, depcruise, eslint, biome)	Build succeeded (12m 06s)
npm run build (packages/framework/fluid-framework)	Build succeeded (6m 01s)
npx mocha --config ./.mocharc.cjs "dist/test/**/*.spec.js" --fgrep "staged"	150 passing, 0 failing
npx mocha --config ./.mocharc.cjs "dist/test/**/*.spec.js" --fgrep "schema"	1449 passing, 6 pending, 2 failing — pre-existing/environmental only (below)
npm run format	Checked 678 files in 4s. No fixes applied.
GitHub merge status	mergeable: MERGEABLE

Pre-existing / environmental failures, not caused by this change:

• snapshotCompatibilityChecker > snapshotSchemaCompatibility > write current view schema snapshot (both the ESM and CJS copies of the same spec, hence 2) — a Windows-only path-separator bug in the test itself: the expected string is built with a literal / (${testSrcPath}/schemaSnapshots/point) while the code under test uses path.join, which yields \ on Windows. The two messages are otherwise character-identical. Neither snapshotCompatibilityChecker.spec.ts nor snapshotCompatibilityChecker.ts is touched by this PR (git diff --name-only upstream/main...HEAD confirms).

Head is now dcc52965d5, which added the cursor / insertable construction-path enforcement and two new test cases. That commit could not be validated with npm run build: the environment has no npm registry access, so it was validated against locally built dependencies instead — stagedSchemaUpgrade.spec.js 22 passing, 0 failing, and the full simple-tree suite 3191 passing, 34 pending, 1 failing (the same Windows path-separator artifact described above). A later independent re-verification of dcc52965d5 type-checked the package again and found only the 11 pre-existing errors in arrayNode.ts / treeDataStore.ts / treeFactory.ts — none of those files are touched by this PR, and all 11 come from stale sibling-package build output in that environment. CI remains authoritative for a clean-install build.
API reports were regenerated by the builds above. Three union-reordering hunks in unrelated JsonAsTree/FluidSerializableAsTree entries (the documented incremental-TS/API-Extractor flake, see .claude/skills/ci-readiness-check/tree-api-checks.md) were discarded, so the committed report diff contains only the stagedRequired* signature changes (FieldKind.Required → FieldKind.Optional) and the isRootPresent removal.

Head is now 616911f6df, which addressed review feedback: it made step 3 actually reachable through TreeView.upgradeSchema, added end-to-end coverage of that path, and made the stagedOptional/stagedRequired markers mutually exclusive in both the props types and at runtime. Validated in the same registry-less environment against locally rebuilt dependencies: tsc on the package and the test project 0 errors, stagedSchemaUpgrade.spec.js 24 passing, 0 failing, the combined simple-tree + shared-tree suites 4442 passing, 113 pending, 2 failing — both failures reproduced identically on the unmodified tree after stashing this branch's changes, so neither is a regression (the Windows path-separator artifact described above, plus an assert short-code mapping artifact from bypassing the repo's assert-tagging build step). packages/dds/tree/api-report/tree.alpha.api.md was regenerated by API Extractor; fluid-framework.alpha.api.md could not be regenerated locally (flub generate entrypoints fails in that environment), so the identical substitution API Extractor emitted for tree was applied to its twelve byte-identical lines. CI is authoritative for both reports.
Test coverage (src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts, mirroring the existing staged optional upgrade suite):

• Stored-schema projection is Optional before opt-in and Required after.
• Compatibility across all three rollout phases, including that a staged view does not revert an already-tightened stored schema, and that a version N client can no longer view the document after tightening.
• An absent root reads as undefined (present root reads normally); an absent object field reads as undefined while sibling fields stay usable, and TreeAlpha.child agrees.
• Blocked undefined writes and delete on both root and object fields; repair by assigning a real value.
• Construction without a value, and with an explicit undefined, throws a UsageError at runtime.
• stagedRequiredRecursive in a recursive schema.
• End-to-end: a four-tree walk of the documented rollout that actually calls TreeView.upgradeSchema — no-op at phase N+1, real tightening at step 3, version N clients losing canView, equivalence at step 4, and a non-opted-in staged client not reverting the tightening.
• An upgrade that narrows for any reason other than a staged-required opt-in is still rejected (canUpgrade === false, and upgradeSchema() throws).

Scope

Scope: 20 changed files, one concern.

Atomicity justification: this is a single API addition that must be threaded through one code path end-to-end — the view field schema, its stored-schema projection, compatibility/discrepancy reporting, the upgrade-schema computation, and the write enforcement points. Splitting it would produce intermediate states where, for example, stagedRequired exists in the type system but projects to a Required stored field (silently breaking old clients) or where the upgrade path reverts a tightening — i.e. unsafe and misleading rather than merely incomplete. Dropping the tests, changeset, or API reports to shrink the count is not an option under repo policy.

Review map (read in this order):

1. The real change (10 files): simple-tree/api/schemaFactoryAlpha.ts (the new API + docs), simple-tree/fieldSchema.ts (staged marker), simple-tree/toStoredSchema.ts (stored-kind projection), simple-tree/api/stagedRequiredUpgrades.ts (new — monotonic upgrade computation), simple-tree/api/discrepancies.ts (scoped compatibility tolerance), simple-tree/node-kinds/object/objectNode.ts (blocked clears + construction guard) shared-tree/schematizingTreeView.ts, and the three construction-path enforcement points simple-tree/api/create.ts, simple-tree/unhydratedFlexTreeFromInsertable.ts and shared-tree/treeAlpha.ts.
2. Supporting surface (4 files): simple-tree/simpleSchema.ts, simple-tree/core/toStored.ts (the new optional policy member), simple-tree/api/schemaCompatibilityTester.ts, simple-tree/api/storedSchema.ts.
3. Barrel exports (2 files): simple-tree/index.ts, simple-tree/api/index.ts.
4. Tests (1 file): stagedSchemaUpgrade.spec.ts carries the new coverage.
5. Generated / mechanical (3 files): the two api-report/*.alpha.api.md files (regenerated by API Extractor, never hand-edited) and the changeset.

Repo harnesses used

• Runtime: claude
• CLAUDE.md: native (claude)

No visual/UI surface is affected by this change, so no screenshots are included.

Authored with Minions.

[github-actions]

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

Based on the diff (1081 lines, 20 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

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Repository review skill: review invoked (repo-native .claude/skills/review, Standard mode: Breaker + API Analyst sub-agents + Inspector orchestrator). .claude/skills/ci-readiness-check/tree-api-checks.md was also consulted for the API-report diffs, per CLAUDE.md.

The design is genuinely good: the lazy read-time failure, the monotonic computeUpgradeSchema, the narrowly-scoped discrepancy tolerance, and permissiveStoredSchemaGenerationOptions.includeStagedRequired: () => false are all the right calls, and the test suite mirrors the existing staged optional upgrade suite closely. The code as written on this branch is correct — I found no correctness or API defect in it. The blocker is that the branch no longer applies to main.

Automated checks (run from packages/dds/tree in a clean worktree at 15a2cc54):

• pnpm install --filter "@fluidframework/tree..." --frozen-lockfile: pass
• npm run build (fluid-build: tsc ESM+CJS, api-extractor, eslint — 193 tasks): pass — Build succeeded
• npm run test:mocha:esm -- --fgrep "staged required upgrade": pass — 6 passing
• npm run test:mocha:esm -- --fgrep "staged": pass — 70 passing, 0 failing
• npm run test:mocha:esm -- --grep "snapshotCompatibilityChecker": 15 passing, 1 failing — pre-existing/environmental, not caused by this PR. write current view schema snapshot compares a literal-/ expected string against a path.join actual, so it only fails on Windows; the two messages are otherwise byte-identical and the spec file is untouched by this PR. Matches the author's own note.
• *.bench.js timeouts: pre-existing/environmental (benchmark timing under load).
• API reports after the full build: the only regenerated churn was union-member reordering inside Omit<> (e.g. "stagedOptionalUpgrade" | "defaultProvider" ↔ "defaultProvider" | "stagedOptionalUpgrade") plus unrelated JsonAsTree/FluidSerializableAsTree reordering and beta/legacy.beta churn. This is exactly the known incremental-TypeScript flake documented in .claude/skills/ci-readiness-check/tree-api-checks.md ("What 'unexpected' looks like"), not report drift — my build was incremental, not a root clean build. Not a finding. The committed reports match the source declarations and show no sign of hand-editing.

Blocking issues:

• packages/dds/tree/src/simple-tree/api/schemaCompatibilityTester.ts:33, packages/dds/tree/src/simple-tree/toStoredSchema.ts:479, packages/dds/tree/src/shared-tree/schematizingTreeView.ts:262, packages/dds/tree/src/simple-tree/api/index.ts:105, packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:544 — the branch has merge conflicts with main and cannot be merged (gh api .../pulls/27952 reports mergeable: false, mergeable_state: dirty; reproduced locally: git merge-tree --write-tree --name-only 9c67a984ca 15a2cc54a1 exits 1 with CONFLICT (content) in those five files). Per the review playbook I am not resolving these — flagging for the author. These are not trivial textual conflicts: since this branch's merge-base, main landed 9e5a315ecf refactor(tree): convert SchemaCompatibilityTester class to checkSchemaCompatibility function (#27658) — which converts the very class this PR adds computeUpgradeSchema to into a free function — and 44f40e8411 feat(tree): runtime schema upgrade mechanism (#27542), which directly overlaps the upgrade-schema computation this PR introduces. Required fix: rebase onto current main, re-express computeUpgradeSchema against the new checkSchemaCompatibility shape, reconcile with the new runtime schema upgrade mechanism (in particular confirm the staged-required token still participates correctly in it, and that getStoredFieldKind's new isStagedOptional/isStagedRequired branching still composes with whatever feat(tree): runtime schema upgrade mechanism #27542 changed in toStoredSchema.ts), then re-run the build and the staged suites. My build/test evidence above was gathered against the pre-rebase base and does not carry over to the merged result for these five files.

Minimum diff to ship: rebase onto current main and reconcile the five conflicted files with #27658 (SchemaCompatibilityTester → checkSchemaCompatibility) and #27542 (runtime schema upgrade mechanism), re-running npm run build and npm run test:mocha:esm -- --fgrep "staged" to confirm the 70 staged tests still pass.

Non-blocking observations

These are informational and require no action before merge.

• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:44 — the object-node branch of getAppliedStagedRequiredUpgrades (the viewSchema.definitions loop, including the storedNode.getFieldSchema(brand(fieldSchema.storedKey)) lookup) has no direct test. The monotonicity guarantee — "a staged client cannot revert an already-tightened stored field" — is asserted only for the root field (stagedSchemaUpgrade.spec.ts:576). I verified the branch is correct by reading (ObjectNodeSchema.fields values are FieldSchemaAlpha & SimpleObjectFieldSchema, so storedKey is the right key, and ObjectNodeStoredSchema.getFieldSchema returns storedEmptyFieldSchema rather than throwing for absent keys), so this is a coverage gap rather than a defect. Worth an assertion on an object field after rebase, since it is the same code the runtime-upgrade merge will touch.
• packages/dds/tree/src/simple-tree/api/storedSchema.ts:62 — the new includeStagedRequired: includeStaged wiring in extractPersistedSchema is untested, and includeStaged now means something directionally different for this third concept: returning true narrows the stored schema (Optional → Required), whereas for staged allowed types and staged optional it widens. A caller who passes () => true to "include everything" silently tightens staged-required fields, and the SchemaUpgrade token the predicate receives carries no discriminator, so selective opt-in is not expressible. The param TSDoc at storedSchema.ts:24 lists all three concepts but does not warn about the inverted direction — worth a sentence.
• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:89 — on the applied.size > 0 path, computeUpgradeSchema builds a fresh options object literal each call. toStoredSchema caches in viewToStoredCache, a WeakMap keyed first on options identity (toStoredSchema.ts:66-69,146-151), so this path never hits the cache and recomputes the whole stored schema on each call. This is what makes it correct (a stale cached result across different stored inputs is impossible — I specifically checked for that), and the call sites are cold, so it is fine; the // Common case: use the cached, allocation-free path comment could just note that the other branch is deliberately uncached.
• packages/dds/tree/src/simple-tree/api/tree.ts:483 and packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:176 — TreeViewAlpha.isRootPresent() and SchemaStaticsAlpha.stagedRequired/stagedRequiredRecursive are new required members on @alpha-exported interfaces, a compile break for any external implementer. In practice both are FF-produced "output" interfaces (the only implementers are SchematizingSimpleTreeView and the internal schemaStaticsAlpha const, both updated here), and this mirrors the precedent set when stagedOptional was added, so no API Council sign-off is required for @alpha-only changes. Worth confirming office-bohemia does not implement them, and optionally noting it in the changeset. (StoredFromViewSchemaGenerationOptions.includeStagedRequired is not a breaking change — that interface appears in no api-report and is not exported from the package entrypoint.)
• Scope: 20 files with an explicit atomicity justification and an ordered review map in the PR description — this satisfies the repo's scope policy for a wide diff. Not a finding; noted so it is not re-raised.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Fix summary — rebased onto main and reconciled with #27658 / #27542

Head is now 8f57978be56a6234e6b3f592e7a9940b74313ae7.

Blocking finding: branch had merge conflicts with main — fixed

Verified before touching anything: git merge-tree --write-tree --name-only upstream/main 15a2cc54a1 exited 1 with CONFLICT (content) in exactly the five files called out in the review. (Note for anyone reproducing: the task worktree's origin is the fork noencke/FluidFramework, so origin/main was stale at the PR's own merge-base and showed a clean merge — the conflicts only reproduce against upstream/main = microsoft/FluidFramework:main, currently c29323d9ca.)

Rebased onto c29323d9ca and reconciled each conflict against the two upstream changes the review identified:

• refactor(tree): convert SchemaCompatibilityTester class to checkSchemaCompatibility function #27658 (SchemaCompatibilityTester class → checkSchemaCompatibility function). computeUpgradeSchema is no longer a method. It is now a free function computeUpgradeSchema(viewSchema, stored, stagedSchemaUpgrades?) in simple-tree/api/stagedRequiredUpgrades.ts, called from checkSchemaCompatibility (schemaCompatibilityTester.ts:47) and from SchematizingSimpleTreeView.upgradeSchema (schematizingTreeView.ts:270). The SchemaCompatibilityTester-based assertions in stagedSchemaUpgrade.spec.ts were rewritten against the free-function form.
• feat(tree): runtime schema upgrade mechanism #27542 (runtime schema upgrade mechanism). StoredFromViewSchemaGenerationOptions was renamed to StagedSchemaUpgradePolicy, and the restrictive/permissive constants moved out of toStoredSchema.ts into the StagedSchemaUpgradePolicy factory in simple-tree/core/toStored.ts. The staged-required member moved with them: restrictive → false, permissive → false (tightening is a narrowing, so it is never part of a maximally permissive schema), enabledStagedUpgrades(...) → set membership. getStoredFieldKind's isStagedOptional/isStagedRequired branching survived the merge unchanged and still composes correctly.
• Composition with the new policy parameter (this is the substantive reconciliation, not a textual one). checkSchemaCompatibility and upgradeSchema now carry a caller-supplied stagedSchemaUpgrades policy that did not exist when this branch was written. computeUpgradeSchema therefore layers the already-applied staged-required upgrades on top of that policy (base.includeStagedRequired?.(u) === true || applied.has(u)) rather than replacing it, so the staged-required token still participates correctly in the runtime upgrade mechanism instead of silently dropping the view's configured stagedUpgradePolicy.

Related fix surfaced by the rebase: includeStagedRequired is now optional

Making it a required member broke compilation of an existing upstream test that constructs a StagedSchemaUpgradePolicy literal (stagedSchemaUpgrade.spec.ts:74, TS2741). That is a real signal: StagedSchemaUpgradePolicy is @alpha @input, so a required member is a compile break for every external implementer. It is now declared includeStagedRequired?(upgrade: SchemaUpgrade): boolean and defaults to never applying the tightening, so existing implementations keep compiling with unchanged behavior. This also let me drop the mechanical includeStagedRequired: () => … additions from toStoredSchema.spec.ts and testTrees.ts, taking the PR from 20 to 18 changed files.

Two documentation corrections came along with it: the {@link SchemaFactoryAlpha.stagedRequired} references were ambiguous (overloaded declaration) and failed api-extractor with ae-unresolved-link, so they now use SchemaStaticsAlpha.stagedRequired like the rest of the PR; and the changeset's stale StoredFromViewSchemaGenerationOptions reference was updated to StagedSchemaUpgradePolicy.

Non-blocking observation addressed

The extractPersistedSchema includeStaged TSDoc (storedSchema.ts:24) now warns that returning true for a staged required upgrade narrows the schema — the opposite direction from the other two staged concepts. This was the one non-blocking note that guards against a genuine silent-tightening footgun; the remaining non-blocking observations (object-field monotonicity coverage, the deliberately uncached branch, the @alpha interface members) were left as-is since they were informational.

Validation

Run from packages/dds/tree unless noted, on the rebased head:

Command	Result
npm run build (tree — 193 fluid-build tasks: tsc ESM+CJS, api-extractor, depcruise, eslint, biome)	Build succeeded
npm run build (packages/framework/fluid-framework)	Build succeeded
npm run test:mocha:esm -- --fgrep "staged"	75 passing, 0 failing
npm run test:mocha:esm -- --fgrep "toStoredSchema"	90 passing, 0 failing
npm run test:mocha:esm -- --fgrep "schema"	725 passing, 3 pending, 1 failing (pre-existing, below)
npm run format	Checked 678 files. No fixes applied.
git merge-tree --write-tree upstream/main HEAD	exit 0 — no conflicts; GitHub now reports mergeable: MERGEABLE

The single failure is snapshotCompatibilityChecker > write current view schema snapshot, unchanged from the review: a Windows-only path-separator bug in the test itself (literal / in the expected string vs. path.join in the code under test). Neither snapshotCompatibilityChecker.spec.ts nor snapshotCompatibilityChecker.ts appears in git diff --name-only upstream/main...HEAD, so it is not caused by this PR.

API reports were regenerated by the builds above, not hand-edited. The only delta beyond the previously reviewed additions is the one new optional member on StagedSchemaUpgradePolicy in tree.alpha.api.md and fluid-framework.alpha.api.md. Per .claude/skills/ci-readiness-check/tree-api-checks.md, I discarded the incremental-build flake that appeared in fluid-framework.legacy.public.api.md (spurious InteriorSequencePlace / SequencePlace / Side re-export lines, unrelated to this change).

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review feedback addressed — stagedRequired now uses the looser view field kind

Head: db18adef61. Scope is now 17 changed files.

Finding: "during the staged phase the field should be optional, not required" — valid, fixed

Reproduced first. On the previous head the view field kind was FieldKind.Required with a lazy read-time UsageError, so view.root / node.field were typed non-undefined yet could throw on a document a version N client had legitimately left empty. That is the opposite convention from stagedOptional, which uses the looser view kind and enforces on write.

Fix: the view field kind is now Optional during the staged phase; the stored field tightens to Required on explicit opt-in.

• schemaFactoryAlpha.ts:432 — createFieldSchema(FieldKind.Optional, ...) with the same defaultProvider as stagedOptional.
• toStoredSchema.ts getStoredFieldKind — inverted to mirror the stagedOptional branch directly above it.
• discrepancies.ts:374 — tolerance flipped to view Optional over stored Required.
• Read-time throws removed from objectNode.ts (proxy getter) and schematizingTreeView.ts (root getter).
• Added a construction guard in objectNode.ts objectToFlexContent:731: with the field now optional in the types, new Obj({}) / new Obj({ value: undefined }) are well-typed, so they throw a UsageError at runtime. This is a runtime rather than compile-time error because a TS mapped type cannot make a property's write type required while its read type is optional.
• undefined assignment / insert / delete rejection is unchanged (setField:417, applyFieldChange:826).
• Removed as now-dead: TreeViewAlpha.isRootPresent() and throwStagedRequiredFieldMissing. The isRootPresent() removal is my inference, not something the reviewer explicitly asked for — flagged on the originating thread for confirmation; it's a one-line revert if it should stay as a general optional-root convenience.

Changeset and PR description rewritten to describe optional-during-staging semantics.

Non-actionable

The github-actions[bot] reviewer-selection sticky comment is a UI affordance, not a finding.

Validation

From packages/dds/tree unless noted:

Command	Result
npm run build (tree, 211 tasks)	Build succeeded (12m 06s)
npm run build (packages/framework/fluid-framework)	Build succeeded (6m 01s)
npx mocha --config ./.mocharc.cjs "dist/test/**/*.spec.js" --fgrep "staged"	150 passing, 0 failing
npx mocha --config ./.mocharc.cjs "dist/test/**/*.spec.js" --fgrep "schema"	1449 passing, 6 pending, 2 failing (pre-existing)
npm run format	Checked 678 files in 4s. No fixes applied.
GitHub merge status	mergeable: MERGEABLE

The 2 failures are snapshotCompatibilityChecker > snapshotSchemaCompatibility > write current view schema snapshot (ESM and CJS copies of the same spec) — a Windows-only path-separator bug in the test's expected string (${testSrcPath}/schemaSnapshots/point vs. path.join). Messages are otherwise character-identical, and neither that spec nor its source is in git diff --name-only upstream/main...HEAD.

API reports were regenerated by the builds. Three union-reordering hunks in unrelated JsonAsTree/FluidSerializableAsTree entries — the documented incremental-TS/API-Extractor flake (.claude/skills/ci-readiness-check/tree-api-checks.md) — were discarded, so the committed report diff contains only the stagedRequired* signature change (FieldKind.Required → FieldKind.Optional) and the isRootPresent removal.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review pass on db18adef61 — no code changes were needed

I re-checked every open item on this PR against the live branch head (db18adef61) before touching anything. All three are already addressed or not attributable to this PR, so this pass makes no commit. Details and evidence below.

---

1. Noah Encke (@noencke) — "the field is required — shouldn't it be optional during this phase?" — ✅ already fixed

This was valid, and it was implemented in db18adef61 (the current head), which is why the thread is already resolved. Re-verified on the live branch just now rather than trusting the earlier write-up:

	claim	verified at
view field kind is Optional	✅	schemaFactoryAlpha.ts:432 — createFieldSchema(FieldKind.Optional, t, { …, stagedRequiredUpgrade: createSchemaUpgrade() })
stored kind tightens to Required only on explicit opt-in	✅	toStoredSchema.ts:478-484 — options.includeStagedRequired?.(isStagedRequired) === true ? FieldKind.Required : f.kind
reads return undefined (no read-time throw)	✅	no throwStagedRequiredFieldMissing anywhere in src/
writes of undefined / delete throw at runtime	✅	objectNode.ts:417 (setField), objectNode.ts:826 (applyFieldChange)
construction without a value throws at runtime	✅	objectNode.ts:731 (objectToFlexContent)

This is exactly the shape you described: looser (optional) read and write types, with the target state enforced by runtime UsageErrors rather than the type system — mirroring how stagedOptional handles the same TS mapped-type limitation. Note the guard at objectNode.ts:731 runs before the defaultProvider branch at :740, so a staged-required field can never silently fall back to an empty default.

One question from that thread is still open and I did not want it lost when the thread was resolved: I also removed TreeViewAlpha.isRootPresent(), since with reads now returning undefined the presence check is just view.root === undefined, and keeping it would add permanent alpha surface for a problem that no longer exists. It was added and removed within this PR, so it is net-zero against main — but if you would rather keep it as a general optional-root convenience, say so and I will restore it (one-line revert). TreeAlpha.child was pre-existing and is untouched.

2. 🔗 "Found some broken links!" bot comment — ❌ not caused by this PR

I treated this as a claim and checked it rather than chasing links. It does not hold up:

• The check itself passed on this exact head. Job 94579746426 (🔗 Broken Link Check, head_sha: db18adef61…) reports conclusion: success with all 23 steps green, including step 8 Check for broken links.
• The comment names no broken link. Its entire payload is the server starting and then [ELIFECYCLE] Command failed with exit code 1 — a tooling/exit-code artifact, not a link report.
• It is repo-wide. Of the currently open PRs that have a link report at all, 5 of 5 say BROKEN — build(eslint-config-fluid): consolidate custom rules into config package #27943 (eslint-config consolidation), feat(container-runtime): Default document schema to declare createBlobPayloadPending support at 2.40.0+ #27948 (container-runtime), refactor(tree): Reserve positional argument in treeChanged event listener signature #27951, Simplified summary process without summarizer nodes #27953 (summarization flow) and this one. Those PRs share no files with this one.
• This PR touches zero docs/website files. All 17 changed files are under .changeset/, packages/dds/tree/**, and two generated api-report/*.api.md files. Build site and 🧪 Website Tests both pass.

So this is pre-existing infrastructure noise in the link-check reporter, out of scope here. I did not change anything for it.

3. "Want me to review it?" reviewer-dispatch bot comment — non-actionable

Checkbox UI for dispatching the review fleet; nothing for the author to do.

---

Validation

No code changed in this pass, so there is no new behavior to cover and I did not re-run the suite. The authoritative evidence for the current head is CI on db18adef61 itself:

Check	Result
Build - client packages (Build Stage Build)	pass (16m 10s)
Build - client packages (Policy checks)	pass
repo-policy-check	pass
pnpm lint	pass
Build site / 🧪 Website Tests	pass
🔗 Broken Link Check	pass
merge status	MERGEABLE

Full ADO test legs (JestTest, RealsvcTinyliciousTest, StressTinyliciousTest, coverage) were still queued at the time of writing.

I attempted a corroborating local npm run build in packages/dds/tree; it failed with 'fluid-build' is not recognized because this is a fresh worktree with no node_modules (node_modules absent at both repo root and package level) — an environment gap, not a code failure. I deliberately did not install the full monorepo to re-prove a zero-line diff that CI already builds green. The targeted results quoted in the PR description (--fgrep "staged" → 150 passing, 0 failing) were produced on this same commit when it was authored.

The PR description already describes the optional-during-staging semantics and carries an accurate scope summary (17 changed files, verified via git diff --name-only c29323d9ca...HEAD) plus the review map, so it needed no edit either.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Re-review of db18adef61 after the view-kind redesign. The redesign itself is correct and my previous finding is genuinely fixed — the view field is now FieldKind.Optional, getStoredFieldKind only tightens on explicit opt-in, and the monotonic computeUpgradeSchema holds up (I traced the concurrent-upgradeSchema race and it is safe: a stale staged proposal rebases to emptyChange in sharedTreeChangeFamily.ts:196-205). What this pass found is a different problem: the write-side enforcement that the whole safety argument rests on does not cover the cursor/insertable construction paths, and the TSDoc for the one step a user must actively perform points at an API that cannot perform it.

Repository review skill: review invoked (repo-native .claude/skills/review, Standard mode — Breaker + API Analyst sub-agents plus the Inspector orchestrator pass), with .claude/skills/comparison-base used to resolve the base and .claude/skills/api-changes invoked because the diff changes two api-report/*.md files. Comparison base c29323d9ca (2 commits each side).

Automated checks:

• ADO Build - client packages (build 416942) on db18adef61: pass — tsc ESM+CJS, api-extractor, AreTheTypesWrong, JestTest, StressTinyliciousTest, Policy checks. Coverage/Realsvc jobs still pending at review time.
• repo-policy-check (build 416941): pass. pnpm lint: pass. Broken Link Check: pass (the earlier failure comment is stale).
• Local npm run build in packages/dds/tree: skipped — this worktree has no node_modules (fluid-build not on PATH) and a full pnpm install + 12-minute build would only re-derive what CI already proved on this exact SHA. Findings below are from source tracing, with each call path read end-to-end.
• API surface: verified via git diff c29323d9ca..HEAD -- "*.api.md". All deltas are @alpha-tier additions in tree.alpha.api.md / fluid-framework.alpha.api.md; computeUpgradeSchema, getAppliedStagedRequiredUpgrades and getStagedRequiredUpgrade correctly stay out of both reports (package-internal barrels only). No API Council sign-off required (@alpha, not @legacy). Changeset present and conformant.

Blocking issues:

• packages/dds/tree/src/simple-tree/api/create.ts:80 — unhydratedFlexTreeFromCursor builds fields with mapCursorFields against the stored schema and never consults the view FieldSchema, so a node whose staged-required field is absent is reconstructed silently. This is the path behind TreeAlpha.importVerbose, TreeAlpha.importCompressed and TreeBeta.clone. None of the three new objectNode.ts guards (:417 setField, :731 objectToFlexContent, :826 applyFieldChange) run on it. Failure mode: a version N+1 client clones or re-imports a node it legitimately read from a version N document (field empty) and inserts it — creating a brand-new empty value. Crucially there is no stored-schema backstop here the way there is for stagedOptional: because stagedRequired deliberately leaves the stored kind Optional during the staged phase, isFieldInSchema (prepareForInsertion.ts:186) accepts the empty field. The pre-existing TODO at prepareForInsertion.ts:181-184 already warns that "clone can result in unhydrated trees which can end up violating their stored schema ... just using the type safe APIs". Required fix: enforce at the field level where the view schema is available — walk ObjectNodeSchema.fields in the cursor/insertable construction path and throw the same UsageError when a field with getStagedRequiredUpgrade(...) !== false has no entry.

• packages/dds/tree/src/simple-tree/prepareForInsertion.ts:131 — same invariant, second hole: prepareForInsertionContextless calls unhydratedFlexTreeFromInsertable(undefined, schema) with no staged-required check, so TreeView.initialize(undefined) and TreeAlpha.create(sf.stagedRequired(T), undefined) create a document whose staged-required root is already empty. A staged-required root is an explicitly supported configuration (stagedRequiredUpgrades.ts:34-40 handles viewSchema.root; the new test at stagedSchemaUpgrade.spec.ts exercises one), and view.root = undefined on that same schema is rejected — so enforcement is inconsistent within one field. Required fix: apply the value === undefined && getStagedRequiredUpgrade(schema) !== false guard in unhydratedFlexTreeFromInsertable / prepareForInsertionContextless.

• Consequence of the two above, which is why they block rather than being polish: the changeset (.changeset/staged-required-field-migration.md:48-50) and the @privateRemarks at schemaFactoryAlpha.ts:130-133 both assert that "a stagedRequired client refuses to clear the field itself, so the remaining race is limited to concurrent clients from two rollout generations behind". That is the stated justification for step 3 being safe — and step 3 is an irreversible narrowing of the stored schema. As written, an N+1 client can itself introduce empty values, so a document can be out of schema immediately after tightening. Either close the gaps or correct the safety claim; shipping the claim as-is is the part I can't sign off on.

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:259-261 — step 3 of the rollout, the only step the user must actively perform, is documented as "explicitly enable the returned staged upgrade ... (see the includeStaged option of {Link (@link) extractPersistedSchema})". extractPersistedSchema (storedSchema.ts:56-70) only serializes a schema snapshot for inspection; it never upgrades a document. A user following this TSDoc literally will never tighten anything. The mechanism that actually makes the referenced TreeView.upgradeSchema tighten the field is StagedSchemaUpgradePolicy.includeStagedRequired supplied through TreeViewConfigurationAlpha.stagedUpgradePolicy (schematizingTreeView.ts:268-272) — which the changeset names correctly but the TSDoc does not. Required fix: name the policy mechanism as the primary opt-in and demote extractPersistedSchema to "snapshot-side equivalent". Relatedly, "the returned staged upgrade" has no referent, since stagedRequired returns a FieldSchemaAlpha, not a SchemaUpgrade; the token is only reachable via FieldSchemaAlpha.isStagedRequired, which lands in tree.alpha.api.md as // (undocumented).

• packages/dds/tree/src/simple-tree/simpleSchema.ts:312 — the new @alpha SimpleFieldSchema.isStagedRequired TSDoc reads "allowing the view schema (where the field is required) to be compatible with ...". The view field is Optional (schemaFactoryAlpha.ts:423; toStoredSchema.ts:479-482 only ever changes the stored kind). This is stale text from the pre-db18adef61 design and states the inverse of the contract, in IntelliSense, on an exported member. Required fix: reword to "(where the field is Optional)".

Minimum diff to ship: guard the staged-required emptiness check at the shared field-construction layer (unhydratedFlexTreeFromInsertable + the cursor path in create.ts) so initialize/create/importVerbose/importCompressed/clone are covered as objectToFlexContent already is, then fix the step-3 TSDoc to name StagedSchemaUpgradePolicy.includeStagedRequired and correct the isStagedRequired "required"→"Optional" wording. If you'd rather scope enforcement narrowly than close the gaps, the alternative acceptable diff is to keep the code as-is and rewrite the safety claim in the changeset and @privateRemarks to state honestly that same-generation clients can still introduce empty values via clone/import.

Non-blocking observations

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:409,425 — the props Omit sets are now cross-contaminated: stagedRequired's omits only "defaultProvider" | "stagedRequiredUpgrade", so sf.stagedRequired(T, { stagedOptionalUpgrade: u }) type-checks (and SchemaUpgrade is publicly constructible). Because getStoredFieldKind (toStoredSchema.ts:474) tests isStagedOptional first and returns early, such a field silently gets stored kind Required from day one — inverting the rollout contract with no error. The reverse is newly possible on stagedOptional too. Cheap fix while you're in the file: omit both keys from both factories. Non-blocking because it requires deliberate misuse of an alpha prop, not a defect on the shipped path.
• packages/dds/tree/src/simple-tree/core/toStored.ts:82 — permissive is still documented as "includes all staged schema upgrades" (and :51, :63) but now sets includeStagedRequired: () => false at :118. The rationale lives only in a code comment; consider promoting it into the exported TSDoc, since "rehearse future document shapes" is exactly the advertised use case that silently no longer applies to staged-required fields.
• packages/dds/tree/src/simple-tree/fieldSchema.ts:430 — the isStagedRequired getter is undocumented (surfaces as // (undocumented) in the API report), leaving no IntelliSense path from sf.stagedRequired(T) to the SchemaUpgrade that enabledStagedUpgrades needs. Same gap exists for isStagedOptional, so this is consistency-neutral, but worth fixing for the newly-documented rollout.
• Positive, no action: the test suite is genuinely good — it asserts the stored projection on both sides of the opt-in, walks all three rollout phases, and pins the non-reversion property explicitly. Scope (17 files) is legitimately atomic and the PR description carries the required review map and atomicity justification, so breadth is not a finding.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review fixes — staged required invariant now enforced on all construction paths

Addresses the three findings from the previous review at db18adef61. Pushed as dcc52965d5.

Blocking finding 1 — the "a staged client never creates an empty value" invariant was unenforced on the cursor / insertable construction paths

Verified as valid. Before this commit, the invariant was only enforced in objectNode.ts (objectToFlexContent, setField, applyFieldChange). Two paths bypassed it entirely, and because stagedRequired deliberately keeps the stored field kind Optional, neither isFieldInSchema nor the stored schema could act as a backstop:

• create.ts unhydratedFlexTreeFromCursor — it walks only the fields present in the cursor against the stored schema, so an absent staged-required field was silently accepted. Reachable via TreeAlpha.importVerbose, TreeAlpha.importCompressed and TreeBeta.clone.
• The root-field path — unhydratedFlexTreeFromInsertable returned undefined for data === undefined after only checking kind !== FieldKind.Optional, which a staged-required field passes. Reachable via TreeAlpha.create, TreeBeta.importConcise and TreeView.initialize (through prepareForInsertionContextless).
• TreeAlpha.importVerbose additionally short-circuits on data === undefined before reaching createFromCursor, so it needed its own check.

Fixes:

File	Change
packages/dds/tree/src/simple-tree/api/create.ts	New checkStagedRequiredFieldsPresent called from unhydratedFlexTreeFromCursor; looks the view node schema up in Context.schema and throws a UsageError for any staged-required field that is absent or empty in the cursor. Also rejects cursor === undefined for a staged-required root in createFromCursor.
packages/dds/tree/src/simple-tree/unhydratedFlexTreeFromInsertable.ts	Rejects undefined root content for a staged-required field.
packages/dds/tree/src/shared-tree/treeAlpha.ts	importVerbose rejects undefined for a staged-required root before its early return.
packages/dds/tree/src/simple-tree/fieldSchema.ts	getStagedRequiredUpgrade widened from FieldSchema to ImplicitFieldSchema so it can be used on the root-field paths.

Behavioural consequence, now documented explicitly in the TSDoc and changeset: importing or cloning legacy content in which the field is empty throws rather than silently reintroducing an empty value. That is the intended trade-off — it fails loudly instead of quietly falsifying the precondition for the irreversible step-3 narrowing.

Blocking finding 2 — schemaFactoryAlpha.ts documented step 3 against extractPersistedSchema

Verified as valid. extractPersistedSchema only dumps a schema snapshot for inspection; it cannot upgrade a document. The step-3 remarks now name the real mechanism — configure the view with a StagedSchemaUpgradePolicy whose includeStagedRequired returns true for the field's upgrade (e.g. StagedSchemaUpgradePolicy.enabledStagedUpgrades) and call TreeView.upgradeSchema — and mention extractPersistedSchema only as the snapshot-side opt-in. The changeset step 3 was corrected the same way.

Blocking finding 3 — simpleSchema.ts:312 TSDoc said the view field is required

Verified as valid. Corrected to "the field is Optional during the staged phase".

Tests

New cases in packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:

• rejects creating empty content for a staged required root — TreeAlpha.create(schemaB, undefined), view.initialize(undefined) and TreeAlpha.importVerbose(schemaB, undefined) all throw; providing a value still works.
• rejects importing or cloning content where a staged required field is empty — TreeAlpha.importVerbose of an object missing the staged-required field throws; TreeBeta.clone of a legacy hydrated node whose field is empty throws; both succeed once the field is populated.

Each new assertion was confirmed to fail before the corresponding fix (the importVerbose(schema, undefined) case is what surfaced the treeAlpha.ts early-return gap).

Validation

This environment could not reach the npm registry, so pnpm install was unavailable. Dependencies were satisfied by building the affected workspace packages from this worktree and linking them into packages/dds/tree/node_modules; commands were then run directly against the local typescript / mocha / biome binaries.

• tsc --noEmit -p packages/dds/tree/tsconfig.json — 11 errors, identical to the pre-change baseline (verified by stashing the diff and re-running). All 11 are in arrayNode.ts/treeDataStore.ts/treeFactory.ts and are caused by stale sibling-package type output in this environment, not by this change. No new errors.
• tsc --noEmit -p packages/dds/tree/src/test/tsconfig.json — clean for stagedSchemaUpgrade.spec.ts; the remaining errors are in unrelated specs from the same stale-dependency cause.
• mocha lib/test/simple-tree/api/stagedSchemaUpgrade.spec.js — 22 passing, 0 failing (all 8 staged required upgrade cases, plus the pre-existing staged-allowed-type and staged-optional suites).
• mocha --recursive "lib/test/simple-tree/**/*.spec.js" — 3191 passing, 34 pending, 1 failing. The single failure is snapshotCompatibilityChecker > write current view schema snapshot, whose actual/expected differ only by Windows path separators (src\test\schemaSnapshots vs src/test/schemaSnapshots) — a pre-existing platform artifact unrelated to this change.
• biome format --write on all changed files — clean.
• eslint on the changed source files — no new errors; the one reported error is import-x/no-unresolved for a pre-existing @fluidframework/runtime-utils import, an artifact of the hand-wired node_modules above.

No API report regeneration was needed: the changes are internal or TSDoc-only, and Fluid's API reports do not include doc comments.

Scope: 8 changed files in this commit, 19 in the PR overall, one concern (staged optional-to-required field migration). The atomicity constraint is that the API, its stored-schema projection, the runtime invariant enforcement, the API reports and the changeset must land together — a document upgraded by a partial version of this feature could not be safely opened by the completed version.

Repo harnesses used

• Runtime: copilot
• Instructions: .github/copilot-instructions.md

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review pass on dcc52965d5 — no code changes; one stale PR-body claim corrected

I re-checked every comment on this PR against the live branch head (dcc52965d5) before touching anything. No comment on this PR is currently actionable against the code, so this pass makes no commit. The one real defect I did find was in the PR description, which overstated how the current head had been validated; that is now corrected. Evidence per item below.

---

1. Noah Encke (@noencke) — "the field is required — shouldn't it be optional during this phase?" — ✅ valid, already fixed

This was correct, and it was implemented in db18adef61. Verified still true at the current head:

• View field kind is FieldKind.Optional (schemaFactoryAlpha.ts), so reads are typed T | undefined and never throw.
• getStoredFieldKind (toStoredSchema.ts) keeps the stored field Optional and tightens to Required only on explicit opt-in.
• The target state is enforced on the write side at runtime, exactly as you described for stagedOptional — objectNode.ts (setField, applyFieldChange, objectToFlexContent), plus the construction paths closed in dcc52965d5 (create.ts, unhydratedFlexTreeFromInsertable.ts, treeAlpha.ts).

The review thread is already resolved; I have not reopened it.

2. github-actions — "🔗 Found some broken links! 💔" — ❌ not attributable to this PR

I treated this as a claim and checked it rather than chasing it:

• This PR changes zero website files. git diff --name-only c29323d9ca...HEAD returns 20 files, all under packages/dds/tree, packages/framework/fluid-framework/api-report, and .changeset. Nothing under website/ or docs/.
• The doc site cannot see this branch's TSDoc. website/package.json builds API docs via build:api-documentation → download-doc-models, and website/infra/download-doc-models.mjs:23 pulls the API model from https://storage.fluidframework.com/api-extractor-json/latest — published artifacts, not this worktree. So a TSDoc {@link} added here cannot influence the rendered site or its links in this run.
• The bot output itself lists no broken URLs — it only shows check-links exiting 1 with an empty report, which is a harness/infra symptom rather than a located link.

No change made. If a maintainer can point at a specific broken URL that traces to this branch, I'll fix it immediately.

3. github-actions — bundle size comparison — ℹ️ informational, expected

Non-actionable. The reported sharedTree.js delta (+1349 parsed / +329 gzip) is the expected cost of a new alpha schema API plus its stored-schema projection and runtime guards. Nothing here indicates a regression to act on.

4. github-actions — "Want me to review it?" reviewer-selection checklist — ℹ️ non-actionable

A bot prompt awaiting a human tick. No author action required.

---

What I actually changed: the PR description

The Validation section was pinned to db18adef61 and presented a table whose top rows claim npm run build Build succeeded. That table was produced two commits ago; dcc52965d5 then changed 8 more files and was validated by a different, weaker method. Left as-is, the description overstated the evidence behind the current head — so I made targeted edits (prose voice, section structure and title untouched):

• The Validation lead-in now says the table was produced at db18adef61, with the dcc52965d5 follow-up validated separately.
• Added an honest paragraph recording how dcc52965d5 was actually validated, and that CI is authoritative for a clean-install build.

Scope: 20 changed files, one concern was already accurate against git diff --name-only c29323d9ca...HEAD (20 files) and is unchanged, as is the review map.

Validation performed this pass

This environment has no npm registry access (registry.npmjs.org fails TLS; corepack cannot fetch pnpm), and the worktree had no node_modules, so a clean install was impossible. I validated against dependencies borrowed read-only from a local checkout (junctioned in, then removed; the worktree is clean and nothing was committed):

• tsc --noEmit -p packages/dds/tree/tsconfig.json — 11 errors, zero in any file this PR touches. All 11 are in arrayNode.ts, treeDataStore.ts and treeFactory.ts, and every one is a has no exported member failure against sibling packages whose build output in that checkout dates to 2 July — i.e. stale dependency artifacts, not this change.
• tsc -p packages/dds/tree/src/test/tsconfig.json — stagedSchemaUpgrade.spec.ts compiles clean; the 31 errors are confined to 7 unrelated specs from the same stale-dependency cause.
• Emit succeeded, but mocha could not run: the borrowed core-utils build predates clamp, so the suite fails at module load. Rebuilding the full sibling dependency graph was not proportionate for a pass that changes no code, so I stopped rather than report a green run I did not get. The 22 passing figure for this head comes from the earlier pass and is reported as such in the description.
• mergeable: MERGEABLE, reviewDecision: REVIEW_REQUIRED, head dcc52965d5.

CI is the authoritative signal for a clean-install build and test run on this branch.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Repository review skill: review invoked (Standard mode: Breaker + API Analyst sub-agents + Inspector orchestrator), with comparison-base executed as its required sub-workflow.

Base resolution: microsoft/FluidFramework:main at c29323d9ca ("fix(devcontainer): document scheduled nested prebuilds (#27945)"). Review side 3 commits, target side 12 commits — no substantial divergence. Head reviewed: dcc52965d5. 20 files, +842/-14. MERGEABLE.

Automated checks:

• ADO Build - client packages (build 417005, ran against head dcc52965d5): fail — npm run ci:build exited 255. Root cause is api-extractor run failing in both @fluidframework/tree and fluid-framework with 9 ae-unresolved-link errors, all from doc comments added by this PR.
• ADO Build - client packages (Policy checks): pass.
• Local api-extractor / build:api-reports: skipped — this worktree has no node_modules or lib for packages/dds/tree, and dependency install could not complete (Corepack/pnpm download failed). The ADO run above is the authoritative evidence; the root cause was independently confirmed by static inspection (see below).
• Static verification of the failure: StagedSchemaUpgradePolicy is declared twice in packages/dds/tree/src/simple-tree/core/toStored.ts — as an interface (line 13) and as a const (line 106). Pre-existing docs in that same file already disambiguate correctly ({@link (StagedSchemaUpgradePolicy:interface)} at line 43, {@link StagedSchemaUpgradePolicyFactory.restrictive} at line 49), so the required pattern is established in-repo.

Blocking issues:

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:264 — Two ambiguous TSDoc links, {@link StagedSchemaUpgradePolicy} and {@link StagedSchemaUpgradePolicy.includeStagedRequired}. API Extractor cannot resolve them because the name has both an interface and a variable declaration, so it errors with (ae-unresolved-link) and fails ci:build. Because the stagedRequired doc block is re-emitted via {@inheritdoc} on the two SchemaFactoryAlpha members, each bad link is reported three times (reported at schemaFactoryAlpha.ts:289, :693, :698), producing 9 errors across the tree and fluid-framework API Extractor runs. Required fix: use member-reference selectors — {@link (StagedSchemaUpgradePolicy:interface)} and {@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}.
• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:265 — Same failure mode for {@link StagedSchemaUpgradePolicy.enabledStagedUpgrades}. enabledStagedUpgrades is declared on StagedSchemaUpgradePolicyFactory (toStored.ts:99), not on the StagedSchemaUpgradePolicy interface, so the selector alone is not enough — the link target must be redirected. Required fix: {@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades | StagedSchemaUpgradePolicy.enabledStagedUpgrades}, matching the existing convention at toStored.ts:53.

Minimum diff to ship: fix the three {@link} targets at schemaFactoryAlpha.ts:264-265 as specified above and confirm Build - client packages goes green.

Note on outstanding human feedback (not counted as my findings): two of Noah Encke (@noencke)'s inline threads are still unresolved — treeAlpha.ts:914 (error-message wording) and create.ts:58 (whether the staged-required check belongs in isFieldInSchema). Please resolve those with the author before merging; I am deliberately not resolving another reviewer's threads.

Non-blocking observations

• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:78 — {@link StagedSchemaUpgradePolicy.includeStagedRequired} is ambiguous in the same way, but does not currently fail the build because computeUpgradeSchema is not exported from the alpha entrypoint (correctly — it is absent from both .api.md reports). It becomes a build break the moment this helper is promoted to public API. Worth fixing in the same pass: {@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}.
• packages/dds/tree/src/simple-tree/core/toStored.ts:51 — The StagedSchemaUpgradePolicyFactory doc still says permissive means "all staged upgrades enabled", but this PR makes permissive.includeStagedRequired return false (toStored.ts:113). The exclusion is well justified and well commented at the implementation site; the summary bullet (and the permissive example at line 63-64) is now the only stale part.
• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:294 — The props type is Omit<FieldPropsAlpha<T>, "defaultProvider" | "stagedRequiredUpgrade">, which still permits passing stagedOptionalUpgrade. If a caller did that, getStoredFieldKind (toStoredSchema.ts:473) takes the staged-optional branch first and would project the field to Required under a restrictive policy — i.e. tightening without the explicit includeStagedRequired opt-in. This requires a caller to deliberately pass a contradictory internal-looking prop, and stagedOptional has the mirror-image gap today, so I am not blocking on it. Cheap hardening if you want it: also Omit the opposite staged marker in both factories.
• Scope: 20 files / +842 / -14, all in packages/dds/tree plus the two generated .api.md reports and one changeset. This is legitimately atomic — new API surface, its stored-schema plumbing, its runtime write guards, and its tests cannot be split into independently shippable slices. No unrelated changes detected. No decomposition finding.
• Test coverage is genuinely good: stored-kind projection before/after opt-in, compatibility across all three rollout phases, the monotonicity guarantee via computeUpgradeSchema on an already-tightened stored schema, read paths for absent values, construction/assignment/clear rejection, stagedRequiredRecursive, and import/clone rejection for importVerbose / create / clone / initialize.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review feedback addressed — 0267a38ac8

Two new threads since dcc52965d5; both verified against the live branch before editing.

1. treeAlpha.ts:914 — "How about just undefined provided for staged required field." — fixed

Valid. TreeAlpha.importVerbose threw a three-clause explanation right below a terse sibling throw. Now:

if (field.kind !== FieldKind.Optional) {
    throw new UsageError("undefined provided for non-optional field.");
}
if (getStagedRequiredUpgrade(field) !== false) {
    throw new UsageError("undefined provided for staged required field.");
}

stagedSchemaUpgrade.spec.ts:910 updated to match (/undefined provided for staged required field/).

2. create.ts:58 — "Should we be doing this validation in isFieldInSchema instead?" — answered, thread left open

I investigated and concluded it can't live there; full evidence is on that thread. Short version:

• isFieldInSchema (schemaChecker.ts:201) takes TreeFieldStoredSchema + SchemaAndPolicy, and SchemaPolicy is just { fieldKinds } (core/schema-stored/schema.ts:128-137) — neither carries staged-ness.
• The stored kind of a staged required field is deliberately Optional until includeStagedRequired is opted into (toStoredSchema.ts:478-483), so from the checker's view a missing child is an in-schema empty optional field. This is the asymmetry with staged allowed types, whose restriction is expressible in stored schema and so is caught there for free.
• schemaChecker.ts sits in feature-libraries, below simple-tree where isStagedRequired lives (simpleSchema.ts:328); its own comment at schemaChecker.ts:164-167 warns this layer is stored-schema-only.
• The one variant that would route through isFieldInSchema — passing a tightened stored schema — breaks TreeBeta.clone, which intentionally derives its context from the source document's stored schema to preserve unknown optional fields (treeBeta.ts:346-352).

I left that thread active rather than resolving it, since it's a design call that's yours to make.

Validation

Run from packages/dds/tree in the task worktree:

command	result
tsc -p tsconfig.json	11 errors, all pre-existing (treeDataStore.ts, treeFactory.ts, arrayNode.ts), none in touched files
tsc -p src/test/tsconfig.json	31 errors in 7 unrelated files, all pre-existing; stagedSchemaUpgrade.spec.ts compiles clean
biome check --formatter-enabled=true on both changed files	clean
direct check of emitted lib/shared-tree/treeAlpha.js	message is exactly "undefined provided for staged required field.", the spec regex matches it, old string gone

Environment limitation, stated plainly: mocha could not be run in this worktree. pnpm install is impossible here (the npm registry fails TLS handshake), so validation used the only available node_modules, whose sibling workspace lib/ artifacts are stale relative to this branch — module load fails on core-utils clamp, then shared-object-base defineDataStore, then runtime-utils DataStoreKindImplementation. Those are package-wiring staleness, not failures in the code under test; I rebuilt core-utils and shared-object-base from this worktree to push through two of them before stopping. The pre-existing tsc errors have the same root cause. The 22 tests in stagedSchemaUpgrade.spec.ts (incl. the assertion I updated) passed on the previous head and the only behavior change is the literal string, which I verified directly in the emitted JS. The PR pipeline covers the full suite.

No API report or changeset change needed — the diff is one internal error message plus its test assertion. Scope is unchanged at 20 changed files.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Repository review skill: review invoked (Standard depth), including its mandatory comparison-base sub-skill.

Base resolved to microsoft/FluidFramework:main at c29323d9ca (single merge-base; 4 review commits / 15 target commits). Head reviewed: 0267a38ac8. PR reports MERGEABLE — no conflicts.

Automated checks:

• pnpm install --filter "@fluidframework/tree...": failed — environment only, not the PR. Corepack cannot reach registry.npmjs.org (ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE), and this worktree has no node_modules/lib.
• build / tsc / mocha / eslint / build:api-reports: skipped — blocked by the install failure above. The finding below is therefore from source tracing, not from a failing test run.

Blocking issues:

• packages/dds/tree/src/shared-tree/schematizingTreeView.ts:274 — step 3 of the documented migration cannot be performed through the public API, so the feature has no terminal state. With the opt-in enabled, computeUpgradeSchema returns a stored schema whose field kind is Required while the document's stored kind is still Optional. upgradeSchema() then gates on allowsRepoSuperset(defaultSchemaPolicy, storedSchema, newSchema). That call resolves to allowsFieldSuperset (feature-libraries/modular-schema/comparison.ts:155-162), which for differing kinds returns required.options.allowMonotonicUpgradeFrom.has("Optional") — and required declares allowMonotonicUpgradeFrom: new Set([identifierFieldIdentifier]) (feature-libraries/optional-field/requiredField.ts:59), which does not contain optional. So the guard fails and upgradeSchema() throws UsageError("Existing stored schema cannot be upgraded to the requested schema…") — precisely the call that schemaFactoryAlpha.ts step 3 and the changeset step 3 instruct applications to make. Required fix: apply the staged-required tightening through the existing non-superset escape hatch — checkout.updateSchema(newSchema, true) (shared-tree/treeCheckout.ts:288 and :1440, already used for the analogous required-root narrowing at shared-tree/schematizeTree.ts:86) — gated so it is used only when the sole non-superset delta is a staged-required upgrade the caller explicitly enabled.

• packages/dds/tree/src/simple-tree/api/schemaCompatibilityTester.ts:49 — same root cause, second user-visible symptom: canUpgrade = allowsRepoSuperset(policy, stored, wouldUpgradeTo) flips to false as soon as an application configures includeStagedRequired to return true. So TreeView.compatibility.canUpgrade reports false for the very configuration the docs tell users to adopt, and isEquivalent (line 53) follows it to false. Required fix: make the compatibility computation treat an opted-in staged-required tightening as upgradable rather than as a forbidden narrowing.

• packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:713 — the defect above is invisible because no test drives the tightening through the public API. The only place the stored schema is tightened is stored.apply(tightenedStoredSchema(schemaB)) against a TestSchemaRepository, and the comment on line 712 ("This is deliberately not a superset change, so it does not go through tryUpdateRootFieldSchema") documents the bypass rather than covering the real path. Every other new test exercises construction/import/clear guards, none calls TreeView.upgradeSchema(). Required fix: add a test that configures a view with StagedSchemaUpgradePolicy.enabledStagedUpgrades(<the stagedRequired upgrade>), calls TreeView.upgradeSchema() on a real tree, and asserts the stored field kind becomes Required and that a subsequent sf.required (step 4) view can open the document.

Minimum diff to ship: let SchematizingSimpleTreeView.upgradeSchema() apply an explicitly opted-in staged-required tightening via checkout.updateSchema(newSchema, true) (with the matching canUpgrade fix), covered by a test that performs step 3 end-to-end through TreeView.upgradeSchema().

Otherwise this is well-built: the WeakMap-keyed toStoredSchema cache means the per-document options object in computeUpgradeSchema (stagedRequiredUpgrades.ts:90) does not leak, the monotonicity guard correctly prevents a staged client from reverting an already-tightened field, the write-side guards cover the object-literal, proxy-set, deleteProperty, insertable, cursor-import and root paths, and both API reports are regenerated consistently. Scope (20 files) is atomic for the feature and carries its review map, so breadth is not a finding.

Non-blocking observations

• packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts:823 — the new guard in applyFieldChange runs before the existing value === undefined && inner.tryGetField(...) === undefined early return, so delete node.field on an already-empty staged-required field now throws instead of being a silent no-op. Defensible and arguably desirable, just noting it is a slightly stricter contract than "must not clear it".
• packages/dds/tree/src/simple-tree/simpleSchema.ts — the new TSDoc says false and undefined are both "not staged required" but then advises comparing !== false, which classifies undefined as staged. Harmless in practice (discrepancies.ts:350 asserts view instanceof FieldSchemaAlpha, so the getter never yields undefined), but the wording is self-contradictory as written.
• The human review thread on packages/dds/tree/src/simple-tree/api/create.ts:58 (whether this validation belongs in isFieldInSchema) is still open and is not mine to resolve.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Repository review skill: review invoked (Standard mode: Breaker + API Analyst sub-agents + Inspector pass). Comparison base resolved to the PR base c29323d9ca (the local main ref in this worktree is 221 commits stale, so a main... three-dot diff reports 200 unrelated files; the real PR diff is 20 files, +840/-14 at head 0267a38ac8).

Automated checks:

• ADO Build - client packages (build 417433): fail — API Extractor ae-unresolved-link, 3 errors in @fluidframework/tree and the same 3 again in fluid-framework.
• Local npm run build / npm run test: skipped — this worktree has no node_modules (verified: packages/dds/tree/node_modules and root node_modules both absent) and no registry access, so nothing could be run locally. CI is the authoritative signal here, and it is red.
• Diff/source inspection of all 20 changed files + call-path tracing of allowsRepoSuperset, allowsFieldSuperset, and the allowMonotonicUpgradeFrom sets: pass (performed).

Blocking issues:

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:264-265 — the build is red. {@link StagedSchemaUpgradePolicy} is ambiguous because that name is declared twice in simple-tree/core/toStored.ts: as an interface (line 13) and as a const (line 106). API Extractor fails with ae-unresolved-link at :289 (the stagedRequired declaration these docs sit above) and again at :693 and :698, which inherit the same comment via {@inheritdoc SchemaStaticsAlpha.stagedRequired}. Required fix — three replacements, all in the stagedRequired TSDoc block; the repo already uses exactly this selector syntax at toStored.ts:43:

  • {@link StagedSchemaUpgradePolicy} → {@link (StagedSchemaUpgradePolicy:interface)}
  • {@link StagedSchemaUpgradePolicy.includeStagedRequired} → {@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}
  • {@link StagedSchemaUpgradePolicy.enabledStagedUpgrades} → {@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades} — this one is doubly wrong: enabledStagedUpgrades is not a member of the StagedSchemaUpgradePolicy interface at all, it lives on the factory (precedent: toStored.ts:53).

• packages/dds/tree/src/shared-tree/schematizingTreeView.ts:274 — the feature's documented step 3 cannot be performed through the public API. upgradeSchema() gates the write on allowsRepoSuperset(defaultSchemaPolicy, storedSchema, newSchema). For a field going Optional → Required that reduces to allowsFieldSuperset(..., monotonicOnly = true) (comparison.ts:145, :161-162), i.e. required.options.allowMonotonicUpgradeFrom.has(optional). In feature-libraries/optional-field/requiredField.ts:59 that set is new Set([identifierFieldIdentifier]) — it does not contain optional. So allowsRepoSuperset returns false and upgradeSchema() throws UsageError("Existing stored schema cannot be upgraded to the requested schema…") (schematizingTreeView.ts:275-277). That makes the migration step this PR documents in schemaFactoryAlpha.ts:238-243 and in the changeset ("configure a StagedSchemaUpgradePolicy whose includeStagedRequired returns true … and call TreeView.upgradeSchema") unreachable: the stored field can never actually become Required via the shipped API. Required fix: either make the opted-in Optional → Required narrowing pass the guard for exactly the SchemaUpgrade tokens the policy enables, or correct the public TSDoc + changeset to document the mechanism that actually works. If the guard is relaxed, it must be paired with a decision about existing document content — nothing in upgradeSchema validates content against the tightened schema, so a naive relaxation can leave a document with an empty field under a Required stored schema.

• packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:635-1171 — the test suite structurally avoids the path that would have caught the above. The describe("staged required upgrade") block never calls view.upgradeSchema() and never builds a view with StagedSchemaUpgradePolicy.enabledStagedUpgrades(...); it tightens by writing the schema repository directly via stored.apply(tightenedStoredSchema(...)) at :713, whose own comment concedes "This is deliberately not a superset change, so it does not go through tryUpdateRootFieldSchema." Both sibling suites do drive the real API (staged allowed type upgrade at :144/:160/:208, staged optional upgrade at :414/:431/:459/:488), because required → optional is a superset. Required fix: add an end-to-end test that opts in via enabledStagedUpgrades(theUpgrade) and calls TreeView.upgradeSchema(), asserting the stored field kind actually becomes Required. Note checkSchemaCompatibility's canUpgrade is only asserted at :694 under the default restrictive policy (where the upgrade is a no-op), never with includeStagedRequired enabled.

Also outstanding, and not mine to close: Noah Encke (@noencke)'s unresolved thread on create.ts:58 asking whether this validation belongs in isFieldInSchema rather than alongside it.

Minimum diff to ship: fix the three {@link} selectors in the stagedRequired TSDoc to turn CI green, and make TreeView.upgradeSchema() actually apply an opted-in staged-required tightening (or correct the docs to describe the real mechanism) with an end-to-end test covering that path.

Non-blocking observations

• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:78 — {@link StagedSchemaUpgradePolicy.includeStagedRequired} carries the same ambiguity, but this file's exports appear in no API report so API Extractor never validates it and it does not break the build. Worth fixing for consistency while you are in there.
• packages/dds/tree/src/simple-tree/toStoredSchema.ts:738-750 — if a field ever carried both stagedOptionalUpgrade and stagedRequiredUpgrade, staged-optional silently wins because it is tested first. Not reachable through the public API (it would require passing stagedOptionalUpgrade through stagedRequired's props, which the Omit discourages, plus a fabricated second SchemaUpgrade), so this is a candidate for an assert rather than a defect.
• Scope: 20 files with a written atomicity justification and a five-step review map in the PR body — the scope policy is met, and the change is legitimately atomic. Not a finding.
• Things I checked that came back clean, so they do not need re-litigating: the construction-path enforcement is complete (objectNode.setField/objectToFlexContent/applyFieldChange, unhydratedFlexTreeFromInsertable, createFromCursor + checkStagedRequiredFieldsPresent, treeAlpha.importVerbose), and the defaultProvider: () => [] is correctly shadowed by those guards; getAppliedStagedRequiredUpgrades monotonicity is sound, including ObjectNodeStoredSchema.getFieldSchema returning storedEmptyFieldSchema (kind forbidden) for an absent field so it is never misread as Required; the toStoredSchema cache-bypass reasoning is accurate; TreeView.initialize correctly still yields Optional via toInitialSchema; the discrepancies.ts !== false suppression cannot see the undefined tri-state because that function asserts view instanceof FieldSchemaAlpha; the new guards cannot fire on remote-op rebase; no hex assert codes were added; and the API surface (optional includeStagedRequired? on an @input interface, release tags, naming, the Recursive cast, internal-only exports, changeset packages/bump/prose) matches existing stagedOptional precedent throughout.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review-feedback pass at 0267a38ac8 — no code change required

I re-checked every open item on this PR against the live branch head (0267a38ac882163d38b17d7a3e177bfe62b36c60). All three are either already satisfied, informational, or a CI infrastructure flake, so I made no commits this round. Evidence below.

1. 🔗 "Found some broken links!" — not caused by this PR; CI infra flake, no broken link was ever reported

The comment body is misleading because the workflow prints a canned "broken links" template for any non-zero exit of the link-check step, and the step failed before the checker ever ran.

Full step log from the run that produced it (Website validation run 31825544008, job 🔗 Broken Link Check, step Check for broken links):

17:53:17Z  $ start-server-and-test "npm run serve -- --no-open" 3000 check-links
17:53:18Z  [SUCCESS] Serving "build" directory at: http://localhost:3000/
17:58:17Z  Error: Timed out waiting for: http://127.0.0.1:3000
               at .../node_modules/wait-on/lib/wait-on.js:131:31
17:58:17Z  [ELIFECYCLE] Command failed with exit code 1.

Reading that: Docusaurus started fine and bound localhost:3000, but wait-on polled 127.0.0.1:3000 and timed out after 5 minutes, so npm run check-links was never executed. Zero links were checked, therefore zero links were found broken. This is the classic localhost → IPv6 ::1 vs. hard-coded IPv4 127.0.0.1 binding race in start-server-and-test, and it is environmental.

Corroborating evidence that it is unrelated to this branch:

• This PR touches no website content. gh pr diff 27952 --name-only returns 20 files: 1 changeset, 2 generated api-report/*.api.md, and 17 files under packages/dds/tree/src/. Nothing under website/ or docs/.
• The same sticky linkreport comment lands on unrelated PRs. PR #27948 (a container-runtime compatibility change, also with no website files) carries an identical "Found some broken links!" comment.
• The link-check step is continue-on-error: true (.github/workflows/website-validation.yml:148), which is why Website validation still reports success on this branch — it is not gating this PR.

No change made. If the flake is worth fixing, that belongs in website-validation.yml / the ci:check-links script (e.g. wait-on targeting localhost rather than 127.0.0.1), which is out of scope for a SharedTree schema API PR.

2. 📦 Bundle size comparison — informational, nothing to act on

The comment states "Pending — the PR's CI build hasn't completed yet." It is a placeholder that the bot rewrites when the build finishes. No author action.

3. "the field is required" (noencke, changeset thread) — already implemented, thread resolved

Confirmed still true at the current head, not just at the commit where it was addressed:

• schemaFactoryAlpha.ts:429-443 — stagedRequired returns FieldSchemaAlpha<FieldKind.Optional, …> and calls createFieldSchema(FieldKind.Optional, t, …). The view field kind is Optional, exactly as you asked, so the read type is T | undefined.
• The write side is a runtime UsageError rather than a compile error, mirroring stagedOptional: construction without a value, assigning/inserting undefined, and delete all throw.
• The changeset (.changeset/staged-required-field-migration.md:17-27) now states the field "is also Optional in the view schema (and reads are typed T | undefined) during this phase" and explains the mapped-type limitation in your own terms.

Still open and intentionally left open

The create.ts:58 thread ("should this validation live in isFieldInSchema?") remains active by design. I posted a detailed rationale for keeping the check in simple-tree — the short version is that isFieldInSchema only sees stored schema, and a staged-required field's stored kind is deliberately Optional, so there is nothing there to check against; the one variant that would route through it regresses TreeBeta.clone for documents with unknown optional fields. That is a judgement call that is yours to make, so I have not resolved it.

Validation

No source files were modified this round, so no build or test run was warranted:

git status --porcelain   →   clean (only the untracked minions worktree marker)
git log --oneline -1     →   0267a38ac8

The validation of record is therefore the run from the previous commit, unchanged: stagedSchemaUpgrade.spec 22 passing / 0 failing; simple-tree suite 3191 passing / 34 pending / 1 failing (snapshotCompatibilityChecker, a pre-existing Windows path-separator artifact in a file this PR does not touch). PR body scope line verified accurate: 20 changed files, matching gh pr diff --name-only.

---

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Reviewed at head 0267a38ac8 against microsoft/FluidFramework:main base c29323d9ca (20 changed files, 854 lines).

Repository review skill: review invoked (Standard mode — Breaker + API Analyst sub-agents + Inspector pass), plus comparison-base for base resolution.

Automated checks:

• ADO Build - client packages (build 417433, the PR's own CI at this head): fail — npm run ci:build exited 255. Failed tasks: @fluidframework/tree: api-extractor run and fluid-framework: api-extractor run (9 ae-unresolved-link errors).
• repo-policy-check, Website validation, Validate CODEOWNERS, license/cla: pass.
• Local build/test: skipped — this worktree has no node_modules (no registry access), so no local build, lint, or mocha run was possible. CI at this exact head is the authoritative signal and is red.

Blocking issues:

• packages/dds/tree/src/shared-tree/schematizingTreeView.ts:274 — The feature's documented step 3 throws; the optional→required tightening cannot be applied through the public API. With includeStagedRequired enabled, computeUpgradeSchema (line 268) correctly returns a schema whose field kind is Required while the stored kind is Optional. Line 274 then calls allowsRepoSuperset(defaultSchemaPolicy, stored, newSchema), which reaches allowsFieldSuperset with the default monotonicOnly = true (feature-libraries/modular-schema/comparison.ts:203, :161-162) and evaluates required.options.allowMonotonicUpgradeFrom.has("optional"). That set is new Set([identifierFieldIdentifier]) (feature-libraries/optional-field/requiredField.ts:57-59) and does not contain optional, so the check returns false and line 275 throws UsageError("Existing stored schema cannot be upgraded to the requested schema…"). Both schemaFactoryAlpha.ts:262-268 and .changeset/staged-required-field-migration.md:26-30 instruct users to perform step 3 by enabling includeStagedRequired and "calling TreeView.upgradeSchema, which tightens the stored field kind from Optional to Required" — that call throws. The pre-existing test at stagedSchemaUpgrade.spec.ts:471-501 independently confirms this narrowing shape throws with /cannot be upgraded to the requested schema/. Required fix: either allow this explicitly opted-in tightening through upgradeSchema() (e.g. a stagedRequired-aware relaxation of the superset guard, so the narrowing is permitted only for upgrades the policy opted into), or correct the TSDoc and changeset to describe the mechanism that is actually supported. Do not ship an alpha API whose headline workflow throws.

• packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:635-737 — No test exercises the documented step 3 end-to-end, which is why the above defect is not caught. Every test in the new staged required upgrade block reaches the tightened state via stored.apply(tightenedStoredSchema(schemaB)) (line 713) against a TestSchemaRepository, and the comment on line 712 states outright that this "is deliberately not a superset change, so it does not go through tryUpdateRootFieldSchema". No new test constructs a TreeViewConfigurationAlpha with stagedUpgradePolicy: StagedSchemaUpgradePolicy.enabledStagedUpgrades(<the stagedRequired upgrade>) over an Optional stored schema and calls view.upgradeSchema() — the exact configuration the docs prescribe. viewSchemaB at line 691 is built with no stagedUpgradePolicy, so it only covers the no-op path. Required fix: add a test that performs the full rollout through TreeView.upgradeSchema() on a real TestTreeProviderLite tree and asserts the stored field kind becomes Required and that other clients behave as documented.

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:264-265 — Breaks the build: 9 ae-unresolved-link api-extractor errors fail both @fluidframework/tree and fluid-framework. StagedSchemaUpgradePolicy has two declarations in simple-tree/core/toStored.ts — the interface at line 13 and the const at line 106 — so the bare {@link StagedSchemaUpgradePolicy} and {@link StagedSchemaUpgradePolicy.includeStagedRequired} are ambiguous. Additionally enabledStagedUpgrades is a member of StagedSchemaUpgradePolicyFactory (toStored.ts:99), not of StagedSchemaUpgradePolicy, so {@link StagedSchemaUpgradePolicy.enabledStagedUpgrades} cannot resolve either. These three links are reported at schemaFactoryAlpha.ts:289, :693 and :698 — the declaration plus the two {@inheritdoc} members that re-emit the same comment (3 links × 3 declarations = the 9 CI errors). Required fix: follow the convention this file's own neighbours already use (toStored.ts:43 and :49-53) — {@link (StagedSchemaUpgradePolicy:interface)}, {@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}, and {@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades}. Apply the same correction to simple-tree/api/stagedRequiredUpgrades.ts:78, which has the identical ambiguous link and only escapes CI because it is not reachable from the public entrypoint rollup.

Minimum diff to ship: disambiguate the three {@link StagedSchemaUpgradePolicy…} targets in schemaFactoryAlpha.ts (and stagedRequiredUpgrades.ts:78) so api-extractor passes, and make the opted-in Optional→Required tightening actually succeed through TreeView.upgradeSchema() — or restate the documented step 3 to match what is really supported — covered by an end-to-end test through the real view path.

Non-blocking observations

• packages/dds/tree/src/simple-tree/core/toStored.ts:117-120 — this diff makes permissive.includeStagedRequired return false, but the (unchanged) TSDoc for that member at toStored.ts:82 still reads "Permissive policy — includes all staged schema upgrades." The inline code comment explains the reasoning well; the doc comment users actually see in IntelliSense is now stale. Consider rewording to note that staged-required narrowing is deliberately excluded and pointing at enabledStagedUpgrades.
• Scope: 20 changed files, and the PR description carries both the atomicity justification and the ordered review map required above the 10- and 20-file thresholds. The scope policy is met — noting this only so it is not mistaken for an unaddressed gap.
• The human review thread on packages/dds/tree/src/simple-tree/api/create.ts:58 (whether this validation belongs in isFieldInSchema) is still open and is left for the author and that reviewer; I have not touched it.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Build fix: Build - client packages (ADO build 417433)

Localization

1. Fault file(s)

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts (TSDoc on SchemaStaticsAlpha.stagedRequired, ~lines 264–266): the doc comment used three {@link StagedSchemaUpgradePolicy…} references that API Extractor could not resolve.
• packages/dds/tree/src/simple-tree/core/toStored.ts (not edited — the cause of the ambiguity): StagedSchemaUpgradePolicy is declared twice, as an interface (line 13) and as a const (line 106).
• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts (computeUpgradeSchema TSDoc, line 78): same latent ambiguity, not yet surfaced by CI because the symbol is not in an API-Extractor-checked entrypoint.

2. Root cause hypothesis

The failure is nine API Extractor ae-unresolved-link errors (not TypeScript compilation errors) emitted during npm run ci:build → build:api-reports, which failed the Build Stage Build job with Bash exited with code '255':

@fluidframework/tree: Error: src/simple-tree/api/schemaFactoryAlpha.ts:289:2 - (ae-unresolved-link)
  The @link reference could not be resolved: The reference is ambiguous because
  "StagedSchemaUpgradePolicy" has more than one declaration; you need to add a
  TSDoc member reference selector

Because StagedSchemaUpgradePolicy is both an interface and a const (the factory instance), a bare {@link StagedSchemaUpgradePolicy} is ambiguous and API Extractor requires a member reference selector. The single doc comment on stagedRequired (schemaFactoryAlpha.ts:289) is inherited via {@inheritdoc} by SchemaFactoryAlpha.stagedRequired (line 693) and its static counterpart (line 698), so one bad comment produced three reported sites × three bad links = nine errors. Both @fluidframework/tree and the fluid-framework aggregator reported the same errors. This was introduced by this PR, which added stagedRequired and its documentation; the pre-existing code in toStored.ts already uses the correct selector form ({@link (StagedSchemaUpgradePolicy:interface)}).

3. Proposed minimal repair

Rewrite only the three ambiguous links in the stagedRequired doc comment, using the exact forms that already resolve successfully elsewhere in the same package:

Before	After
{@link StagedSchemaUpgradePolicy}	{@link (StagedSchemaUpgradePolicy:interface)}
{@link StagedSchemaUpgradePolicy.includeStagedRequired}	{@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}
{@link StagedSchemaUpgradePolicy.enabledStagedUpgrades}	{@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades}

The third link is retargeted to the factory interface because enabledStagedUpgrades is a member of StagedSchemaUpgradePolicyFactory, not of the StagedSchemaUpgradePolicy interface. This is the narrowest possible repair: it touches comment text only, changes no runtime code, no type signature, and no .api.md (Fluid API reports do not carry @link text — verified: zero @link occurrences under packages/dds/tree/api-report/).

What changed

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:264-266 — disambiguated the three @link references above.
• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:78 — same disambiguation applied to the identical latent ambiguity added by this PR, so it cannot surface once that symbol is exported.

Commit: 38f053df. Diff is 2 files, +4/−3, comments only.

Validation

A full local pnpm install is not possible in this environment: the package registry is unreachable from this machine (corepack fetching pnpm-11.15.1.tgz fails with ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE against registry.npmjs.org), and the worktree has no node_modules. So API Extractor could not be executed locally. Instead the repair was verified by two offline checks plus direct evidence from the failing CI run itself:

1. TSDoc syntax parse check — all three replacement forms parse cleanly under @microsoft/tsdoc (the parser API Extractor uses):

   OK  {@link (StagedSchemaUpgradePolicy:interface)}
   OK  {@link (StagedSchemaUpgradePolicy:interface).includeStagedRequired}
   OK  {@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades}
   

2. Resolution proven by the failing build itself — both replacement targets are already used and already resolve in the same API Extractor run that produced these errors: toStored.ts:43 uses {@link (StagedSchemaUpgradePolicy:interface)} and toStored.ts:53 uses {@link StagedSchemaUpgradePolicyFactory.enabledStagedUpgrades}, and neither was reported as unresolved in build 417433. includeStagedRequired is a declared member of the StagedSchemaUpgradePolicy interface (toStored.ts:39).

3. No remaining ambiguity — grep '\{@link StagedSchemaUpgradePolicy[.} |]' over packages/dds/tree/src now returns zero matches.

The change is confined to comment text, so it cannot affect compilation, tests, or the generated API reports; the authoritative confirmation is the next CI run of Build - client packages.

Other checks

No other failing check on this PR is attributable to this diff. repo-policy-check, Policy checks, Website validation, Validate CODEOWNERS, and license/cla all passed on build 417433 / the corresponding GitHub Actions runs; Build - client packages was the only FAILURE, and its sole error class was the ae-unresolved-link set addressed above.

Scope

Scope: 20 changed files, one concern (the sf.stagedRequired staged optional-to-required migration API).

This PR is at the 20-file boundary and is deliberately atomic: the API surface (schemaFactoryAlpha.ts, stagedRequiredUpgrades.ts, toStored.ts, toStoredSchema.ts, configuration.ts, schematizingTreeView.ts, entrypoint/index re-exports) cannot be split from the schema-projection behavior it introduces without shipping an intermediate state in which a stagedRequired field is declarable but not correctly projected into stored schema — an unsafe, data-affecting half-state. The generated/mechanical portion of the diff is the api-report/*.api.md files and the index.ts / entrypoints/alpha.ts re-export lines; the tests (toStoredSchema.spec.ts, stagedSchemaUpgrade.spec.ts, testTrees.ts, and the call-site updates in existing specs) are required regression coverage and the mechanical fallout of toStoredSchema gaining a required options parameter. Read order: toStored.ts → toStoredSchema.ts → schemaFactoryAlpha.ts → stagedRequiredUpgrades.ts → tests.

Fixed by Minions (Dallas — Engineer · claude-opus-5)

[Noah Encke (noencke)]

VERDICT: REQUEST_CHANGES

Repository review skill: review invoked (Standard mode: Breaker + API Analyst sub-agents + Inspector), with comparison-base for base resolution.

Base: microsoft/FluidFramework:main at c29323d9ca · Head: 38f053dfd2 · 20 changed files (+841/−14).

Automated checks:

• Build - client packages (ADO 417520, authoritative CI at head): in progress — not yet green at the time of review. The prior build for this PR (417433) failed and 38f053dfd2 is the fix; the result is still unverified.
• repo-policy-check, pnpm lint, Policy checks, PR template placeholder content, CLA: pass.
• Local npm run build / mocha for packages/dds/tree: skipped — packages/dds/tree/node_modules is absent in this environment and there is no npm registry access, so no install/build was possible. Findings below are from source reading, and the key one is proven by the PR's own test assertions rather than by a run I performed.

Blocking issues:

• packages/dds/tree/src/shared-tree/schematizingTreeView.ts:274 — Step 3 of the documented migration is unreachable: TreeView.upgradeSchema() throws instead of tightening the field. computeUpgradeSchema correctly returns a stored schema with the field as Required once includeStagedRequired is enabled, but upgradeSchema() then gates on allowsRepoSuperset(defaultSchemaPolicy, storedSchema, newSchema), which returns true only iff newSchema ⊇ storedSchema (feature-libraries/modular-schema/comparison.ts:188). Optional → Required is a narrowing, so the guard fails and the call throws UsageError("Existing stored schema cannot be upgraded to the requested schema…"). Even if that check were passed, updateSchema(newSchema) at line 284 is called without allowNonSupersetSchema, so treeCheckout.ts:1442-1447 would fire assert 0xbe6. This is proven by the PR's own passing test at stagedSchemaUpgrade.spec.ts:705-710, which asserts canUpgrade: false for exactly this pair (stored Optional, computed Required) — canUpgrade is the same allowsRepoSuperset(policy, stored, wouldUpgradeTo) call. Yet schemaFactoryAlpha.ts:293-300 and .changeset/staged-required-field-migration.md:34-36 both instruct users to enable includeStagedRequired and call TreeView.upgradeSchema. As shipped, the only way to tighten the stored schema is TestSchemaRepository.apply() (test-only) — extractPersistedSchema explicitly only dumps a snapshot. Required fix: give upgradeSchema() a staged-required-aware path that permits exactly the enabled staged-required narrowings and calls this.checkout.updateSchema(newSchema, true), or otherwise make the documented opt-in actually apply; if the tightening is intentionally out of scope for this PR, remove step 3 from the TSDoc and changeset so the API does not document behavior it does not have.

• packages/dds/tree/src/test/simple-tree/api/stagedSchemaUpgrade.spec.ts:634-970 — The new suite never exercises the documented step-3 flow, which is why the defect above is invisible. Every view.upgradeSchema() / stagedUpgradePolicy usage in this file sits in the pre-existing staged and staged optional upgrade suites (all at lines ≤ 505); the new staged required upgrade suite contains none. It instead simulates the tightening with stored.apply(tightenedStoredSchema(schemaB)) against a TestSchemaRepository, annotated "This is deliberately not a superset change, so it does not go through tryUpdateRootFieldSchema" (line 969) — the workaround documents the bug rather than covering it. Required fix: add a test that builds a view with stagedUpgradePolicy: StagedSchemaUpgradePolicy.enabledStagedUpgrades(<the field's SchemaUpgrade>), calls view.upgradeSchema(), and asserts the stored root/field kind became Required and that a sf.required (version N+2) view can then open the document.

• packages/dds/tree/src/simple-tree/api/schemaFactoryAlpha.ts:295-298 — A field can be marked staged-optional and staged-required simultaneously, silently defeating the feature's whole purpose. The props type is Omit<FieldPropsAlpha<TCustomMetadata>, "defaultProvider" | "stagedRequiredUpgrade">, which removes only stagedRequiredUpgrade; stagedOptionalUpgrade remains assignable, and stagedRequired spreads ...props before setting its own marker (line 439-442). getStoredFieldKind (toStoredSchema.ts:473-483) checks isStagedOptional first and returns early, so the isStagedRequired marker is ignored and — under the default restrictive policy — the field projects to a Required stored field immediately, breaking exactly the version-N clients this API exists to protect. Required fix: omit "stagedOptionalUpgrade" as well from the props type on both stagedRequired and stagedRequiredRecursive (and symmetrically on stagedOptional), and/or reject the mutually-exclusive combination in createFieldSchema.

Minimum diff to ship: make TreeView.upgradeSchema() actually apply enabled staged-required tightenings (permit that specific narrowing and pass allowNonSupersetSchema to updateSchema), cover it with an end-to-end view.upgradeSchema() test, and make stagedOptionalUpgrade unassignable through stagedRequired's props.

Non-blocking observations

These are informational and do not require action before merge.

• packages/dds/tree/src/simple-tree/core/toStored.ts:82 — StagedSchemaUpgradePolicy.permissive sets includeStagedRequired: () => false. The inline comment explains why (a narrowing is never part of a maximally permissive schema), but the factory's own doc reads as "enable everything," so the exception is easy to miss. Worth stating on the permissive TSDoc that staged-required upgrades are never included and require enabledStagedUpgrades.
• packages/dds/tree/src/simple-tree/fieldSchema.ts:427 — FieldSchemaAlpha.isStagedRequired is the only way to obtain the SchemaUpgrade token that step 3 requires, but it is undocumented (it surfaces as // (undocumented) in both alpha API reports). Documenting it, and adding an example that threads it into StagedSchemaUpgradePolicy.enabledStagedUpgrades, would make the opt-in discoverable from IntelliSense alone.

Verified clean (no findings): the write-enforcement guards are complete across the construction/mutation paths I and the Breaker traced — objectToFlexContent, setField, applyFieldChange, createFromCursor/checkStagedRequiredFieldsPresent, unhydratedFlexTreeFromInsertable, TreeAlpha.importVerbose, plus importConcise, importCompressed, node constructors and TreeView.initialize; stored-key vs property-key handling in checkStagedRequiredFieldsPresent; the getStoredFieldKind rewrite is behavior-preserving for existing stagedOptional inputs; the getAppliedStagedRequiredUpgrades monotonicity scan (a staged view will not revert an already-tightened stored schema); every includeStagedRequired?.() call site uses optional invocation correctly. Scope: 20 files with the required atomicity justification and review map present in the description — policy met, not a finding. API surface: alpha-only, no API Council review required, minor bump and __section: tree correct, API reports consistent with source, no broken {@link} targets. The uncached computeUpgradeSchema fallback is bounded to view creation/schema change (compatibility is memoized in currentCompatibility), so it is not a meaningful performance regression. No security concerns: error messages carry schema identifiers and field keys only, never document values.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review feedback addressed — 616911f6df

All three blocking findings were reproduced against the branch before editing, and all three were valid. Details below.

1. CRITICAL — documented migration step 3 was unreachable ✅ fixed

Confirmed: schematizingTreeView.ts gated upgradeSchema() on allowsRepoSuperset(policy, stored, newSchema), but opting into a staged-required upgrade narrows Optional → Required, so the guard threw UsageError. The same defect existed one layer up in schemaCompatibilityTester.ts:49, so canUpgrade reported false exactly when the user had opted in — the reviewer's citation of the passing canUpgrade: false assertion was precisely the symptom.

Fix (packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts): computeUpgradeSchemas() now returns an UpgradeSchemaProjection with two stored schemas built from the same view schema and the same options except includeStagedRequired:

• wideningOnly — enables only the staged-required upgrades already applied in the stored schema.
• target — additionally enables the ones explicitly opted into.

upgradeSchema() now validates allowsRepoSuperset(stored, wideningOnly) and passes target to checkout.updateSchema(target, allowNonSupersetSchema) when target is not a superset. Because the two projections can differ only in staged-required field kinds, the "upgrades never narrow" guarantee is preserved for everything else, and the only permitted narrowing is provably the one the caller opted into. When nothing is opted in, target === wideningOnly and behavior is byte-identical to before (the pre-existing staged-optional rejection test still passes unchanged). checkSchemaCompatibility derives canUpgrade from wideningOnly and isEquivalent from target.

updateSchema(newSchema, allowNonSupersetSchema?: true) is pre-existing and already used by schematizeTree.ts:86, so assert 0xbe6 is no longer reachable on this path.

2. HIGH (tests) — no test drove view.upgradeSchema() ✅ fixed

Confirmed: the staged-required suite simulated the tightening via TestSchemaRepository.apply, with a comment conceding it is not a superset change — which is why the defect was invisible. Two tests added to stagedSchemaUpgrade.spec.ts:

• "applies the tightening through TreeView.upgradeSchema once explicitly opted in" — a four-tree end-to-end walk of the documented migration: phase N, phase N+1 (upgrade is a no-op), step 3 (opt in, call upgradeSchema(), assert the stored root field kind becomes Required), version-N clients correctly losing canView, step 4 equivalence, and a monotonicity check that a non-opted-in staged client does not revert the stored schema.
• "still rejects an upgrade which narrows for reasons other than a staged required opt in" — asserts canUpgrade === false and that upgradeSchema() still throws for an unrelated narrowing, pinning the guard that the fix relaxes.

The misleading comment on the old TestSchemaRepository.apply line now cross-references the new end-to-end test.

3. HIGH (API) — conflicting staged markers ✅ fixed

Confirmed: each factory omitted only its own marker, so sf.stagedRequired(T, { stagedOptionalUpgrade }) type-checked, and getStoredFieldKind gives staged-optional precedence — silently projecting a Required stored field immediately and breaking exactly the version-N clients the API exists to protect.

All six Omit<FieldPropsAlpha<…>, …> props types in schemaFactoryAlpha.ts now omit both markers, the two implementations explicitly clear the opposite marker at runtime, and getStoredFieldKind (toStoredSchema.ts) asserts a field is never both staged optional and staged required.

Docs

The stagedRequired TSDoc and the changeset now warn that step 3 is the one upgrade that narrows the stored schema, that TreeView.upgradeSchema does not scan the document, and that the application is responsible for ensuring no document still has the field empty.

Validation

Run against a shadowed dependency graph (this environment has no npm registry access — corepack/npm fail with ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE — so node_modules was reconstructed by symlinking a sibling checkout and rebuilding the 14 first-party packages in this branch's dependency chain from source):

Check	Result
tsc --project ./tsconfig.json (packages/dds/tree)	0 errors
tsc --project ./src/test/tsconfig.json	0 errors
mocha lib/test/simple-tree/api/stagedSchemaUpgrade.spec.js	24 passing, 0 failing
mocha lib/test/simple-tree/** + lib/test/shared-tree/**	4442 passing, 113 pending, 2 failing
biome check on all changed source files	clean
api-extractor run --local (packages/dds/tree)	regenerated tree.alpha.api.md

The 2 failures were proven pre-existing by stashing this branch's changes, rebuilding, and re-running the same two specs on the unmodified tree — identical failures. They are environment artifacts, not regressions: snapshotCompatibilityChecker.spec compares Windows \ against / inside an asserted error message, and treeNodeApi.spec "rejects operations beyond the post-edit length" fails because assert short code 0xd19 is not mapped back to its message when the repo's assert-tagging build step is bypassed.

sharedTree.spec.js was excluded from the broad run because it requires a newer @fluidframework/local-driver than the shadowed checkout provides.

API reports. packages/dds/tree/api-report/tree.alpha.api.md was regenerated by api-extractor and contains exactly the 12 expected Omit<…> lines. packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md could not be regenerated here — flub generate entrypoints fails in this shadowed graph with No export declarations found for "defineDataStore", so lib/alpha.d.ts is never produced. The 12 corresponding lines in that rollup were byte-identical to tree's pre-change lines, so the exact same substitution api-extractor emitted for tree was applied. Please confirm CI's API-report check agrees; if it reports drift, the CI-generated version is authoritative.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[github-actions]

🔗 Found some broken links! 💔

Run a link check locally to find them. See Checking for Broken Links for more information.

linkcheck output

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-website@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/
[ELIFECYCLE] Command failed with exit code 1.

[Noah Encke (noencke)]

VERDICT: APPROVE

Re-review of 616911f6df (previous review was 38f053dfd2). All three blocking findings from that review are genuinely fixed — I re-traced each mechanism against the live branch rather than trusting the fix summary.

Repository review skill: review invoked (Standard depth — Breaker + API Analyst tracks plus the Inspector pass), including its mandatory comparison-base sub-skill.

Base: microsoft/FluidFramework:main at c29323d9ca (single merge-base; 6 review commits / 19 target commits — below the divergence gate). Head: 616911f6df · 20 changed files, +1047/−34. PR reports MERGEABLE — no conflicts.

Automated checks:

• repo-policy-check, pnpm lint, Policy checks, SDL Sources Analysis, Validate CODEOWNERS, PR template placeholder content, Website Tests, CLA: pass.
• Build - client packages (ADO 417574): queued / in progress at review time — not yet green. This is the one check that compiles and tests @fluidframework/tree, so the type-level and test-level verification of this diff is still outstanding. Merge should wait for it.
• Local npm run build / mocha for packages/dds/tree: skipped — neither the repo root nor packages/dds/tree has node_modules in this environment and the npm registry is unreachable (npm ping hangs), so no install, build or test run was possible. Findings below come from source reading and cross-file tracing.

Blocking issues:

• None

Verification of the three previously-blocking findings:

1. upgradeSchema() could not perform the documented step 3 — fixed. schematizingTreeView.ts:270-289 now splits the projection via computeUpgradeSchemas into target (with opted-into tightenings) and wideningOnly (without). The UsageError guard is applied to wideningOnly, so the "an upgrade never narrows" rule is still enforced for everything except the opt-in, and the call is now this.checkout.updateSchema(target, isSuperset ? undefined : true). I confirmed against treeCheckout.ts:1440-1448 that the second parameter is allowNonSupersetSchema?: true and that passing true is exactly what suppresses assert 0xbe6, so the previously-unreachable path now completes. The bypass is sound because target and wideningOnly are built from identical options except includeStagedRequired (stagedRequiredUpgrades.ts:648-658), so they can differ only in staged-required field kinds.
2. No end-to-end coverage of step 3 — fixed. stagedSchemaUpgrade.spec.ts:744-803 ("applies the tightening through TreeView.upgradeSchema once explicitly opted in") is a real 4-client TestTreeProviderLite test walking phases N → N+1 (no-op without opt-in) → N+1 with StagedSchemaUpgradePolicy.enabledStagedUpgrades(requiredUpgrade) → N+2, asserting the stored root kind flips optional → required, that a version-N view loses canView, that a sf.required view is then isEquivalent, and — importantly — that a staged view which never opted in does not revert the tightening on a subsequent upgradeSchema(). stagedSchemaUpgrade.spec.ts:805-834 adds the negative case proving unrelated narrowings are still rejected. This is exactly the coverage that was missing.
3. A field could be marked staged-optional and staged-required at once — fixed. stagedOptionalUpgrade is now omitted from the props type of stagedRequired/stagedRequiredRecursive and stagedRequiredUpgrade from stagedOptional/stagedOptionalRecursive (schemaFactoryAlpha.ts:209-231, 322-421, reflected in both alpha API reports); each factory also explicitly writes the opposite marker as undefined after the ...props spread, closing the cast/as escape hatch; and getStoredFieldKind (toStoredSchema.ts:471-482) now asserts the two are mutually exclusive. Per CLAUDE.md, that new assert correctly uses a string literal rather than a hex code.

Also re-verified as behavior-preserving for existing consumers: with no staged-required field in play, wideningOnly is functionally identical to the old toUpgradeSchema(root, upgrades), so canUpgrade is unchanged; and the new isEquivalent conjunction (schemaCompatibilityTester.ts:305-310) reduces to the old canView && canUpgrade && stored ⊇ target because its third conjunct is implied by canUpgrade. The staged-optional rollout suites are untouched and still gate that path.

Minimum diff to ship: n/a

Non-blocking observations

These are informational and do not require action before merge.

• packages/dds/tree/src/simple-tree/api/stagedRequiredUpgrades.ts:641 — the cached fast path is effectively dead code. The guard is applied.size === 0 && base.includeStagedRequired === undefined, but all three built-in policies now define the member (toStored.ts:114 restrictive, :122 permissive, :130 enabledStagedUpgrades), and resolveStoredSchemaGenerationOptions(undefined) returns restrictive. So base.includeStagedRequired is essentially never undefined, and every checkSchemaCompatibility / upgradeSchema call falls into the branch that builds two fresh options objects and therefore misses the viewToStoredCache (keyed on options identity, toStoredSchema.ts:147). Net effect: each view creation and each stored-schema-change event performs two full uncached schema→stored projections where main performed one cached lookup. The blast radius is bounded — checkSchemaCompatibility is memoized per view in currentCompatibility and only recomputed in update() — so this is a constant-factor cost on viewWith, not a hot loop, which is why it is not blocking. A concrete fix: test whether the policy can ever return true rather than whether the member is present, e.g. base.includeStagedRequired === undefined || base === StagedSchemaUpgradePolicy.restrictive || base === StagedSchemaUpgradePolicy.permissive, or drop the now-redundant includeStagedRequired: () => false from restrictive and permissive and let the optional-member default carry it.
• packages/dds/tree/src/simple-tree/fieldSchema.ts:430 — FieldSchemaAlpha.isStagedRequired is still the only way to obtain the SchemaUpgrade token that step 3 needs, and it is still // (undocumented) in both alpha API reports. The new test at stagedSchemaUpgrade.spec.ts:647 shows the idiom (schemaB.isStagedRequired), but an IntelliSense-only user has no path to it. Adding a doc comment with an enabledStagedUpgrades example would make the documented opt-in discoverable.
• packages/dds/tree/src/simple-tree/api/create.ts:58 — the human review thread asking whether this validation belongs inside isFieldInSchema is still open and unanswered. For what it is worth, the code's own rationale looks correct to me: isFieldInSchema validates against the stored schema, where a staged required field is deliberately still Optional, so it has no information with which to reject the empty case. I am deliberately not resolving that thread — it is the human reviewer's to close.

Verified clean (no findings): the write-enforcement guards remain complete across objectToFlexContent, setField (which also covers root assignment via schematizingTreeView.ts:548), applyFieldChange, createFromCursor / checkStagedRequiredFieldsPresent, unhydratedFlexTreeFromInsertable, TreeAlpha.importVerbose, importCompressed, TreeBeta.clone and TreeView.initialize; staged-required is reachable only on object fields and roots (sf.map/sf.array take allowed types, not field schemas), so the object-node-only scan in checkStagedRequiredFieldsPresent is not a gap; stored-key vs property-key handling is correct; getAppliedStagedRequiredUpgrades monotonicity is sound and safe against an unrelated stored schema (absent fields resolve to the forbidden kind, never required); every includeStagedRequired?.() call site uses optional invocation. API surface: alpha-only; computeUpgradeSchema, computeUpgradeSchemas, UpgradeSchemaProjection, getAppliedStagedRequiredUpgrades and getStagedRequiredUpgrade are exported from the internal simple-tree barrel but not from src/index.ts, so they add no package surface; the two report deltas are confined to the intended additions; adding an optional member to StagedSchemaUpgradePolicy and to @sealed SimpleFieldSchema is non-breaking. Changeset is minor with __section: tree and its step-3 narrative now matches the shipped behavior. Scope: 20 files with the required atomicity justification and review map in the description — policy met, not a finding. No security concerns: error messages carry schema identifiers and field keys only, never document values.

Review by Minions (Ripley — Lead / Explorer · claude-opus-5)

[Noah Encke (noencke)]

Review pass on 616911f6df — no code change required

I re-checked the PR thread against the live branch head (616911f6df, working tree clean, 20 changed files). The only new item since the last fix commit is the automated broken links report. I verified it rather than acting on it, and it does not belong to this PR.

Finding: 🔗 "Found some broken links!" (github-actions[bot], 2026-08-14T20:20:38Z) — not caused by this PR

Verdict: invalid for this branch / pre-existing repo-wide failure. No change made.

Evidence:

1. This PR touches no website or docs content. All 20 changed files are under packages/dds/tree/**, packages/framework/fluid-framework/api-report/, and .changeset/. Nothing under docs/ and no Docusaurus config, sidebar, or markdown-link surface is modified.

2. The identical report is posted on unrelated open PRs. PR build(client): update eslint config to 14.0 #27973 — whose entire diff is common/build/eslint-config-fluid/CHANGELOG.md, pnpm-lock.yaml, pnpm-workspace.yaml — received a byte-identical comment at 2026-08-14T20:20:46Z, eight seconds after this one. Sweeping the 25 most recent open PRs, 13 of them currently carry the same sticky linkreport comment (build(client): update eslint config to 14.0 #27973, Advance oldest supported client lower bound to 2.0.0 #27972, Require an explicit oldest supported client #27971, Expose point-in-time APIs through new legacy beta entrypoint #27968, test: Update example packages to use playwright instead of jest + puppeteer #27964, Simplified summary process without summarizer nodes #27953, Add staged optional-to-required SharedTree field migration API (sf.stagedRequired) #27952, refactor(tree): Reserve positional argument in treeChanged event listener signature #27951, feat(container-runtime): Default document schema to declare createBlobPayloadPending support at 2.40.0+ #27948, build(eslint-config-fluid): consolidate custom rules into config package #27943, feat(tree): Promote alpha change event APIs to beta #27942, schema compat property keys #27938, feat(fluid-runner): support file and directory converter outputs #27937). A failure that reproduces across PRs with disjoint diffs is a main-side/infra failure, not a per-PR regression.

3. The report carries no actual broken link. The captured linkcheck output ends at:

   [SUCCESS] Serving "build" directory at: http://localhost:3000/
   [ELIFECYCLE] Command failed with exit code 1.
   

   The Docusaurus server starts and serves fine; npm run check-links exits non-zero without emitting a single offending URL. That is the shape of a harness/environment failure, not a link regression.

4. No CI run on this branch failed. Every workflow run on work/W-msqynx4d00al7d6c at head 616911f6df is completed/success, including Website validation (run 31836553699). Update link report (.github/workflows/linkcheck-reporter.yml) is a workflow_run reporter that downloads the website-validation-results artifact and re-posts it as a sticky comment — it reports the state of the site build, and does not gate this PR.

Fixing a repo-wide website link-check failure from a SharedTree schema-API PR would broaden the scope well past this PR's single concern, so I have deliberately left it alone. If it needs an owner, it should be a separate work item against the website/docs area.

Previously addressed feedback (unchanged, for context)

• .changeset/staged-required-field-migration.md — "the field is required" / read type should be optional → fixed in db18adef61, thread resolved.
• treeAlpha.ts:914 — error message wording → fixed in 0267a38ac8, thread resolved.
• create.ts:58 — "should this validation live in isFieldInSchema?" → I pushed back with rationale (isFieldInSchema only ever sees stored schema, which is deliberately Optional for a staged-required field, so staged-ness is not observable there). That thread is intentionally left active for Noah Encke (@noencke) to overrule; I have not resolved it unilaterally.

Validation

No source files were modified in this dispatch, so no build or test run was needed or performed. The branch is byte-identical to the previously validated head:

$ git log --oneline -1
616911f6df fix: address review feedback on github:microsoft/fluidframework#27952
$ git status --porcelain
?? .minions-worktree      # engine metadata, untracked, not part of the diff

The validation of record therefore remains the run reported for 616911f6df, plus the all-green GitHub Actions runs on that SHA listed above.

Scope: 20 changed files, one concern (staged optional-to-required field migration). Nothing was added by this dispatch.

Fixed by Minions (Ripley — Lead / Explorer · claude-opus-5)

[github-actions]

Bundle size comparison

Base commit: d352c8b829a1fdf1ce0a83627f98b857560b32c3
Head commit: 616911f6df5be58b85d14fb4fa077005f2cb231f

Notable changes

• 🔴 fluidFrameworkAllAlpha.js: parsed 784809 → 788813 (+4004), gzip 215077 → 216122 (+1045)
• 🔴 fluidFramework.js: parsed 403647 → 405756 (+2109), gzip 114397 → 114886 (+489)
• 🔴 sharedTree.js: parsed 393051 → 395153 (+2102), gzip 111840 → 112330 (+490)

Per-bundle deltas

@fluid-example/bundle-size-tests

• 🔴 fluidFrameworkAllAlpha.js: parsed 784809 → 788813 (+4004), gzip 215077 → 216122 (+1045)
• azureClient.js: parsed 624841 → 624897 (+56), gzip 166637 → 166680 (+43)
• odspClient.js: parsed 597129 → 597185 (+56), gzip 159777 → 159820 (+43)
• aqueduct.js: parsed 531229 → 531264 (+35), gzip 142107 → 142136 (+29)
• 🔴 fluidFramework.js: parsed 403647 → 405756 (+2109), gzip 114397 → 114886 (+489)
• 🔴 sharedTree.js: parsed 393051 → 395153 (+2102), gzip 111840 → 112330 (+490)
• containerRuntime.js: parsed 309137 → 309151 (+14), gzip 84559 → 84567 (+8)
• sharedString.js: parsed 176471 → 176478 (+7), gzip 49800 → 49808 (+8)
• experimentalSharedTree.js: parsed 160665 → 160665 (0), gzip 46265 → 46265 (0)
• matrix.js: parsed 160316 → 160323 (+7), gzip 45790 → 45797 (+7)
• loader.js: parsed 145704 → 145718 (+14), gzip 39286 → 39302 (+16)
• odspDriver.js: parsed 103906 → 103927 (+21), gzip 32404 → 32411 (+7)
• directory.js: parsed 67110 → 67117 (+7), gzip 18859 → 18866 (+7)
• 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
• map.js: parsed 47205 → 47212 (+7), gzip 14455 → 14462 (+7)
• odspPrefetchSnapshot.js: parsed 45635 → 45649 (+14), gzip 15242 → 15250 (+8)
• 252.js: parsed 44362 → 44362 (0), gzip 13735 → 13735 (0)
• summarizerDelayLoadedModule.js: parsed 30717 → 30717 (0), gzip 7716 → 7716 (0)
• socketModule.js: parsed 26469 → 26476 (+7), gzip 7896 → 7904 (+8)
• createNewModule.js: parsed 12454 → 12454 (0), gzip 4797 → 4797 (0)
• summaryModule.js: parsed 3789 → 3789 (0), gzip 1857 → 1857 (0)
• connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
• sharedTreeAttributes.js: parsed 847 → 854 (+7), gzip 499 → 508 (+9)
• debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
• FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

[Noah Encke (noencke)]

Review pass on 616911f6df — no code change required

I re-checked the PR thread against the live branch head. Local HEAD is 616911f6df5be58b85d14fb4fa077005f2cb231f, which matches headRefOid from the GitHub API, and the tracked diff is 20 changed files (+1047 / -34) — so the PR body's scope claim is still accurate and no description edit was needed.

Since the last fix commit there is exactly one new comment: the automated bundle size comparison. I verified it rather than acting on it.

Finding: 🔴 bundle size increases (github-actions[bot], 2026-08-14T20:46:14Z) — expected and proportionate; not a regression

Verdict: informational, no gate, no change made.

Evidence:

1. The 🔴 is a reporting marker, not a budget violation. .github/workflows/pr-bundle-size-comments.yml:463 sets const NOTABLE_THRESHOLD = 500;, and getRenderProps (:469-477) emits 🔴 for any parsed-size increase ≥ 500 bytes. There is no size budget, no threshold failure, and no core.setFailed on the delta anywhere in the workflow — it is a marocchino/sticky-pull-request-comment post only. The comment is a measurement, not a verdict.

2. The workflow is not a required check and did not fail. No bundle-size check run exists on this SHA. Every check run on 616911f6df is success, with only Build - benchmark-tool, Build - eslint-plugin-fluid and lint-docs neutral (skipped) — including Build - client packages and all of its Build/Coverage/AreTheTypesWrong/Jest/Tinylicious stages, plus repo-policy-check and Validate CODEOWNERS.

3. The size delta is proportionate to the shipped source added. Breaking the diff down by category:

   Category	Files	Added	Removed
   Shipped source (packages/dds/tree/src, non-test)	16	+514	−20
   Test (stagedSchemaUpgrade.spec.ts)	1	+448	−2
   Generated API reports	2	+34	−12
   Changeset	1	+51	0

   Only the first row can reach a bundle. A net +494 lines of shipped TypeScript producing +2102 parsed bytes on sharedTree.js is ≈ 4.3 bytes per net-added line — on the lean side, which is what you'd expect here given how much of that 514 is JSDoc and type-only declarations (UpgradeSchemaProjection, the isStagedRequired accessors, the optional includeStagedRequired policy member) that are fully erased at runtime.

4. The largest delta lands where the new API is actually exposed. fluidFrameworkAllAlpha.js (+4004 parsed / +1045 gzip) is the bundle that includes the alpha surface, and sf.stagedRequired / stagedRequiredRecursive / computeUpgradeSchemas are alpha APIs. sharedTree.js (+2102) and fluidFramework.js (+2109) carry the same implementation. The remaining bundles move by 0–56 bytes, i.e. incidental. Nothing shows a new module being pulled into a bundle that shouldn't have it — stagedRequiredUpgrades.ts is imported only from the simple-tree API layer that already ships in these bundles.

Adding a new alpha feature has a non-zero code size cost; +490 gzip on sharedTree.js for a full staged migration API is a reasonable price and there is no accidental bloat to remove.

Re-confirming the earlier link-check item with stronger evidence

The 🔗 "Found some broken links!" comment (2026-08-14T20:20:38Z) was previously rebutted as a repo-wide/infra failure rather than a regression from this PR. That rebuttal now has direct confirmation it didn't have at the time: the 🔗 Broken Link Check check run on 616911f6df is success, as is 🧪 Website Tests. The sticky comment is a workflow_run reporter re-posting an artifact and is stale relative to the actual check result on this head. Still no change made, and this PR touches nothing under docs/.

Still-open thread (unchanged, deliberately left active)

simple-tree/api/create.ts:58 — "Should we be doing this validation in isFieldInSchema instead of outside?" remains active for Noah Encke (@noencke). I independently re-verified the standing rebuttal rather than taking it on faith, and it holds architecturally:

• isFieldInSchema (feature-libraries/schemaChecker.ts) is typed (childNodes, schema: TreeFieldStoredSchema, schemaAndPolicy: SchemaAndPolicy, onError) => T | undefined. Its entire view of the field is the stored schema.
• Staged-required-ness is carried only on the view schema: getStagedRequiredUpgrade (simple-tree/fieldSchema.ts) narrows to FieldSchemaAlpha and reads isStagedRequired, i.e. propsAlpha?.stagedRequiredUpgrade ?? false.
• By design the stored projection of a staged-required field is Optional until the step-3 opt-in — that is the whole point of the feature. So an emptiness check inside isFieldInSchema is not merely awkward to factor, it is not expressible: the stored schema it receives legitimately permits the empty field, and making it expressible would require handing view-schema knowledge to the stored-schema validator, inverting the layering.

I have deliberately not resolved that thread — the reviewer should get to overrule this.

Validation

No source files were modified in this dispatch, so no build or test run was performed and none is claimed. The branch is unchanged from the previously validated head:

$ git rev-parse HEAD
616911f6df5be58b85d14fb4fa077005f2cb231f
$ git status --porcelain
?? .minions-worktree      # engine metadata, untracked, not part of the diff

The validation of record therefore remains the run reported for 616911f6df, plus the all-green check runs on that exact SHA cited above.

Scope: 20 changed files, one concern (staged optional-to-required field migration). Nothing was added by this dispatch.

Fixed by Minions (Rebecca — Architect · claude-opus-5)