Skip to content

fix(cli): forward function requests without cloning (CLI-2415) - #6580

Open
7ttp wants to merge 3 commits into
developfrom
7ttp/cli-2415-cli-request-cloning-can-stall-an-early-edge-function
Open

fix(cli): forward function requests without cloning (CLI-2415)#6580
7ttp wants to merge 3 commits into
developfrom
7ttp/cli-2415-cli-request-cloning-can-stall-an-early-edge-function

Conversation

@7ttp

@7ttp 7ttp commented Sep 11, 2026

Copy link
Copy Markdown
Member

TL;DR

Prevents early Edge Function responses from stalling locally.

Cloning tees the request body, leaving an unused stream branch that can block early responses,
which we've now fixed by forwarding the body without cloning and covering the failure with a runtime regression test.

ref:

@7ttp 7ttp self-assigned this Sep 11, 2026
@7ttp
7ttp requested a review from a team as a code owner September 11, 2026 18:17

@github-actions github-actions 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.

🤖 AI Review

The request-forwarding fix is correct, and no runtime correctness or security defect was found. Of Claude’s four findings, two quality concerns are confirmed, one test-reliability concern remains uncertain without reproducing the pre-fix behavior, and the naming concern is refuted. Codex reported no findings.

Findings

Severity Location Category Sources Claim
🟡 MINOR apps/cli/src/shared/functions/serve-main-offline.e2e.test.ts:500 test-reliability claude The regression test may not reliably exercise the pre-fix body-stream stall, and timeout failures lack container diagnostics.
🟡 MINOR packages/stack/src/functions/serve.main.ts:198 test-coverage claude The stack-owned request-forwarding fix has no regression test covering an early response while its request body remains unread.
⚪ NIT apps/cli/src/shared/functions/serve.main.ts:291 maintainability claude Neither request-forwarding implementation documents the non-obvious requirement not to clone the request, allowing the body-tee stall to be reintroduced.
Refuted findings (kept for transparency, not posted as review comments)
  • apps/cli/src/shared/functions/serve.main.ts:288 (naming): The clonedURL identifier is stale because the function no longer clones anything.
    Refuted: The identifier still accurately describes the distinct URL object created from the request URL at line 288; only the Request clone was removed.

Stats

Claude findings: 4 · Codex findings: 0 · Confirmed: 2 · Refuted: 1 · Uncertain: 1


Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread apps/cli/src/shared/functions/serve-main-offline.e2e.test.ts Outdated
Comment thread packages/stack/src/functions/serve.main.ts
Comment thread apps/cli/src/shared/functions/serve.main.ts
@7ttp
7ttp force-pushed the 7ttp/cli-2415-cli-request-cloning-can-stall-an-early-edge-function branch from 87bff55 to 74c0892 Compare September 11, 2026 19:34
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.

CLI request cloning can stall an early Edge Function response

2 participants