Deliver accepted artifacts without redundant asset scans or compiler files - #273
Merged
Merged
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes release/deployment integrity verification logic and failure behavior in a security- and operations-sensitive path that warrants final human review.
Pull request overview
This PR refactors release artifact delivery verification to avoid depending on any local compiler/build tree, ensuring delivery validates only published, immutable remote assets and fails fast on deterministic errors. It shifts the “compiled vs approved WASM interface” comparison into release package preparation, where the compiled bytes exist and the approved remote bytes are hash-authenticated.
Changes:
- Replaces delivery-time “read local compiler WASM + retry readiness loop” with strict, timeout-bounded remote object GET/HEAD verification against a prepared (authenticated) release descriptor.
- Moves WASM interface comparison into
prepare-release-package.tsby fetching the approved remote WASM and comparing it to the locally compiled WASM before building/packaging. - Adds an isolated regression test that runs remote artifact verification with an empty repository root and asserts request counts plus immediate failure behavior.
File summaries
| File | Description |
|---|---|
| tools/playsrc/tests/release-remote-artifact.test.ts | Adds an isolated regression test ensuring delivery verification works without a local games/wasm tree and fails immediately on deterministic errors. |
| tools/playsrc/src/prepare-release-package.ts | Moves the compiled-vs-approved WASM interface check into package preparation by reading local compiled bytes and fetching approved remote bytes. |
| tools/playsrc/src/deploy.ts | Refactors remote object verification to consume a prepared release descriptor, remove readiness retries/compiler-tree reads, and enforce per-request timeouts. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+123
to
+125
| if (response.status !== 200) throw new DeploymentError(`remote object ${descriptor.sha256} metadata response differs (HTTP ${response.status})`) | ||
| if (response.headers.get("content-length") !== descriptor.byteLength || response.headers.get("etag") === null | ||
| || response.headers.get("access-control-allow-origin") !== TF2_APPLICATION_ORIGIN) throw new DeploymentError(`remote object ${descriptor.sha256} metadata differs`) |
Hona
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Failure and correction
Release33459511801 accepted the exact package, then attempted an obsolete compiler-tree read inside a redundant remote asset scan. Its broad catch retried ENOENT for ten minutes.
Regression
A bounded isolated test executes real delivery control flow with an empty compiler tree and synthetic static files, mocking only acceptance/production boundaries. It requires receipt and all three package checks, permits only the three application readiness requests, rejects any asset-origin scan, and proves missing local files fail before production. It performs no production operation and produces no browser receipt. Focused delivery/package/interface tests: 8 pass, 36 assertions.
CAS descriptors are unchanged. No upload, 2650-object rescan or solver/gameplay campaign is repeated. After merge, only the newly source-bound artifact and its required headed startup receipt are prepared.