Skip to content

feat(odsp-driver): support ProgID on create container requests#27642

Draft
WillieHabi wants to merge 1 commit into
microsoft:mainfrom
WillieHabi:wh/progid-page
Draft

feat(odsp-driver): support ProgID on create container requests#27642
WillieHabi wants to merge 1 commit into
microsoft:mainfrom
WillieHabi:wh/progid-page

Conversation

@WillieHabi

@WillieHabi WillieHabi commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Adds create-time ProgID propagation to the ODSP driver so hosts can provide routing metadata when creating ODSP-backed Fluid files. The optional `progId` argument is carried from `createOdspCreateContainerRequest` through the create-container flow and appended as a URL-encoded query parameter on the ODSP snapshot create-file API call.

This supports AB#76947 for `.page` files that need to share an extension while routing to different experiences.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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

Based on the diff (189 lines, 9 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

@WillieHabi WillieHabi requested a review from jason-ha July 7, 2026 16:52
@WillieHabi WillieHabi assigned jatgarg and unassigned jatgarg Jul 7, 2026
@WillieHabi WillieHabi requested a review from jatgarg July 7, 2026 16:52
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🔭 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: ❌ Request Changes

0 Exterminate, 1 Squash, 1 Investigate

Findings

Sev # Area File What Fix
🦟 Squash H1 API Compatibility packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts:29 The public @beta @legacy function createOdspCreateContainerRequest now has 7 positional parameters (4 required, 3 optional), with progId appended as yet another trailing optional argument rather than folding the growing set of optional/extensible options into a named-argument object. The PR's own changeset usage example demonstrates the resulting ergonomics problem: callers who only want to set progId must pass undefined, undefined, progId, which is exactly the pattern the Fluid API conventions call out to avoid ('Named arguments (object literals) for extensible APIs... use when many parameters, optional arguments, or future extensibility'). Any future addition will make this worse and is highly error-prone (positional optional args are easy to mis-order or miscount). Introduce an overload (or a new preferred entry point) that accepts an options object for the optional parameters (e.g. { createShareLinkType?, containerPackageInfo?, progId? }), and mark the current positional-only overload as @deprecated with migration guidance, rather than continuing to append positional optional parameters to the existing signature.
🐜 Investigate M1 Testing packages/drivers/odsp-driver/src/createOdspCreateContainerRequest.ts:81-89 Every new test that passes progId always passes containerPackageInfo as undefined (createNewUtilsTests.spec.ts:301, odspCreateContainer.spec.ts:122, odspDriverResolverTest.spec.ts:44). No test exercises progId combined with a defined containerPackageInfo (and/or createShareLinkType). The URL is built by concatenating containerPackageParam, shareLinkRequestParams (each conditionally prefixed with &), and progIdParam (unconditionally prefixed with & when defined). A future edit to this ampersand-joining logic (e.g. reordering params or changing a prefix) could silently produce a malformed query string (missing &, duplicate &&, or a dropped param) when all three optional parameters are supplied together, and none of the current tests would catch it. Add a test to createOdspCreateContainerRequest tests that calls it with all of createShareLinkType, containerPackageInfo, and progId set simultaneously, then parses request.url with URLSearchParams and asserts that containerPackageName, the share-link params (e.g. createLinkScope/createLinkRole), and progId are all present and correctly valued (not just substring-included), catching malformed concatenation.

View workflow run

AB#76947

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

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/

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

Crawling...

Stats:
  290753 links
    1929 destination URLs
    2179 URLs ignored
       0 warnings
       0 errors


@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Bundle size comparison

Base commit: 32bbb06cf7f7a59075624cc48c89c81b7687d890
Head commit: 8966ecd3b561897c0374c26247309599f76d47e3

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

  • azureClient.js: parsed 619098 → 619154 (+56), gzip 164824 → 164871 (+47)
  • odspClient.js: parsed 591824 → 591911 (+87), gzip 158916 → 158971 (+55)
  • aqueduct.js: parsed 525587 → 525622 (+35), gzip 140709 → 140739 (+30)
  • fluidFramework.js: parsed 393001 → 392766 (-235), gzip 111618 → 111539 (-79)
  • sharedTree.js: parsed 382388 → 382146 (-242), gzip 109007 → 108920 (-87)
  • containerRuntime.js: parsed 303937 → 303951 (+14), gzip 83213 → 83222 (+9)
  • sharedString.js: parsed 175984 → 175991 (+7), gzip 49445 → 49453 (+8)
  • experimentalSharedTree.js: parsed 160798 → 160798 (0), gzip 45804 → 45804 (0)
  • matrix.js: parsed 159845 → 159852 (+7), gzip 45411 → 45418 (+7)
  • loader.js: parsed 145256 → 145270 (+14), gzip 39062 → 39076 (+14)
  • odspDriver.js: parsed 104329 → 104381 (+52), gzip 32649 → 32671 (+22)
  • directory.js: parsed 66616 → 66623 (+7), gzip 18532 → 18540 (+8)
  • 748.js: parsed 58793 → 58793 (0), gzip 17826 → 17827 (+1)
  • map.js: parsed 46709 → 46716 (+7), gzip 14310 → 14317 (+7)
  • odspPrefetchSnapshot.js: parsed 45642 → 45656 (+14), gzip 15276 → 15285 (+9)
  • 594.js: parsed 44493 → 44493 (0), gzip 13744 → 13744 (0)
  • summarizerDelayLoadedModule.js: parsed 30749 → 30749 (0), gzip 7753 → 7753 (0)
  • socketModule.js: parsed 26477 → 26484 (+7), gzip 7885 → 7894 (+9)
  • createNewModule.js: parsed 12480 → 12574 (+94), gzip 4786 → 4825 (+39)
  • summaryModule.js: parsed 3797 → 3797 (0), gzip 1860 → 1860 (0)
  • connectionState.js: parsed 724 → 724 (0), gzip 429 → 429 (0)
  • sharedTreeAttributes.js: parsed 666 → 673 (+7), gzip 432 → 441 (+9)
  • debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
  • FluidFramework-HashFallback.js: parsed 422 → 422 (0), gzip 316 → 316 (0)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants