This repository was archived by the owner on Aug 24, 2026. It is now read-only.
Repository navigation
Fix false-green network failures + robustness gaps (review round 2) - #22
Merged
Merged
Conversation
Multi-dimension review (verified against installed chrome-devtools-mcp@1.1.0)
surfaced a class of false-negatives plus several robustness/validation gaps.
Headline bug — transport-layer failures scored as PASS:
NetworkFormatter emits `status` as a STRING: a numeric code for completed
responses, but `failure.errorText` ("net::ERR_CONNECTION_REFUSED", DNS/blocked)
for transport failures and "pending" for in-flight requests. The old
`Number(status) >= 500/400` checks turn those into NaN, which is neither >=500
nor >=400, so a request that flat-out failed produced neither a failure nor a
warning — analyzeQuality returned 'pass' and the run exited 0. The
assert-no-http-errors scenario had the same NaN blind spot. Both now share a
classifyNetworkStatus() helper (new core/network.mjs) so they cannot drift.
Also fixed:
- devtools-client: pass maxBuffer (64 MiB) to execFile; large snapshot/network/
console JSON would otherwise crash the run at Node's 1 MiB stdout default.
- runner: isolate each post-scenario collection (console/network/lighthouse) so
one failure (e.g. a Lighthouse timeout) no longer discards the whole report;
writeReport now runs in finally so the report is always persisted best-effort.
- cli: validate --timeout (reject valueless/NaN/<=0 instead of failing every wait
scenario instantly); sanitize profile.name and guard outDir so a malicious
--out/name can't fs.rm an ancestor of cwd.
- quality: favicon-404 ignore now compares URL pathname (matches cache-buster
query strings); also surface console 'assert' messages, not just 'error'.
- snapshot: findBySpec requires a positive matcher (role/nameIncludes) so a
missing selector no longer silently matches the root node.
- chatbot: empty-input adds an answerDoneText-count oracle (catches an app that
echoes blank input by reusing nodes, which the node-delta heuristic misses).
- profile: require consentAgreeButton for consent scenarios; validate viewport
format and integer maxNodeDelta.
- docs: document common scenario fields, consent requirement, screenshot/fileName,
transport-failure handling.
- tests: +16 (network classifier, scenario oracles, profile validation,
findBySpec guard, transport-failure regression). 31 pass.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
배경
멀티 에이전트 코드리뷰(5개 차원 + 적대적 검증, 설치된
chrome-devtools-mcp@1.1.0소스 대조)에서 나온 확정 발견을 수정합니다. 각 수정은 실제 dep 소스로 검증했고, 변경분은 자기 적대적 검증(회귀/vacuous 테스트 탐지)도 통과했습니다.핵심 버그 — transport 실패가 PASS로 처리됨 (false-green) 🔴
NetworkFormatter.#getStatusFromRequest는status를 문자열로 방출합니다:"200","502")failure.errorText("net::ERR_CONNECTION_REFUSED", DNS 실패, 차단)"pending"기존
Number(status) >= 500/400비교는Number('net::ERR_...')→NaN이라 둘 다 false. 즉 요청이 완전히 실패해도 failure도 warning도 아님 →analyzeQuality가'pass'반환, exit 0. API가 전부 연결 거부되는 최악의 경우가 CI에서 초록불로 통과했습니다.assert-no-http-errors도 동일한 NaN 맹점이 있었습니다.→ 공유 헬퍼
classifyNetworkStatus()(신규core/network.mjs)로 두 경로를 통일해 drift를 방지했습니다.그 외 수정
devtools-clientmaxBuffer(64 MiB) — 대형 snapshot/network/console JSON이 Node 1 MiB 기본값에서 런을 크래시시키던 문제runnerwriteReport를 finally로 옮겨 항상 best-effort 저장cli--timeout검증(값 없음/NaN/≤0 거부),profile.name/--out경로 방어(cwd 상위 디렉터리fs.rm방지)qualityassert타입도 surfacesnapshotfindBySpec가 양성 매처(role/nameIncludes) 필수 — 누락 셀렉터가 root에 매칭되던 문제chatbotprofiledocs검증
npm run check통과,npm test31/31 pass (+16 신규: 네트워크 분류기, 시나리오 오라클, 프로필 검증, findBySpec 가드, transport 실패 회귀)리뷰에서 false positive로 걸러낸 것 (수정 안 함)
페이지네이션 기본값(전체 반환이 맞음), 이미 수정된 answerDoneText/maxNodeDelta/pollSnapshot/parseJsonOutput, README drift 등은 거짓으로 확인되어 제외.