Skip to content

Check runtime implementations against private interfaces - #28228

Open
Alex Villarreal (alexvy86) wants to merge 3 commits into
microsoft:mainfrom
alexvy86:work-item-analysis
Open

Alex Villarreal (alexvy86) wants to merge 3 commits into
microsoft:mainfrom
alexvy86:work-item-analysis

Conversation

@alexvy86

@alexvy86 Alex Villarreal (alexvy86) commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

[Agent-generated]

Description

Add compile-time assertions for internal runtime contracts that cannot be declared directly in exported classes' implements clauses. This catches implementation drift without exposing private members or changing runtime behavior.

  • Check ContainerRuntime's root-parent contract, its typed context wrapper, and FluidDataStoreContext's private parent-context contract.
  • Check FluidDataStoreRuntime's actual configuration member types against Required. Keep this assertion in the source module because declaration emit erases private member types.
  • Reuse the existing requireAssignableTo helper and update comments to make the checks discoverable.

Addresses AB#48376. The bounded audit of other production casts found no additional equivalent gaps requiring changes.

Reviewer Guidance

The review process is outlined in the pull request guidelines.

The container's signal adaptation remains intentional. Internal configuration properties remain optional across versions, but the current implementation must provide them all.

Validated ESM/CJS compilation, ESLint, formatting, package policy, dependency consistency, and documentation builds. Mocha: datastore 47 passing; container-runtime 1,066 passing and 6 pending. Eight in-memory mutation probes verified compile-time failures. Emitted datastore JavaScript and declarations are unchanged apart from comments/source maps. No changeset is included because there is no consumer-facing change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added area: tools area: runtime Runtime related issues area: repo Repo related work area: website base: main PRs targeted against main branch labels Sep 15, 2026
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

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

Based on the diff (85 lines, 5 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

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Fleet Review — Clean

No issues found across the reviewer fleet for this run.

View run

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review issues were identified.

Pull request overview

Adds compile-time assertions for private runtime contracts without changing runtime behavior or public APIs.

Changes:

  • Validates context assignability and runtime configuration types.
  • Documents the assertion locations.
File summaries
File Reviewed change
packages/runtime/datastore/src/dataStoreRuntime.ts Validates internal configuration types.
packages/runtime/container-runtime/src/test/types/internalInterfaces.ts Adds context compatibility assertions.
packages/runtime/container-runtime/src/channelCollection.ts Documents assertion locations.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the area: build Build related issues label Sep 15, 2026
@alexvy86
Alex Villarreal (alexvy86) marked this pull request as ready for review September 15, 2026 16:49
@github-actions

Copy link
Copy Markdown
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output

$ start-server-and-test "npm run serve -- --host 127.0.0.1 --no-open" http://127.0.0.1:3000 check-links
1: starting server using command "npm run serve -- --host 127.0.0.1 --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 --host 127.0.0.1 --no-open

[SUCCESS] Serving "build" directory at: http://127.0.0.1:3000/

> fluid-framework-website@0.0.0 check-links
> linkcheck http://127.0.0.1:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  658102 links
    3579 destination URLs
    3898 URLs ignored
       0 warnings
       0 errors


@github-actions

Copy link
Copy Markdown
Contributor

Bundle size comparison

Base commit: abcf10205e9500f21f63faf1ac3a1097e70ab376
Head commit: 8e779a8d103e462775a36e8a3a70720fcf5dacf8

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

  • fluidFrameworkAllAlpha.js: parsed 805447 → 805503 (+56), gzip 221236 → 221314 (+78)
  • azureClient.js: parsed 634336 → 634331 (-5), gzip 169987 → 170074 (+87)
  • odspClient.js: parsed 606600 → 606711 (+111), gzip 163051 → 163198 (+147)
  • aqueduct.js: parsed 538211 → 538222 (+11), gzip 144462 → 144511 (+49)
  • fluidFramework.js: parsed 415378 → 415411 (+33), gzip 117809 → 117844 (+35)
  • sharedTree.js: parsed 404757 → 404783 (+26), gzip 115252 → 115270 (+18)
  • containerRuntime.js: parsed 315027 → 315009 (-18), gzip 86438 → 86439 (+1)
  • sharedString.js: parsed 175191 → 175198 (+7), gzip 49636 → 49643 (+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 → 106753 (+58), gzip 33226 → 33294 (+68)
  • directory.js: parsed 65669 → 65676 (+7), gzip 18493 → 18502 (+9)
  • 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
  • odspPrefetchSnapshot.js: parsed 46463 → 46444 (-19), gzip 15511 → 15522 (+11)
  • map.js: parsed 45820 → 45827 (+7), gzip 14119 → 14127 (+8)
  • 252.js: parsed 44384 → 44384 (0), gzip 13741 → 13741 (0)
  • summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
  • socketModule.js: parsed 27108 → 27078 (-30), gzip 8067 → 8104 (+37)
  • createNewModule.js: parsed 12464 → 12464 (0), gzip 4792 → 4805 (+13)
  • summaryModule.js: parsed 3888 → 3888 (0), gzip 1874 → 1874 (0)
  • connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
  • sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 496 → 506 (+10)
  • debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
  • FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review comments remain, and the supplied validation indicates the changes are safe.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: build Build related issues area: repo Repo related work area: runtime Runtime related issues area: tools area: website base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants