Skip to content

test: drop the unread reply assignment in the near-miss path loop - #100

Merged
askalf merged 1 commit into
mainfrom
test/near-miss-unused-reply
Sep 27, 2026
Merged

askalf merged 1 commit into
mainfrom
test/near-miss-unused-reply

Conversation

@askalf

@askalf askalf commented Sep 27, 2026

Copy link
Copy Markdown
Owner

Resolves CodeQL alert #22 (js/useless-assignment-to-local, _test_proxy.mjs:294).

The near-miss path loop asserts that upstream never saw raw PII and that X-Redacted >= 2. It also read the reply into text and never checked it. This removes that line.

A restore assertion was not added in its place: the echo stub routes on req.url.includes("/chat/completions"), case-sensitive and undecoded, so it answers /v1/Chat/Completions and /v1/chat/completion%73 with a 404 even though cordon classifies and redacts both. Restore on those paths is a stub question, not something this loop tests.

npm test: all suites pass (proxy 115/0).

The loop asserts on what reached upstream and on X-Redacted; the reply body was read into text and never checked (CodeQL js/useless-assignment-to-local, alert #22).
@github-actions github-actions Bot added tests Test suite and CI size/XS Under 10 hand-written lines labels Sep 27, 2026

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve. Removes a body read in the near-miss path loop whose result was never asserted on. The remaining assertions use the response headers and the stub's recorded upstream calls, both available once the response resolves, so the loop's coverage is unchanged and it now matches the sibling loop below it. The text variable is still declared and used by the other sections.

@askalf
askalf merged commit 7fe39d9 into main Sep 27, 2026
10 checks passed
@askalf
askalf deleted the test/near-miss-unused-reply branch September 27, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Under 10 hand-written lines tests Test suite and CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants