Enable noUncheckedIndexedAccess for odsp-driver

[shlevari]

How contribute to this repo.

Guidelines for Pull Requests.

Description

Enables TypeScript's noUncheckedIndexedAccess option for @fluidframework/odsp-driver production and test builds, resolving AB#34163.

Production indexed accesses now use explicit undefined checks and throw when required values are missing. The PR introduces no production non-null assertion operators. Test-only non-null assertions remain where a failed presence assumption naturally fails the test.

Compact snapshot validator functions accept possibly undefined indexed values and reject them through the existing incorrectServerResponse validation path. Functions that require valid nodes retain narrow contracts, with callers checking before invocation.

Reviewer Guidance

The review process is outlined in the pull request guidelines.

Please focus on the explicit failure paths for missing array and record entries and on the compact snapshot validator behavior.

[github-actions]

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

Based on the diff (421 lines, 28 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

[github-actions]

🔭 PR Review Fleet Report

Note

This report is generated by an experimental AI review fleet and is provided as a beta feature. Findings are a starting point for discussion, not a gate. Use your own judgement.

Verdict: ⚠️ Approve with Suggestions

0 Disastrous, 1 Dangerous, 2 Disagreeable

Findings

Sev	#	Area	File	What	Fix
🐊 Dangerous	H1	Testing	packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts:266	combineProtocolAndAppSnapshotTree() now throws an explicit Error("Snapshot tree is missing the .app tree") when snapshotTree.trees['.app'] is undefined, replacing the old implicit-crash behavior. There is no test at all for combineProtocolAndAppSnapshotTree/getSnapshotTree (grep of the test directory finds zero references), so neither the happy path (app+protocol trees combined correctly) nor this new explicit failure path is exercised. If a future regression in readTree/snapshot fetching stops populating .app, this would silently start throwing a different, less specific error than intended (or worse, if the guard is accidentally removed later, nothing would catch it) without any test noticing.	Add a unit test that constructs an ISnapshotTree whose trees map lacks a .app entry and asserts that combineProtocolAndAppSnapshotTree (or getSnapshotTree through a minimal concrete subclass) throws with the '.app tree' message. Also add a positive test that combines app+protocol trees and asserts the resulting blobs/trees/id match expectations.
🐍 Disagreeable	M1	Testing	packages/drivers/odsp-driver/src/odspSnapshotParser.ts:33	buildHierarchy() now throws Snapshot tree entry '<path>' has no parent node when lookup[entryPathDir] is undefined (e.g., a flat-tree entry appears before its parent directory entry, or references a directory that was never emitted). Existing tests in jsonSnapshotFormatTests.spec.ts only exercise well-ordered, well-formed flat trees, so this new validation path for malformed/out-of-order ODSP snapshot data is untested — a server-side ordering bug would previously produce a confusing generic TypeError and now produces this message, but nothing verifies the message or that it fires at all.	Add a test that calls convertOdspSnapshotToSnapshotTreeAndBlobs (or buildHierarchy if exported) with a flat tree entry whose path's parent directory was never added to lookup (e.g. entry "a/b" with no prior "a" tree entry), and assert it throws with a message matching /has no parent node/.
🐍 Disagreeable	M2	Testing	packages/drivers/odsp-driver/src/compactSnapshotParser.ts:303	parseCompactSnapshotResponse() now explicitly throws when records.snapshot or records.blobs is undefined (lines 303 and 307), guarding against a compact snapshot binary that's missing the snapshot or blobs record. snapshotFormatTests.spec.ts only parses well-formed compact snapshots produced by convertToCompactSnapshot, so there is no test constructing/parsing a malformed compact snapshot missing one of these top-level records — a corrupted-response scenario (bad service payload) that this code is specifically defending against goes unverified.	Add a test that builds a root NodeCore with only some of the required top-level records (e.g. omit the 'blobs' record while keeping 'snapshot', 'ops', 'sequenceNumber') and calls parseCompactSnapshotResponse on it, asserting it throws 'Compact snapshot is missing the blobs record' (and a companion test for the missing-snapshot-record case).

---

View workflow run

[Copilot]

🟡 Changes recommended

Address the compact snapshot error classification and string telemetry issues before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Enables noUncheckedIndexedAccess for the ODSP driver’s production and test builds, adding explicit handling for potentially missing indexed values.

Changes:

• Adds bounds and presence checks across snapshot, delta, URL, summary, and buffer processing.
• Updates compact snapshot parsing and serialization.
• Adjusts tests and removes obsolete lint suppression.

File summaries

File	Reviewed changes
packages/drivers/odsp-driver/tsconfig.json	Enables strict unchecked indexed-access checking.
packages/drivers/odsp-driver/src/zipItDataRepresentationUtils.ts	Adds node and buffer validation. Nit (1 vote): missing string inputs report BlobCore instead of String.
packages/drivers/odsp-driver/src/WriteBufferUtils.ts	Narrows boolean code mapping.
packages/drivers/odsp-driver/src/test/zipItDataRepresentationTests.spec.ts	Tests missing node handling.
packages/drivers/odsp-driver/src/test/tsconfig.json	Inherits strict indexed-access checking.
packages/drivers/odsp-driver/src/test/socketTests/socketMock.ts	Guards protocol version access.
packages/drivers/odsp-driver/src/test/socketTests/deltaConnectionUpdateTests.spec.ts	Guards metadata fields.
packages/drivers/odsp-driver/src/test/prefetchSnapshotTests.spec.ts	Guards snapshot tree access.
packages/drivers/odsp-driver/src/test/opsCaching.spec.ts	Guards cached operation access.
packages/drivers/odsp-driver/src/test/odspPointInTimeDocumentService.spec.ts	Guards stream batches and calls.
packages/drivers/odsp-driver/src/test/getUrlAndHeadersWithAuth.spec.ts	Guards authorization header access.
packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts	Guards required stub calls.
packages/drivers/odsp-driver/src/test/deltaStorageService.spec.ts	Guards message access.
packages/drivers/odsp-driver/src/test/createNewUtilsTests.spec.ts	Guards blob IDs.
packages/drivers/odsp-driver/src/ReadBufferUtils.ts	Adds buffer bounds validation.
packages/drivers/odsp-driver/src/odspUrlHelper.ts	Validates URL capture groups.
packages/drivers/odsp-driver/src/odspSummaryUploadManager.ts	Guards summary entries.
packages/drivers/odsp-driver/src/odspSnapshotParser.ts	Validates snapshot roots and operations.
packages/drivers/odsp-driver/src/odspDriverUrlResolver.ts	Validates URL decomposition.
packages/drivers/odsp-driver/src/odspDocumentStorageServiceBase.ts	Guards versions and snapshot trees.
packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts	Guards message boundaries.
packages/drivers/odsp-driver/src/odspDeltaStorageService.ts	Guards delta response entries.
packages/drivers/odsp-driver/src/localOdspDriver/localOdspDeltaStorageService.ts	Guards cached messages.
packages/drivers/odsp-driver/src/fetchSnapshot.ts	Guards snapshot operations.
packages/drivers/odsp-driver/src/createFile/createNewUtils.ts	Guards summary entries.
packages/drivers/odsp-driver/src/compactSnapshotWriter.ts	Guards operation indexing.
packages/drivers/odsp-driver/src/compactSnapshotParser.ts	Moderate (3 votes): missing records.snapshot uses a generic Error instead of the typed incorrectServerResponse path; also affects line 306.
packages/drivers/odsp-driver/eslint.config.mts	Removes the obsolete unchecked-access rule.

Review details

Suppressed comments (2)

packages/drivers/odsp-driver/src/compactSnapshotParser.ts:308

• The missing records.blobs case has the same error-classification problem: a malformed compact response becomes a plain Error rather than the existing incorrectServerResponse validation error. Assert the narrowed value with assertNodeCoreInstance before passing it to readBlobSection, matching the other compact snapshot fields.

	if (blobsNode === undefined) {
		throw new Error("Compact snapshot is missing the blobs record");
	}

packages/drivers/odsp-driver/src/zipItDataRepresentationUtils.ts:620

• The new undefined-input branch reports expectedNodeType: "BlobCore", although this helper validates a string and NodeType has a distinct "String" value. Missing string fields will therefore emit misleading parser telemetry; use "String" here and in the existing fallback below so both invalid-input paths describe the helper's actual contract.

export function getStringInstance(node: NodeTypes | undefined, message: string): string {
	if (node === undefined) {
		throwBufferParseException(node, "BlobCore", message);
	}

• Files reviewed: 28/28 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Bundle size comparison

Base commit: 1d6e255160539d52adad83e4ff645863dce34d0e
Head commit: 453ee7ae081dfe308c000c100c8eb0ad171b00d2

Notable changes

• 🔴 odspClient.js: parsed 606600 → 608216 (+1616), gzip 163052 → 163579 (+527)
• 🔴 odspDriver.js: parsed 106695 → 108183 (+1488), gzip 33227 → 33685 (+458)
• 🔴 odspPrefetchSnapshot.js: parsed 46463 → 47462 (+999), gzip 15512 → 15830 (+318)

Per-bundle deltas

@fluid-example/bundle-size-tests

• fluidFrameworkAllAlpha.js: parsed 805447 → 805503 (+56), gzip 221237 → 221315 (+78)
• azureClient.js: parsed 634336 → 634331 (-5), gzip 169989 → 170073 (+84)
• 🔴 odspClient.js: parsed 606600 → 608216 (+1616), gzip 163052 → 163579 (+527)
• aqueduct.js: parsed 538211 → 538222 (+11), gzip 144462 → 144508 (+46)
• fluidFramework.js: parsed 415378 → 415411 (+33), gzip 117810 → 117845 (+35)
• sharedTree.js: parsed 404757 → 404783 (+26), gzip 115252 → 115271 (+19)
• containerRuntime.js: parsed 315027 → 315009 (-18), gzip 86438 → 86437 (-1)
• sharedString.js: parsed 175191 → 175198 (+7), gzip 49637 → 49644 (+7)
• experimentalSharedTree.js: parsed 161846 → 161846 (0), gzip 46722 → 46722 (0)
• matrix.js: parsed 153720 → 153727 (+7), gzip 44381 → 44388 (+7)
• loader.js: parsed 147328 → 147344 (+16), gzip 40038 → 40049 (+11)
• 🔴 odspDriver.js: parsed 106695 → 108183 (+1488), gzip 33227 → 33685 (+458)
• directory.js: parsed 65669 → 65676 (+7), gzip 18493 → 18502 (+9)
• 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
• 🔴 odspPrefetchSnapshot.js: parsed 46463 → 47462 (+999), gzip 15512 → 15830 (+318)
• map.js: parsed 45820 → 45827 (+7), gzip 14120 → 14127 (+7)
• 252.js: parsed 44384 → 44384 (0), gzip 13741 → 13741 (0)
• summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
• socketModule.js: parsed 27108 → 27200 (+92), gzip 8069 → 8149 (+80)
• createNewModule.js: parsed 12464 → 12679 (+215), gzip 4792 → 4849 (+57)
• summaryModule.js: parsed 3888 → 3958 (+70), gzip 1874 → 1899 (+25)
• connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
• sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 496 → 506 (+10)
• debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
• FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)