Skip to content

Fix/response clone#141

Closed
godronus wants to merge 15 commits into
betafrom
fix/response-clone
Closed

Fix/response clone#141
godronus wants to merge 15 commits into
betafrom
fix/response-clone

Conversation

@godronus

@godronus godronus commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

godronus and others added 14 commits June 5, 2026 08:26
Bump the StarlingMonkey submodule from 0.3.0 (9dda8ba) to the tip of
godronus/gcore/integration (84f5d52), which carries two pending upstream
patches on top of 0.3.0:

  - feat(fetch): implement Response.clone() (PR #312)
  - fix(fetch): body.blob() sets Blob.type from Content-Type header (issue #311)

Add context/PATCHES.md documenting the applied patches, their upstream
PR/issue links, and the rebase procedure for future StarlingMonkey bumps.
Update CONTEXT_INDEX.md and CLAUDE.md to make the file discoverable.
…on tests

Implement Response.clone() in the StarlingMonkey runtime and add blob.type
propagation from Content-Type headers. Both fixes are applied via the
godronus/gcore/integration fork branch (SHA 84f5d52) pending upstream review
of PR #312 and issue #311.

Expose Response.clone() in types/globals.d.ts (instance method, previously
commented out). Update KNOWN_LIMITATIONS.md to reflect the implementation.

Restructure integration-tests/test-application from a single flat JS file
into a typed, extensible architecture:
- Convert to TypeScript with Hono routing
- Split into handlers/ (WASM context) and checks/ (Node.js, auto-discovered)
- routes.ts as single source of truth for route paths and test names
- Build artefacts consolidated under dist/ (one .gitignore entry)
- Add temporary Response.clone() prod-invocation guard — remove when
  PR #312 merges upstream and submodule is rebased

Add pnpm test:app:build and test:app:check scripts for local development.
Add context/PATCHES.md documenting the integration branch, rebase procedure,
and test guard removal checklist.
…k runner

Add tsconfig.json for the test application so fastedge-build resolves
SDK types directly from ../../types rather than the missing installed
package. Pass --tsconfig in build-test-app.js and create-test-app.js.

Fix run-test-app-checks.js: remove --bundle=false (incompatible with
--external), switch to --bundle so relative imports (routes.ts) are
inlined into each check file, and fix ESLint errors (regex u flag,
await-in-loop, no-plusplus, catch param naming, dynamic import comment).
…ation tests

Add four test scenarios covering the full clone surface:
- Constructed response text() on both branches (basic correctness)
- Constructed response getReader() + mutation guard: drain original, fill(0)
  all chunks, verify clone reads independently
- Incoming fetch() text() on both branches (HttpIncomingBody materialisation)
- Incoming fetch() getReader() + mutation guard: the precise original bug
  scenario — host-backed body, both branches via getReader()

Tests 2 and 4 currently fail: the tee implementation enqueues the same
Uint8Array reference to both branches rather than cloning each chunk, so
fill(0) on the original's chunks corrupts the clone's queued data. This is
a separate bug from the materialisation fix in PR #312 and blocks release.

Update KNOWN_LIMITATIONS.md and CONTEXT_INDEX.md to document the tee chunk
independence bug and mark Response.clone() as partially implemented.
Comment thread .github/scripts/prod-invocation/create-test-app.js Outdated
Comment thread scripts/run-test-app-checks.js Outdated

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.

Pull request overview

This PR updates the FastEdge SDK’s Fetch API type surface to include Response.clone(), and refactors/extends the production-invocation test application so runtime regressions (notably around Response.clone() behavior) can be validated against a live deployed app.

Changes:

  • Added Response.clone(): Response to the public types/globals.d.ts declarations.
  • Reworked the prod-invocation test app into a TypeScript + Hono structure with auto-discovered Node-based checks and new local runner scripts.
  • Updated CI workflow/scripts and repo context docs to reflect the new prod-invocation test harness and StarlingMonkey patch tracking.

Reviewed changes

Copilot reviewed 31 out of 35 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
types/globals.d.ts Declares Response.clone() in the public Fetch API type surface.
scripts/run-test-app-checks.js Local script to compile and run all prod-invocation check modules against a deployed URL.
scripts/build-test-app.js Local script to build the prod-invocation TypeScript app into a WASM binary via fastedge-build.
runtime/fastedge/build.sh Adjusts StarlingMonkey runtime build parallelism behavior.
pnpm-lock.yaml Locks new dependency set including hono.
package.json Adds hono devDependency and new test:app:* scripts.
integration-tests/test-application/types.ts Adds shared interfaces for handler/check modules.
integration-tests/test-application/tsconfig.json Adds TS config for the prod-invocation test app + typechecking against SDK types.
integration-tests/test-application/test-app.ts New Hono-based entrypoint that registers handler modules.
integration-tests/test-application/test-app.js Removes the previous flat JS test app implementation.
integration-tests/test-application/routes.ts Centralizes route paths and human-readable test names.
integration-tests/test-application/README.md Documents the prod-invocation test app structure, usage, and temporary guard policy.
integration-tests/test-application/handlers/secret.ts Handler to validate secret injection.
integration-tests/test-application/handlers/response-clone.ts Temporary guard handler exercising multiple Response.clone() scenarios.
integration-tests/test-application/handlers/outbound-fetch.ts Handler to validate outbound fetch behavior.
integration-tests/test-application/handlers/multi-chunk-source.ts Helper route serving multi-chunk streaming data for clone/tee validation.
integration-tests/test-application/handlers/env.ts Handler to validate env var access (BUILD_SHA).
integration-tests/test-application/handlers/echo.ts Handler to echo request method/headers/body.
integration-tests/test-application/checks/secret.ts Node check for the secret handler.
integration-tests/test-application/checks/response-clone.ts Node check validating the Response.clone guard response assertions.
integration-tests/test-application/checks/outbound-fetch.ts Node check for outbound fetch behavior.
integration-tests/test-application/checks/env.ts Node check verifying BUILD_SHA matches the expected SHA.
integration-tests/test-application/checks/echo.ts Node check validating request echo behavior.
integration-tests/test-application/.gitkeep Keeps the directory present in git.
context/PATCHES.md Documents applied StarlingMonkey patches and rebase/retirement procedure.
context/KNOWN_LIMITATIONS.md Adds/updates confirmed runtime limitations, including Response.clone() status.
context/development/TESTING_GUIDE.md Updates testing guide with prod-invocation layer and commands.
context/CONTEXT_INDEX.md Adds references and decision-tree pointers for new context docs and workflows.
context/CHANGELOG.md Records the Response.clone()/blob.type work and new test harness infrastructure.
CLAUDE.md Adds pointer to PATCHES.md for StarlingMonkey submodule bump workflow.
.gitignore Updates ignore rules to ignore the new prod-invocation dist directory.
.github/workflows/prod-invocation.yaml Updates workflow paths/env vars to use dist outputs and new test URL.
.github/scripts/prod-invocation/invoke-test-app.js Invokes auto-discovered compiled check modules instead of hardcoded checks.
.github/scripts/prod-invocation/create-test-app.js Builds TS test app to WASM and compiles check modules to dist for Node import.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

.github/scripts/prod-invocation/invoke-test-app.js:44

  • In a catch (error) block, error is unknown and not guaranteed to have a .message property. As written, a non-Error throw would cause a secondary exception while reporting, masking the real failure.
    } catch (error) {
      core.warning(`Attempt ${attempt} failed: ${error.message}`);
      if (attempt === MAX_RETRIES) {
        throw new Error(
          `Test application invocation failed after ${MAX_RETRIES} attempts: ${error.message}`,
        );

Comment thread integration-tests/test-application/test-app.ts
Comment thread integration-tests/test-application/test-app.ts
Comment thread integration-tests/test-application/handlers/secret.ts
Comment thread integration-tests/test-application/handlers/outbound-fetch.ts
Comment thread integration-tests/test-application/handlers/env.ts
Comment thread integration-tests/test-application/checks/echo.ts
Comment thread .github/scripts/prod-invocation/create-test-app.js
Comment thread scripts/run-test-app-checks.js Outdated
Comment thread scripts/run-test-app-checks.js Outdated
Comment thread runtime/fastedge/build.sh
@godronus godronus closed this Jun 9, 2026
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.

3 participants