fix: the fifth leaking branch, and a claim that outran its code again - #216
Merged
Conversation
A verification pass on #215 returned NOT CONFIRMED. That commit asserted "every branch that returns endpoint-authored text now goes through one #safe()". A fifth branch did not: error: streamError ?? this.#safe(describeUnusableBody(rawSeen)) The scrub sat on the FALLBACK. `streamError` holds the endpoint's own message, parsed straight out of an SSE `error` event by both wire-protocol parsers, and it reached DispatchResult.error — and logs/dispatches.jsonl — verbatim. It is also the likeliest shape of all of them: a 200 with text/event-stream carrying an error object is how OpenAI-compatible gateways and Anthropic both report an auth failure once the stream is open, and jobs only ever stream. Reproduced against the built artifact with both a URL-embedded key and one echoed from the Authorization header. That is the second time a commit message about this file claimed a wider fix than the code delivered, so the #safe docblock no longer asserts completeness — it enumerates the branches, and the enumeration is checked. Also: describeFetchFailure has always scrubbed the base URL but was never given the api key, leaving the last unscrubbed-for-key site in the file. Sabotage, each separately: moving #safe back onto the fallback fails exactly the two mid-stream tests; dropping the key argument fails exactly the fetch-failure test. Verified at the built artifact, not the diff — all seven error branches driven with a stubbed fetch, no leak on any. npm run check: 1226 passed, 8 skipped, exit 0.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A verification pass on #215 returned NOT CONFIRMED.
#215 asserted that "every branch that returns endpoint-authored text now goes through one
#safe()". A fifth branch did not:The scrub sat on the fallback.
streamErrorholds the endpoint's own message, parsed straight out of an SSEerrorevent by both wire-protocol parsers, and it reachedDispatchResult.error— andlogs/dispatches.jsonl— verbatim.It is also the likeliest shape of the lot: a 200 with
text/event-streamcarrying an error object is how OpenAI-compatible gateways and Anthropic both report an auth failure once the stream is open, and jobs only ever stream. Reproduced against the built artifact with a URL-embedded key and one echoed from theAuthorizationheader.The claim, not just the code
This is the second commit message about this file to describe a wider fix than the code delivered. The
#safedocblock no longer asserts completeness — it enumerates the branches that assignDispatchResult.errorand says which are ours and which are scrubbed. An enumeration can be checked against the file; a claim of completeness cannot.Also fixed:
describeFetchFailurehas always scrubbed the base URL but was never given the api key — the last unscrubbed-for-key site in the file.Sabotage
#safemoved back onto the fallbackdescribeFetchFailureVerified at the artifact
All seven error branches driven against built
dist/with a stubbed fetch, no network:npm run check: 1226 passed, 8 skipped, exit 0.