feat: add safe interactive GitHub Pages demo - #33
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds a zero-network browser terminal demo, static Pages artifact generation and validation, browser and unit tests, Pages deployment, and updated CI action pins. ChangesPages demo
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PagesBuild
participant PagesArtifact
participant GitHubPages
participant LiveVerifier
GitHubActions->>PagesBuild: Build demo artifact
PagesBuild->>PagesArtifact: Generate and validate _site
PagesBuild->>GitHubPages: Upload Pages artifact
GitHubPages-->>GitHubActions: Deployment URL
LiveVerifier->>GitHubPages: Fetch deployed page
GitHubPages-->>LiveVerifier: Page and revision content
LiveVerifier->>GitHubPages: Fetch deployed JavaScript bundle
GitHubPages-->>LiveVerifier: Bundle content
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.github.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
.github/workflows/pages.yml (1)
36-37: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftBootstrap npm from a reviewed, integrity-locked source.
Line 37 installs the deployment package manager outside a committed lockfile. This makes the Pages build depend on a registry-resolved toolchain that is not fully reviewed with the repository dependencies.
Use a committed bootstrap lockfile or a verified npm artifact. Apply the same bootstrap pattern to the other workflows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pages.yml around lines 36 - 37, Replace the unpinned registry installation in the “Install the repository npm version” workflow step with a committed, integrity-locked bootstrap lockfile or verified npm artifact. Apply the same locked bootstrap approach to npm setup steps in the other workflows, preserving the required npm version.Source: Linters/SAST tools
scripts/check-pages.js (1)
25-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe symlink assertion is unreachable.
lstatdoes not follow symlinks. For a symlink,fileStats.isFile()returnsfalse, so line 28 throws first with the messagemust be a regular file. Line 29 can never fail. A symlink is still rejected, so the artifact stays safe, but the error message misidentifies the cause. Order the checks so the symlink case reports itself.♻️ Proposed change
const fileStats = await lstat(filePath); - assert(fileStats.isFile(), `${filename} must be a regular file`); assert(!fileStats.isSymbolicLink(), `${filename} must not be a symlink`); + assert(fileStats.isFile(), `${filename} must be a regular file`); totalBytes += (await stat(filePath)).size;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-pages.js` around lines 25 - 31, Reorder the assertions in the actualFiles loop so fileStats.isSymbolicLink() is checked before fileStats.isFile(). Preserve both validations and ensure symlinks report “must not be a symlink” while other non-regular entries report “must be a regular file.”scripts/build-pages.js (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
replaceAllfor the build placeholder.
replacewith a string pattern substitutes only the first occurrence. Ifdemo/index.htmlever contains__LIT_SHELL_BUILD__twice, the artifact keeps an unresolved placeholder andscripts/check-pages.jsfails the build at line 55.replaceAllremoves that failure mode and keeps the missing-placeholder guard intact.♻️ Proposed change
-const renderedHtml = sourceHtml.replace('__LIT_SHELL_BUILD__', buildRevision); +const renderedHtml = sourceHtml.replaceAll( + '__LIT_SHELL_BUILD__', + buildRevision, +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-pages.js` around lines 19 - 22, Update the replacement in the rendered HTML generation flow to use replaceAll for every __LIT_SHELL_BUILD__ occurrence, while preserving the existing renderedHtml === sourceHtml guard for detecting a missing placeholder.tests/browser/pages-demo.spec.ts (1)
11-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard
afterAllagainst a failedbeforeAll.If
startPagesDemoFixturerejects, for example when_siteis missing,fixturestaysundefined.afterAllthen throwsTypeError: Cannot read properties of undefined, and that error replaces the actionable message from the fixture in the report.♻️ Proposed change
test.afterAll(async () => { - await fixture.close(); + await fixture?.close(); });Declare the variable accordingly:
-let fixture: PagesDemoFixture; +let fixture: PagesDemoFixture | undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/browser/pages-demo.spec.ts` around lines 11 - 17, Update the fixture variable used by the beforeAll and afterAll hooks to allow an unset value, and guard fixture.close() in afterAll so cleanup runs only when startPagesDemoFixture successfully assigned a fixture. Preserve the original fixture-startup error when initialization fails.demo/demo-websocket.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExport
DEMO_SHELL/DEMO_CWDinstead of duplicating the literals in demo/app.ts. The shell path and working directory are defined once in demo/demo-websocket.ts but re-typed as separate string literals in demo/app.ts. A future edit to one side without the other would silently break session validation (mismatchedshell/cwdbetween the client'sspawn()call and the server-side constants).
demo/demo-websocket.ts#L12-13: exportDEMO_SHELLandDEMO_CWDalongsideDEMO_WEBSOCKET_URL.demo/app.ts#L36-37: import and use the exportedDEMO_SHELL/DEMO_CWDinstead of the literals'/bin/lit-shell-demo'and'/demo'.demo/app.ts#L55-56: use the same imported constants in thespawn()options instead of re-typing the literals.🔧 Proposed fix
-const SESSION_ID = 'browser-demo-session'; -const DEMO_SHELL = '/bin/lit-shell-demo'; -const DEMO_CWD = '/demo'; +const SESSION_ID = 'browser-demo-session'; +export const DEMO_SHELL = '/bin/lit-shell-demo'; +export const DEMO_CWD = '/demo';-import { DEMO_WEBSOCKET_URL, installDemoWebSocket } from './demo-websocket.js'; +import { + DEMO_CWD, + DEMO_SHELL, + DEMO_WEBSOCKET_URL, + installDemoWebSocket, +} from './demo-websocket.js'; ... - terminal.shell = '/bin/lit-shell-demo'; - terminal.cwd = '/demo'; + terminal.shell = DEMO_SHELL; + terminal.cwd = DEMO_CWD; ... await terminal.spawn({ - shell: '/bin/lit-shell-demo', - cwd: '/demo', + shell: DEMO_SHELL, + cwd: DEMO_CWD, allowJoin: false, enableHistory: false, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@demo/demo-websocket.ts` at line 1, Export DEMO_SHELL and DEMO_CWD from the constants section alongside DEMO_WEBSOCKET_URL in demo-websocket.ts, then import and reuse them throughout app.ts for session validation and spawn() options instead of duplicating the shell path and working-directory literals.demo/index.html (1)
5-8: 🔒 Security & Privacy | 🔵 TrivialNo clickjacking protection; likely unfixable within GitHub Pages constraints.
The CSP has no
frame-ancestorsdirective, so the page can be embedded in a hostile iframe. Note thatframe-ancestorsis not enforceable when delivered via a<meta>tag per the CSP spec, andX-Frame-Optionscannot be set as an HTTP header on GitHub Pages without a fronting proxy. If clickjacking protection becomes a requirement later, front the deployment with a service that can set response headers (e.g., Cloudflare Pages or a CDN in front of GitHub Pages).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@demo/index.html` around lines 5 - 8, No code change is required in the demo/index.html CSP: frame-ancestors is ineffective in a meta tag, and GitHub Pages cannot provide the required X-Frame-Options or CSP response headers. Document this deployment limitation only if needed, and use a fronting service with configurable response headers if clickjacking protection becomes mandatory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@demo/demo-websocket.ts`:
- Around line 431-443: Update DemoWebSocket.send so it throws InvalidStateError
only when readyState is CONNECTING; for CLOSING or CLOSED, discard the data and
return before validating or enqueueing the handleClientMessage microtask, while
preserving the existing OPEN behavior.
In `@demo/style.css`:
- Line 14: Update the Stylelint configuration to enable camelCase SVG-derived
keywords via the value-keyword-case option’s camelCaseSvgKeywords setting, so
the optimizeLegibility declaration in the stylesheet passes linting without
changing its casing.
In `@tests/fixtures/browser/pages-demo-fixture.ts`:
- Around line 105-110: Update the catch handler in the request handler to guard
writeHead with a response.headersSent check, avoiding header writes after
response.end has begun. Replace the raw error response with a fixed generic body
and log the caught error via console.error for diagnostics.
- Around line 130-134: Update closeServer to call server.closeAllConnections()
immediately after server.close() and before awaiting the 'close' event, ensuring
idle keep-alive connections are terminated while preserving the existing
listening guard.
---
Nitpick comments:
In @.github/workflows/pages.yml:
- Around line 36-37: Replace the unpinned registry installation in the “Install
the repository npm version” workflow step with a committed, integrity-locked
bootstrap lockfile or verified npm artifact. Apply the same locked bootstrap
approach to npm setup steps in the other workflows, preserving the required npm
version.
In `@demo/demo-websocket.ts`:
- Line 1: Export DEMO_SHELL and DEMO_CWD from the constants section alongside
DEMO_WEBSOCKET_URL in demo-websocket.ts, then import and reuse them throughout
app.ts for session validation and spawn() options instead of duplicating the
shell path and working-directory literals.
In `@demo/index.html`:
- Around line 5-8: No code change is required in the demo/index.html CSP:
frame-ancestors is ineffective in a meta tag, and GitHub Pages cannot provide
the required X-Frame-Options or CSP response headers. Document this deployment
limitation only if needed, and use a fronting service with configurable response
headers if clickjacking protection becomes mandatory.
In `@scripts/build-pages.js`:
- Around line 19-22: Update the replacement in the rendered HTML generation flow
to use replaceAll for every __LIT_SHELL_BUILD__ occurrence, while preserving the
existing renderedHtml === sourceHtml guard for detecting a missing placeholder.
In `@scripts/check-pages.js`:
- Around line 25-31: Reorder the assertions in the actualFiles loop so
fileStats.isSymbolicLink() is checked before fileStats.isFile(). Preserve both
validations and ensure symlinks report “must not be a symlink” while other
non-regular entries report “must be a regular file.”
In `@tests/browser/pages-demo.spec.ts`:
- Around line 11-17: Update the fixture variable used by the beforeAll and
afterAll hooks to allow an unset value, and guard fixture.close() in afterAll so
cleanup runs only when startPagesDemoFixture successfully assigned a fixture.
Preserve the original fixture-startup error when initialization fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 766a3c6c-7b9f-47dd-9412-b6dc8ab03a39
⛔ Files ignored due to path filters (1)
demo/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/pages.yml.github/workflows/release.yml.gitignore.markdownlint-cli2.mjs.prettierignoreREADME.mddemo/app.tsdemo/demo-websocket.tsdemo/index.htmldemo/style.csseslint.config.mjsknip.jsonpackage.jsonscripts/build-pages.jsscripts/check-pages.jstests/browser/pages-demo.spec.tstests/demo-websocket.test.tstests/fixtures/browser/pages-demo-fixture.tstsconfig.json
| send(data: string | ArrayBufferLike | Blob | ArrayBufferView): void { | ||
| if (this.readyState !== DemoWebSocket.OPEN) { | ||
| throw new DOMException('WebSocket is not open', 'InvalidStateError'); | ||
| } | ||
| if (typeof data !== 'string') { | ||
| throw new TypeError('The demo protocol accepts text messages only'); | ||
| } | ||
|
|
||
| queueMicrotask(() => { | ||
| if (this.readyState === DemoWebSocket.OPEN) | ||
| this.handleClientMessage(data); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '400,465p' demo/demo-websocket.ts
echo "== ws.send usages in repo =="
rg -n "\.send\(" --glob '*.{ts,js,mjs,mts}' . | head -200
echo "== WebSocket declarations/usages =="
rg -n "DemoWebSocket|implements WebSocket|readyState|CONNECTING|CLOSING|CLOSED" --glob '*.{ts,js,mjs,mts}' . | head -200
echo "== package scripts/tests =="
if [ -f package.json ]; then cat package.json; fiRepository: lsadehaan/lit-shell
Length of output: 22586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== browser fixture send-related excerpt =="
sed -n '120,175p' tests/fixtures/browser/lit-shell-fixture.ts
echo "== package-lock relevant DemoWebSocket install =="
node - <<'JS'
const fs = require('fs');
const p = 'package-lock.json';
if (!fs.existsSync(p)) { console.log('no package-lock.json'); process.exit(0); }
const lock = JSON.parse(fs.readFileSync(p, 'utf8'));
function find(name, path='node_modules') {
for (const k of Object.keys(lock.packages || {})) {
if (k.endsWith('node_modules/lit-shell') || k.includes(name)) {
console.log(k, lock.packages[k].version);
}
}
}
find('tests'); find('demo');
JS
echo "== send type/source in Node/typescript lib =="
rg -n "interface WebSocket|send\\(" node_modules/@types/node lib.dom.d.ts 2>/dev/null | head -100 || true
echo "== inspect lock commit/tag evidence if present =="
sed -n '165,230p' package-lock.json | grep -n "d06368b91b4382266881968086b9dc65b1cb40\|lit-shell" || trueRepository: lsadehaan/lit-shell
Length of output: 5649
🌐 Web query:
WHATWG WebSocket API send CLOSING CLOSED InvalidStateError discard data
💡 Result:
According to the WHATWG WebSockets Standard, the WebSocket.send method behavior regarding readyState is as follows: 1. InvalidStateError: The send method throws an InvalidStateError DOMException only if the WebSocket connection's readyState is CONNECTING [1][2]. 2. CLOSING and CLOSED states: If the readyState is CLOSING or CLOSED, the send method does not throw an exception; instead, it silently discards the data [3][4][5]. The specification dictates that the transmission steps only proceed if the connection is established and the closing handshake has not yet started [1][2]. Consequently, if the connection is in the process of closing or is already closed, the request to send data is ignored by the user agent [3][6].
Citations:
- 1: https://websockets.spec.whatwg.org/
- 2: https://whatpr.org/html/4288/web-sockets.html
- 3: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/send
- 4: Issue with "WebSocket.send()": Exceptions thrown listed is incorrect mdn/content#2589
- 5: https://github.com/mdn/content/blob/main/files/en-us/web/api/websocket/send/index.md
- 6: https://undici.nodejs.org/api/WebSocket
Do not throw from send() in CLOSING or CLOSED.
DemoWebSocket implements the browser WebSocket interface, but send() currently throws for every non-OPEN state. The WebSocket sending steps only throw when readyState is CONNECTING; for CLOSING and CLOSED, they discard the request. Match this behavior before enqueueing the handleClientMessage microtask.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@demo/demo-websocket.ts` around lines 431 - 443, Update DemoWebSocket.send so
it throws InvalidStateError only when readyState is CONNECTING; for CLOSING or
CLOSED, discard the data and return before validating or enqueueing the
handleClientMessage microtask, while preserving the existing OPEN behavior.
| color: #e7edf4; | ||
| background: #071019; | ||
| font-synthesis: none; | ||
| text-rendering: optimizeLegibility; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stylelint's value-keyword-case flags optimizeLegibility.
With the default camelCaseSvgKeywords: false, Stylelint's value-keyword-case rule requires optimizelegibility (lowercase) unless the rule option is explicitly enabled for camelCase SVG-derived keywords. Confirm the project's Stylelint config already sets camelCaseSvgKeywords: true; if not, this line fails the code-quality check referenced in the PR objectives.
🔧 One way to resolve this without changing the keyword casing
+ /* stylelint-disable-next-line value-keyword-case -- optimizeLegibility is the conventional SVG-derived casing */
text-rendering: optimizeLegibility;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| text-rendering: optimizeLegibility; | |
| /* stylelint-disable-next-line value-keyword-case -- optimizeLegibility is the conventional SVG-derived casing */ | |
| text-rendering: optimizeLegibility; |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 14-14: Expected "optimizeLegibility" to be "optimizelegibility" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@demo/style.css` at line 14, Update the Stylelint configuration to enable
camelCase SVG-derived keywords via the value-keyword-case option’s
camelCaseSvgKeywords setting, so the optimizeLegibility declaration in the
stylesheet passes linting without changing its casing.
Source: Linters/SAST tools
| async function closeServer(server: Server): Promise<void> { | ||
| if (!server.listening) return; | ||
| server.close(); | ||
| await once(server, 'close'); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package files =="
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json)$' || true
echo
echo "== engines declarations =="
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').rglob('package.json'):
if any(part in {'.git','node_modules','dist','build','.turbo'} for part in p.parts):
continue
data=json.loads(p.read_text())
print(p)
for k in ('engines','volta'):
if k in data: print(' ', k, ':', data[k])
PY
echo
echo "== fixture lines =="
sed -n '1,190p' tests/fixtures/browser/pages-demo-fixture.ts 2>/dev/null || true
echo
echo "== dependency mentions for Playwright/Node =="
rg -n '"playwright"|playwright|node@|>=18|engines:|typescript' package.json tests pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: lsadehaan/lit-shell
Length of output: 16783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package files =="
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json)$' || true
echo
echo "== engines declarations =="
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').rglob('package.json'):
if any(part in {'.git','node_modules','dist','build','.turbo'} for part in p.parts):
continue
data=json.loads(p.read_text())
print(p)
for k in ('engines','volta'):
if k in data:
print(' ', k, ':', data[k])
PY
echo
echo "== fixture lines =="
sed -n '1,190p' tests/fixtures/browser/pages-demo-fixture.ts 2>/dev/null || true
echo
echo "== dependency mentions for Playwright/Node =="
rg -n '"playwright"|playwright|node@|>=18|engines:|typescript' package.json tests pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: lsadehaan/lit-shell
Length of output: 16783
🌐 Web query:
Playwright browser tests keep alive connections closeAllConnections documentation
💡 Result:
There is no method named closeAllConnections within the official Playwright public API [1][2][3]. The confusion likely stems from the Node.js http.Server.prototype.closeAllConnections method, which is occasionally used internally by Playwright's test infrastructure [4][5] or misidentified by users encountering errors in their own code [4]. Key distinctions regarding connection management in Playwright: 1. Node.js closeAllConnections: This is a native Node.js method added in v18.2.0 [4][6]. It is intended to forcefully close all established HTTP(S) connections to a server [7][8]. If you see this method mentioned in the context of Playwright, it is typically referring to Playwright's internal test harness or a custom server implementation, not a public Playwright API [4][5]. 2. Playwright Browser Lifecycle: To manage browser connections gracefully in Playwright, you should use official API methods: - browserContext.close: Closes a specific context and its pages [1][2]. - browser.close: Closes the entire browser instance [1][2]. It is best practice to close individual contexts first to ensure artifacts (like HARs or videos) are flushed [1][2]. 3. Keep-Alive and Connection Management: - Playwright's APIRequestContext: By default, it uses the Node.js HTTP agent. If you are experiencing connection issues or need persistent connections, you may need to configure the underlying HTTP agent (e.g., using libraries like hpagent) [9]. - Request Headers: If a server requires keep-alive connections, you can explicitly add the Connection: keep-alive header to your request configuration [10]. If you are encountering a "closeAllConnections is not a function" error, verify that you are not mistakenly calling this method on a Playwright object when you intended to use a native Node.js http.Server instance, and ensure your Node.js version is compatible if you are using that native method [4].
Citations:
- 1: https://playwright.dev/docs/api/class-browser
- 2: https://playwright.dev/docs/next/api/class-browser
- 3: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/browser.ts
- 4: [Bug]: "TypeError: this._server.closeAllConnections is not a function" when running BiDi tests on Linux microsoft/playwright#34288
- 5: node:http: Server.closeAllConnections() shuts down the listening socket oven-sh/bun#31301
- 6: http: added closeAllConnections and closeIdleConnections to http.server nodejs/node#42812
- 7: http: server.closeAllConnections does not destroy upgraded (web)sockets nodejs/node#53536
- 8: https://stackoverflow.com/questions/77196622/does-server-closeallconnections-also-close-idle-connections
- 9: [Feature] playwright.request creates a new connection per request microsoft/playwright#14155
- 10: [Bug]: route.continue() works but route.fetch() with route.fulfill() times out microsoft/playwright#31037
Close idle keep-alive sockets before waiting for server close.
server.close() stops accepting new connections, but await once(server, 'close') still waits for existing connections to drain. If a Playwright browser keeps the fixture server alive after teardown, add server.closeAllConnections() before awaiting the server’s 'close' event. closeAllConnections() is supported in this project’s declared Node engine range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/fixtures/browser/pages-demo-fixture.ts` around lines 130 - 134, Update
closeServer to call server.closeAllConnections() immediately after
server.close() and before awaiting the 'close' event, ensuring idle keep-alive
connections are terminated while preserving the existing listening guard.
2 participants
Summary
Safety boundary
The demo accepts only demo://terminal, uses no native WebSocket, fetch, XHR, eval, child process, PTY, or Docker API, and ships with connect-src none. Unsupported commands are rejected by an allowlist interpreter.
Validation
Deployment
Pages is configured for GitHub Actions with a master-only environment policy. After merge, the workflow publishes https://www.idnteq.net/lit-shell/ from the reviewed artifact and verifies the exact deployed commit SHA.