feat(remote-control): cache rewritten tunnel responses with ETag validation - #3718
Conversation
🦋 Changeset detectedLatest commit: 31de021 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a698ec582b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Cache rewritten Remote Control tunnel responses with ETag validation. |
There was a problem hiding this comment.
Describe the user-visible caching behavior
This entry exposes implementation details such as “rewritten tunnel responses” and “ETag validation” rather than telling users what changed. Rewrite it in observable terms, for example that unchanged Remote Control assets are reused across page loads.
AGENTS.md reference: AGENTS.md:L86-L88
Useful? React with 👍 / 👎.
| const etag = rewrittenResponseETag(body); | ||
| headers.push('Cache-Control', 'no-cache', 'ETag', etag); | ||
| if (requestMatchesETag(parsed.headers, etag)) { | ||
| headers.push('Content-Length', '0'); |
There was a problem hiding this comment.
Omit the zero Content-Length from 304 responses
When a matching revalidation occurs, this advertises a zero-length representation even though a Content-Length on a 304 must equal the length that the corresponding 200 response would have sent. Caches can use 304 fields to update the stored response metadata, leaving a nonempty cached asset with Content-Length: 0; omit this header unless the selected representation's actual length is supplied.
Useful? React with 👍 / 👎.
| const etag = rewrittenResponseETag(body); | ||
| headers.push('Cache-Control', 'no-cache', 'ETag', etag); |
There was a problem hiding this comment.
Make gzip validators weak or encoding-specific
When rewritten HTML, JS, or CSS is large enough for gzip negotiation, this strong ETag is computed from the identity body and then returned unchanged for both gzip and identity responses. Those representations are not byte-identical, so clients and intermediaries cannot safely use the tag for strong cross-variant operations such as range or cache combination; mark the shared validator weak or derive separate tags from the final encoded bodies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27148a23e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (requestMatchesETag(parsed.headers, etag)) { | ||
| return Buffer.from(`HTTP/1.1 304 Not Modified\r\n${headerLines(headers)}\r\n\r\n`); |
There was a problem hiding this comment.
Restrict synthesized 304s to successful GET or HEAD requests
This converts any rewritten response with a matching body hash into 304 Not Modified without checking the request method or upstream status. For example, a POST carrying an ETag obtained from a prior GET can execute its local side effect and then have its matching HTML response replaced with a 304; similarly, an upstream 4xx/5xx HTML response can be hidden behind a cached response. Only synthesize a 304 for an eligible successful GET/HEAD response and preserve the upstream result otherwise.
Useful? React with 👍 / 👎.
| for (const token of value.split(',')) { | ||
| if (candidates.includes(token.trim())) return true; |
There was a problem hiding this comment.
Honor wildcard If-None-Match validators
When a GET or HEAD for an existing rewritten asset uses If-None-Match: *, this matcher never succeeds because it only compares literal generated tags. The tunnel advertises its own validator and therefore needs to handle the wildcard itself; otherwise local handlers that do not implement conditional requests return a full 200 response and body instead of the required 304.
Useful? React with 👍 / 👎.
…onor wildcard If-None-Match
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31de021cea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const headers = filterResponseHeaders(response.rawHeaders, rewritten); | ||
| if (rewritten) headers.push('Cache-Control', 'no-cache'); | ||
| if (rewritten) { | ||
| const etag = rewrittenResponseETag(body); |
There was a problem hiding this comment.
Compute HEAD ETags from the GET representation
For a HEAD request, Node's HTTP client exposes no response body, so this hashes an empty JS/CSS body (or only the injected HTML fragment) rather than the representation returned by GET. Consequently, changed assets keep the same HEAD validator and a later conditional HEAD can incorrectly receive 304 Not Modified; the generated validator also disagrees with the GET validator. Either limit this synthesized validation path to GET or derive HEAD validators from the corresponding full representation.
Useful? React with 👍 / 👎.
Related Issue
Remote Control Web UI 静态资源每次打开都通过隧道全量重拉(chunk 拆分调研的附带发现,单独立项修复)。
Problem
隧道转发时对被 URL 重写的响应(HTML/JS/CSS)强制
Cache-Control: no-cache且不生成 ETag:本地服务对内容被改写的资源给出的 ETag 已不匹配改写后的内容,被直接剥掉。浏览器因此无法使用协商缓存,每次打开 Remote Control 都要经隧道重新传输全部 JS/CSS bundle——在 feat-187 gzip 压缩后仍要付一次完整的上行传输和 30s 超时风险。What changed
在隧道 client(
packages/remote-control)为被重写的响应生成内容寻址 ETag 并支持协商缓存:sha256弱 ETag(W/"…",gzip 与 identity 变体共用)返回给浏览器(替代不再匹配的源站 ETag);保留Cache-Control: no-cache(语义为每次 revalidate,内容变更即刻生效,不会缓存过期内容)。If-None-Match(含*通配)与改写后内容的 ETag 匹配则直接回304 Not Modified,不再传输 body——静态内容未变时第二次打开 Remote Control 的资源传输接近零(304 帧仅几百字节)。Last-Modified,避免基于改写前文件 mtime 的验证器造成缓存不一致。Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.