feat: Image Upload and Test Hardening#294
Merged
Merged
Conversation
…kflows Adds 1x1.png fixture and updates theme-extensions tests to ensure clean ArrayBuffer usage.
…sHandler - DownloadAssetsInputSchema gets three optional fields (all backward-compatible): - fileMode (default 0o600): Unix permission bits for written files - tempDir (default outputDir): override atomic-write temp directory - assetsSubdir (default 'assets'): override assets subdirectory name - atomicRename() helper handles EXDEV cross-device fallback (copyFile+unlink) - assetsSubdir is guarded via path.basename() to strip traversal attempts - project-ext.ts: downloadAssets() now accepts an optional opts object - 3 new tests cover each option path; all 11 unit tests pass
| const tempFilename = `.tmp-${crypto.randomBytes(8).toString('hex')}-${filename}`; | ||
| const tempFullPath = path.join(resolvedTempDir, tempFilename); | ||
|
|
||
| await fs.writeFile(tempFullPath, Buffer.from(buffer), { flag: 'wx', mode: fileMode }); |
- domhandler: switch from value import to 'import type { AnyNode }' so tsc
can resolve the type without domhandler in package.json (it's a transitive
dep of cheerio, but not guaranteed to be hoisted in CI)
- CodeQL: temp filenames now contain only crypto.randomBytes — network-derived
filenames are never embedded in the temp path. The sanitized filename only
appears in the final atomicRename destination, which tsc/CodeQL can verify
is safe (sanitizeFilename strips everything non-alphanumeric)
- test: update writeFile assertion to check '.tmp-' instead of 'badname'
since temp paths are now fully random; rename-dest assertion unchanged
tsc resolves 'import type' the same as value imports — if the package isn't in package.json, the typecheck fails even in CI environments that don't hoist transitive deps. domhandler is pinned to 5.0.3 to match the version used by cheerio@1.0.0-rc.12.
| const tempScreenshotFilename = `.tmp-screen-${crypto.randomBytes(8).toString('hex')}`; | ||
| const tempScreenshotPath = path.join(resolvedTempDir, tempScreenshotFilename); | ||
|
|
||
| await fs.writeFile(tempScreenshotPath, Buffer.from(screenshotBuffer), { flag: 'wx', mode: fileMode }); |
…T from SDK surface These methods don't belong in the SDK: - inferTheme: CSS parser using cheerio heuristics, not an API operation - syncTheme: thin composition of existing public API methods - themePrompt: pure string utility with no SDK coupling - createSessionREST: exploratory spike that leaked into the API surface All four return untyped Promise<any> and create opinionated behavior that belongs in consumer code, not a thin API client. uploadImage and downloadAssets remain — they genuinely cannot be generated (private REST endpoints, complex filesystem orchestration).
- Remove inferThemeTool, themePromptTool, syncThemeTool from proxy virtual tools, listTools handler, and index.ts exports - Update proxy tests to only reference downloadAssetsTool - Rewrite PR.md to document final API surface
3dc1ff2 to
7617179
Compare
…ial .screens fallback - stitch-ext.ts: remove createProjectREST (untested spike on public surface) - test/fixtures/real-image.png: delete orphaned 706KB fixture - download-handler.ts: remove || .screens fallback on design system response - download.test.ts: align mock to use designSystems key (matches real API)
Adds DownloadAssetsInput, DownloadAssetsResult, DownloadedScreenTrace, and DownloadAssetsErrorCode to the package exports, matching the existing upload type exports.
TDD: 4 new tests covering collision dedup, empty/undefined fallback, and special character stripping. Replaces inline regex in download-handler with a shared slugify() function that tracks seen slugs and appends a numeric suffix on collision. Prevents silent file overwrites when screens share titles.
…dler TDD: 2 new tests for screenshot download failure and design system export failure warning collection. TSC: Extends DownloadAssetsResult with optional warnings?: string[] field. Silent catch blocks now push descriptive warning messages instead of swallowing errors.
TDD: 1 new test proving peak concurrency drops from 10 to ≤5. Replaces unbounded Promise.all with a runWithConcurrency() utility that uses task factories and Promise.race to maintain a bounded concurrency pool. Prevents CDN rate limiting on asset-heavy screens.
TSC: adds DownloadAssetsOutput type to spec/download.ts and exports it.
TDD: 2 new tests proving warnings flow through Project.downloadAssets().
Project.downloadAssets() now returns { screens, warnings } instead of
a bare DownloadedScreenTrace[]. warnings is always present (empty array
when no issues occurred).
Replaces this['client'] bracket notation in uploadImage with the same (this as any).client pattern used by downloadAssets. Documents the access violation consistently pending the Issue 3.3 codegen fix.
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.
This PR adds two handwritten
Projectextensions for operations the codegen pipeline cannot express, a virtual tool layer for the MCP proxy, and test/workflow cleanup.Usage
Image Upload
Asset Download
With options:
API Surface
Project.uploadImage(filePath, opts?): Promise<Screen[]>Uploads an image file to the project via the private
BatchCreateScreensREST endpoint and returnsScreenobjects. Supports PNG, JPEG, and WebP.filePathstringopts.titlestring?opts.createScreenInstancesboolean?true)Errors are thrown as
StitchErrorwith codesNOT_FOUND,AUTH_FAILED, orUNKNOWN_ERROR.Project.downloadAssets(outputDir, opts?): Promise<DownloadedScreenTrace[]>Downloads all screens and referenced assets to a local directory. Rewrites URLs in HTML to produce self-contained output. Uses atomic temp-file renames, hash-based filenames, and path-traversal guards.
outputDirstringopts.fileModenumber?0o600)opts.tempDirstring?outputDir)opts.assetsSubdirstring?"assets")Returns
DownloadedScreenTrace[]:Errors are thrown as
StitchErrorwith codesNOT_FOUND,NETWORK_ERROR, orVALIDATION_ERROR.Virtual Tools (MCP Proxy)
The SDK exposes
download_assetsas a virtual tool throughStitchProxy, allowing MCP-connected agents to download project assets without direct SDK access.Changes
Handwritten Extensions (
src/project-ext.ts)Project.uploadImage— REST upload of image assets (PNG, JPEG, WebP) viaBatchCreateScreens, returning domainScreenobjects.Project.downloadAssets— Downloads all screens and assets, rewriting URLs to be self-contained. Returns a trace array for CLI integration.Typed Service Contracts (
src/spec/)upload.ts— Zod-validated input schema,UploadImageResultdiscriminated union,UploadImageSpecinterface.download.ts— Zod-validated input schema with defaults,DownloadedScreenTracetype,DownloadAssetsSpecinterface.Handlers
upload-handler.ts— File reading, base64 encoding, MIME validation, multipart upload orchestration.download-handler.ts— Atomic temp-file writes, hash-based asset filenames (truncated to 100 chars), Cheerio-based URL rewriting, EXDEV fallback, path-traversal detection.MCP Proxy
proxy/virtual-tools.ts—download_assetsvirtual tool definition for agent-side asset downloading.proxy/handlers/callTool.ts— Routes virtual tool calls to SDK methods, forwards all others to Stitch.proxy/handlers/listTools.ts— Merges virtual tools into the remote tool list.Test Suite
upload.test.ts— 16 tests covering happy path, unsupported formats, missing files, auth failures, and edge cases.download.test.ts— 15 tests covering HTML rewriting, asset hashing, design system export, path traversal, and trace results.proxy.test.ts— 3 unit tests + 10 integration tests for virtual tool routing and forwarding.ArrayBufferconstruction in mocks (no shared pool leaks), zeroas anyassertions.Workflow Cleanup
fleet-analyze.yml,fleet-dispatch.yml,fleet-label.yml,fleet-merge.yml, andjules-merge-conflicts.yml. Onlyci.ymlremains.