Skip to content

feat: write app deployment manifest to CDN#8015

Merged
n1ru4l merged 5 commits into
mainfrom
feat-app-deployment-manifest
May 6, 2026
Merged

feat: write app deployment manifest to CDN#8015
n1ru4l merged 5 commits into
mainfrom
feat-app-deployment-manifest

Conversation

@n1ru4l

@n1ru4l n1ru4l commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Background

By knowing the app deployment name and version it is currently impossible to retrieve a list of all documents that belong to the version via the CDN.

Description

Write a app deployment manifest to the CDN that contains all the document hashes and whether the app deployment is enabled or disabled.

The endpoint /apps/:appname/:appVersion returns a JSON manifest with the following information:

{
  id: "some uuid",
  isActive: false,
  appName: "app-name",
  appVersion: "app-version",
  // list of all hashes this app version contains
  documentHashes: ["aaa", "bbb", "ccc", "ddd"],
}

This enabled routers to pre-load documents of app deployments and the router MCP plugin to load all available tools for an app deployment version.

Out of Scope

This pull request does not introduce backfilling app deployment manifests for existing published app deployments.

  • We can introduce this later on before we start adjusting gateway/router to leverage this endpoint
  • For now this manifest is only needed in the context of the MCP gateway plugin, where this trade-off is acceptable

Checklist

  • Input validation
  • Output encoding
  • Access control
  • Testing

@github-actions

github-actions Bot commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-hive/laboratory 0.1.7-alpha-20260506084217-324127779ee5404ae9ec5e1804f8c062f92275bc npm ↗︎ unpkg ↗︎
@graphql-hive/render-laboratory 0.1.7-alpha-20260506084217-324127779ee5404ae9ec5e1804f8c062f92275bc npm ↗︎ unpkg ↗︎
hive 11.1.0-alpha-20260506084217-324127779ee5404ae9ec5e1804f8c062f92275bc npm ↗︎ unpkg ↗︎

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 27, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces the generation and storage of app deployment manifests in S3 during activation and retirement, along with a new CDN worker endpoint to serve these manifests. It includes a new integration test to verify the end-to-end flow. A debug console.log statement in the readAppDeploymentManifest method should be removed.

Comment thread packages/services/cdn-worker/src/artifact-storage-reader.ts Outdated
@github-actions

github-actions Bot commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🐋 This PR was built and pushed to the following Docker images:

Targets: build

Platforms: linux/amd64

Image Tag: 324127779ee5404ae9ec5e1804f8c062f92275bc

n1ru4l and others added 2 commits April 27, 2026 12:06
@n1ru4l
changeset
308eb36
@n1ru4l @gemini-code-assist
Update packages/services/cdn-worker/src/artifact-storage-reader.ts
d1b9d4c 
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@n1ru4l n1ru4l requested review from dotansimha and jdolle April 28, 2026 09:10
@n1ru4l n1ru4l marked this pull request as ready for review April 28, 2026 09:12
@n1ru4l n1ru4l requested a review from kamilkisiela April 28, 2026 09:34
n1ru4l
n1ru4l commented Apr 28, 2026
View reviewed changes
Comment thread packages/services/cdn-worker/src/artifact-handler.ts
Comment on lines -464 to -466
// We're using here a public location, because we expose the Location to the end user and
// the public S3 endpoint may differ from the internal S3 endpoint. E.g. within a docker network.
// If they are the same, private and public locations will be the same.

@n1ru4l n1ru4l Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no idea what this comment means 😅

dotansimha
dotansimha approved these changes Apr 28, 2026
View reviewed changes

@dotansimha dotansimha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, LGTM!

One question: this will only be available for app deployment that will be activated after deploying this change, right?
Do you see a value in back-filling manifests or it's not needed?

@n1ru4l

n1ru4l commented Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@dotansimha see the description: 😅

image

I think we can be fine with shipping this now and then re-evaluate based on actual usage (all current app deployment versions could be already retired by then) whether we need to backfill once we update router/gateway to use this new manifest endpoint.

jdolle
jdolle reviewed Apr 28, 2026
View reviewed changes
Comment thread integration-tests/tests/api/app-deployments.spec.ts Outdated
expect(activateAppDeployment.error).toBeNull();

const cdnAccess = await createCdnAccess();
const persistedOperationUrl = `${cdnAccess.cdnUrl}/apps/app-name/app-version`;

@jdolle jdolle Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you fetch before activating?

jdolle
jdolle approved these changes Apr 28, 2026
View reviewed changes

@jdolle jdolle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd like to see a test covering what happens before activating the app deployment but otherwise looks great

@n1ru4l

n1ru4l commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

@jdolle The manifest does not exist before that. BUt it makes sense to add a test for it!

@n1ru4l n1ru4l merged commit 80b7600 into main May 6, 2026
22 of 23 checks passed
@n1ru4l n1ru4l deleted the feat-app-deployment-manifest branch May 6, 2026 14:52
@theguild-bot theguild-bot mentioned this pull request May 6, 2026
Merged
This was referenced Jun 4, 2026
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdolle jdolle jdolle approved these changes
@dotansimha dotansimha dotansimha approved these changes
@kamilkisiela kamilkisiela Awaiting requested review from kamilkisiela
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@n1ru4l @jdolle @dotansimha