Clean up remaining build vocabulary and tighten technical claims#2
Merged
Merged
Conversation
Calling the published JS artifact a "binary" or "TypeScript executable" misled readers — the project ships `dist/index.js` (a JS file with a node shebang) via npm, no native binary anywhere. Replaced those usages in README, e2e test names, comments, and the e2e fixture body with "compiled output" / "built dist/index.js" / "compiled JS output" depending on context. Kept npm-`bin` jargon (BUILT_BIN, TSX_BIN, spawnCompiled) and the SPEC line 43 future-feature mention of `bun --compile` / Node SEA — those uses are accurate. Also corrected several other inaccuracies: - release.yml line 62: the comment claimed awk did NR/sub trimming, but the script only prints-after-match until the next header. Rewrote the comment to describe what the awk block actually does. - .npmignore: the group headers misclassified tests/ + tsconfig.json as "source files" and claimed CLAUDE.md is rendered into the GitHub release body (only CHANGELOG.md is). Rewrote both headers. - package.json description: omitted CLI mode. Replaced with one line that covers both MCP and CLI surfaces. - scripts/postbuild.mjs: misattributed the +x effect to npm `bin` resolution. Rewrote to credit the shebang-based launch path. - docs/SPEC.md: "All 7 error codes throw from core.ts" was imprecise — only five throw explicitly; network_error, timeout, and sometimes http_error are translated by classifyError. Tightened the wording. - src/core.ts: softened the HTTP/2 framing comment to acknowledge the plain- HTTP fallback the same comment already noted; documented the AbortError branch in classifyError that the comment previously omitted. Text-only changes. `npm run build` and `npm test` (50/50) pass.
A follow-up to b162fb2 that addresses items the previous pass kept as "npm-`bin` jargon" plus several technical inaccuracies surfaced by a fresh audit. Build vocabulary (the dist/ artifact is plain JavaScript transpiled by tsc — not a binary or compiled executable): - tests/e2e.test.ts: spawnCompiled → spawnBuilt, BUILT_BIN → BUILT_JS, "compiled output" → "built output" across 10 test names and comments. - tests/cli.test.ts: TSX_BIN → TSX_CLI. - src/index.ts: "node binary" → "path to node" in the argv comment. Kept legitimate uses intact: Bun's --compile / Node SEA references in docs/SPEC.md:43 (those tools really do produce binaries), npm's literal .bin/ folder paths, libuv "native" assertion, "pure Node" idiom. Technical accuracy in src/core.ts (comment-only — runtime untouched): - Sec-CH-UA decoy comment: Chrome's GREASE rotation changes BOTH the brand token and its version per major (130: "Not?A_Brand";v="99", 131: "Not_A Brand";v="24"). Previous comment claimed only the version rotated. - CommonMark escape rules: setext underlines are = or - on a line by themselves; list markers require -/+/* then whitespace or EOL. The previous "===" / "- " restatement was overly specific. - Sec-Fetch-User: ?1 is always-on; real browsers omit it on non-user-activated navigations. Added a one-line comment noting the deliberate simplification. - decodeEncodedCodeTags: trailing requirement is whitespace, /, or & (the start of >), not "end-of-tag". - "exit code 1" → "sets process.exitCode = 1" — matches cli.ts's actual mechanism documented in its own comments. Overreaching claims softened: - src/mcp.ts tool description + README:115: only pure client-rendered SPAs with no extractable static HTML return extraction_failed. SPAs with server-rendered or SEO-prerendered HTML extract whatever they ship. Previous wording implied a determinism the pipeline doesn't have. - README:68 + docs/SPEC.md:28: "Modern MCP clients hide content[]" replaced with concrete client names (Claude Code CLI, VS Code/Copilot). Behavior varies across clients; concrete examples are honest. - README:3 lede: "request rate" → "request fingerprint". markfetch has no rate-limit logic; the actual contrast is shape (HTTP/2 + headers), not pacing. - README:74 Stdio-clean ANSI clause: "could corrupt protocol framing" → "keeping stderr parseable for shell consumers". Stdout-discipline is already covered by the prior sentence in the same bullet. Stale PRD references (the PRD was deleted in commit 52e2139): - src/core.ts: dropped "PRD §4 calls out that" from the client-hints derivation comment. - tests/server.test.ts:436: "(Principle #4: stderr is fatal-only)" → "(stderr-is-fatal-only invariant per SPEC.md)". - tests/server.test.ts:670: "PRD §5: file at savePath is only ever the markdown" → reformulated to cite README and SPEC.md instead. Text-only changes. `npm run build` and `npm test` (50/50) pass.
|
vasylenko
added a commit
that referenced
this pull request
May 13, 2026
Two issues caused 14 failures on the windows-latest runner of PR #4 run #2: 1. tests/cli.test.ts and tests/_helpers.ts:spawnAndCaptureExit spawn `./node_modules/.bin/tsx` directly. On Windows that's a `.cmd` shim Node's native child_process.spawn cannot launch without `shell: true`. Switch both to `node --import <tsx-loader-file-url> <entry>`, which bypasses the shim entirely and works identically on all platforms. The loader URL is an absolute file:// URL resolved once at test startup, so it stays correct even when individual tests override the child cwd to a tmpdir. spawnClient (StdioClientTransport-based) is intentionally left alone — the MCP SDK handles cross-platform spawn internally, and the server tests passed on Windows already. 2. src/sandbox.ts:checkPath returned error messages that ran path arguments through JSON.stringify(), which escapes backslashes in Windows paths ('C:\\Users\\...'). The unit-test assertion `result.reason.includes(dir)` failed because the literal `dir` isn't a substring of the JSON-escaped form. Switch to single-quote framing ('<path>'); substring check works on Windows now and the error output is more readable for humans too. All 66 active tests pass locally on macOS; 2 win32-gated tests skip.
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
1 participant
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.
What
Follow-up to b162fb2 that addresses items the previous pass kept as "npm-
binjargon" plus several technical inaccuracies surfaced by a fresh audit. Text-only — no runtime behavior change.