fix: resolve evaluation serialization, benchmark metrics, encoding, and script stability issues

[AbhiPra24]

Summary of Changes

This PR resolves multiple reliability, platform compatibility, and metric calculation bugs across the skills repository:

1. mcp-builder ():

   • Fixed by extracting text content from MCP result blocks before serializing to prompt context.
   • Captured full response text across all content blocks.

2. skill-creator ():

   • Fixed delta sign inversion where alphabetical sorting caused old_skill to be treated as primary and with_skill as baseline.
   • Preserved eval_name from eval_metadata.json in benchmark.json runs.
   • Enforced UTF-8 encoding across file operations.

3. skill-creator eval viewer ( & ):

   • Fixed escapeHtml() to encode double and single quotes (", '), preventing layout breakage and attribute injection in tooltips.
   • Escaped </script> tag sequences in embedded JSON.
   • Added UTF-8 encoding to file read/write operations.

4. skill-creator validation & utils (, , ):

   • Added zero-dependency fallback YAML parsing to quick_validate.py so validation works even if PyYAML is not installed.
   • Enforced non-empty requirements for name and description.
   • Fixed multiline YAML parsing in utils.py across blank lines.
   • Fixed import fallback in package_skill.py and corrected CLI help documentation.

5. webapp-testing ():

   • Used DEVNULL for piped subprocess streams to prevent buffer deadlocks on long builds.
   • Terminated child process groups on shutdown to prevent orphaned server processes from occupying ports.

6. pdf & docx & office tools:

   • Handled PDFs without AcroForm fields in extract_form_field_info.py to avoid AttributeError.
   • Created output directories automatically in convert_pdf_to_images.py.
   • Used cross-platform temporary directories and fixed timeout error handling in accept_changes.py.
   • Bypassed Linux-only socket shims on macOS and Windows in soffice.py.

7. web-artifacts-builder ():

   • Cleaned all <link rel="icon" tags for modern Vite scaffolds.
   • Guarded tsconfig.app.json updates when the file is absent.

8. Skills Frontmatter Compliance:

   • Updated claude-api and claude-academy-guide descriptions to comply with the 1024-character specification limit.

Testing & Verification

• Validated all 19 skills using quick_validate.py (100% pass rate).
• Compiled all modified Python scripts without syntax or type errors.
• Verified viewer HTML encoding and benchmark delta calculation.

[98zc5g5jyw-arch]

Babysit review — please split and rebase

This PR mixes 5 unrelated change types (utf-8 batch fix, MCP serialization, soffice platform shim, viewer.html XSS escaping, custom YAML fallback parser) across 18 files AND is currently dirty (conflict — the claude-academy-guide dir was renamed to academy-guide on main). Two specific notes:

• The custom 73-line YAML fallback parser duplicates what skill-creator: declare pyyaml dependency for quick_validate.py #1599 already solved by declaring pyyaml>=6.0 — prefer the dependency.
• utf-8 fixes overlap heavily with fix(skill-creator): add encoding=utf-8 to all text file operations #1591/fix(skill-creator): enforce utf-8 encoding across scripts and fix claude-api max description length #1596/skill-creator: fix three eval-harness defects that silently report a 0.0 trigger rate #1629/skill-creator: add --scan-full-turn, so evals measure whether a skill fires at all rather than whether it fires first #1630; only merge the pieces not already covered.
  Suggest splitting into 2-3 focused PRs and rebasing on latest main.

[AbhiPra24]

Rebased cleanly on latest main and addressed the review feedback:

1. Rebase & conflict resolution: Resolved the claude-academy-guide conflict (dropped redundant changes since Rename claude-academy-guide skill to academy-guide and shorten its description #1605 renamed and updated it upstream).
2. YAML dependency: Removed the custom fallback parser in quick_validate.py, relying directly on pyyaml per skill-creator: declare pyyaml dependency for quick_validate.py #1599.
3. Scope consolidation: Trimmed overlapping changes; retained targeted fixes for:
   • MCP TextContent block serialization & multi-block capture (mcp-builder/scripts/evaluation.py)
   • Benchmark delta sign calculation & eval name retention (skill-creator/scripts/aggregate_benchmark.py)
   • HTML quote and script tag escaping (eval-viewer/viewer.html, generate_review.py)
   • Subprocess pipe deadlock prevention and process group cleanup (webapp-testing/scripts/with_server.py)
   • Cross-platform socket shim handling on non-Linux systems (soffice.py)
   • Frontmatter 1024-character specification compliance (claude-api/SKILL.md)
4. Verification: All 19 skills passed validation via quick_validate.py and all modified Python scripts compiled cleanly.

[98zc5g5jyw-arch]

Review: fix: resolve evaluation serialization, benchmark metrics, encoding, and script stability issues

Verdict: Approve with minor comments (COMMENT)

No critical issues found. The serialization, metric-direction, encoding, and stability fixes are genuine and verified against the actual data shapes in the repo (e.g. connections.call_tool returns result.content, a list of content blocks, so the new _serialize_tool_result list branch actually triggers). A few robustness nits and one bounded busy-spin edge case below.

Verified correct (looks good)

• mcp-builder/scripts/evaluation.py:86-100 — _serialize_tool_result correctly unwraps TextContent.text for the real list[ContentBlock] shape returned by connections.py:70 (result.content). Confirmed with a mock of the SDK types.
• skill-creator/scripts/aggregate_benchmark.py:211-218 — delta direction fix is order-independent and correct; checked all 8 orderings of with_skill/without_skill/new_skill/old_skill, including the alphabetical-sort case that caused the original sign inversion. eval_name preservation (lines 132, 249) is clean.
• skill-creator/eval-viewer/generate_review.py:280 — .replace("</", "<\\/") is safe: json.dumps never emits a bare \/ (backslashes are doubled), so no double-escaping; \/ round-trips through JSON.parse to /; the </script sequence is gone from the inline script. Verified by round-trip test.
• skill-creator/eval-viewer/viewer.html:1097-1102, 1133-1137, 1229-1242 — escapeHtml now encodes " and '; all interpolated fields are wrapped. Correct.
• skill-creator/scripts/run_eval.py:102-114, 132-152 — removing the unconditional break after the post-exit drain (which skipped line processing) and replacing early return False with continue/pending_tool_name = None fixes real false negatives. The dead select path is now live.
• docx/scripts/accept_changes.py:78-82 — timeout now returns an error message instead of a fake success (CLI already gates on "Error" in message, so this is a genuine fix).
• pdf/scripts/extract_form_field_info.py:48 (get_fields() or {}), pdf/scripts/convert_pdf_to_images.py:10 (os.makedirs(..., exist_ok=True)), quick_validate.py (UTF-8 read + empty name/description now rejected), utils.py continuation handling (blank lines inside folded descriptions), soffice _needs_shim platform gate (soffice.py:55-63), with_server.py process-group kill, init-artifact.sh sed/existsSync guards — all correct.
• No new imports beyond stdlib; no dependency-declaration violations.

Warnings

1. skill-creator/scripts/run_eval.py:101-114, 170-171 — busy-spin on truncated final line. If the process exits with a partial line in buffer (no trailing \n), process.stdout.read() keeps returning b"", the line-processing loop never fires, and poll() is not None and not buffer is False, so the loop spins (CPU burn) until the outer time.time() - start_time < timeout expires. Bounded by timeout, results unaffected, but a worker burns a full core for up to timeout seconds. Suggest tracking EOF (e.g. break when process.poll() is not None and not remaining or when read returns b""), not just when the buffer is empty.
2. Incomplete UTF-8 enforcement in aggregate_benchmark.py — the PR's stated goal ("Enforced UTF-8 encoding") is only partially applied: open(timing_file) (line 146) and both output writes open(output_json, "w") / open(output_md, "w") (lines 387/393) still use locale-default encoding. On Windows (cp1252 default), a non-ASCII skill name or eval name will raise UnicodeEncodeError when writing benchmark.json/.md. Same for pdf/scripts/extract_form_field_info.py:113 (open(json_output_path, "w")). This diverges from the encoding="utf-8" standard already applied in quick_validate.py/utils.py here and in #1591/#1596.
3. webapp-testing/scripts/with_server.py:70-77 — DEVNULL discards server logs entirely; a server that starts, binds the port, then crashes at request time is now undiagnosable, and "Server N stopped" (line 113) prints even when the kill was swallowed. Acceptable trade-off for the deadlock fix, but consider redirecting to a per-server temp log file (or at least documenting the loss).
4. docx/scripts/accept_changes.py:18, 63 — file://{LIBREOFFICE_PROFILE} uses a raw temp path. tempfile.gettempdir() on Windows (e.g. C:\Users\First Last\AppData\Local\Temp) contains spaces and backslashes that break the unencoded file:// URI; also a fixed shared profile dir is unsafe for concurrent runs (pre-existing). Prefer Path(...).as_uri() for the -env:UserInstallation value.

Suggestions

• mcp-builder/scripts/evaluation.py:86-100 — guard block.text being None (parts.append(block.text or "")): a None text member currently raises TypeError inside the join, which the caller's try/except degrades into an "Error executing tool" message. Also consider defensively unwrapping result.content when a non-list CallToolResult-like object is passed (older SDK shapes fall back to str() repr today).
• soffice.py:50 / accept_changes.py:14 — move the newly added import platform / import tempfile to the top import block (PEP8); mid-file placement after other imports is legal but easy to miss in review.
• skill-creator/scripts/package_skill.py:116-119 — main()'s usage text still says python utils/package_skill.py while the docstring now says python -m scripts.package_skill; align them.
• skills/claude-api/SKILL.md:2-3 — the description/trigger text was substantially trimmed (SKIP clause dropped the "overrides all triggers" phrasing). This changes trigger behavior for every consumer of the skill; please confirm the reduced wording was intentional (token savings) and still covers the intended skip cases.

[AbhiPra24]

Thanks for the thorough and constructive review @98zc5g5jyw-arch! I've addressed all the feedback in commit 851a1a9:

1. EOF handling & busy-spin fix (run_eval.py): Updated loop termination to break immediately once process.poll() is not None after stdout is drained, preventing any CPU burn on trailing buffers without newlines.
2. UTF-8 encoding consistency: Added explicit encoding="utf-8" across aggregate_benchmark.py (timing file read, JSON output, Markdown output) and extract_form_field_info.py (JSON output).
3. Robustness & Windows paths (accept_changes.py): Used Path(LIBREOFFICE_PROFILE).as_uri() for -env:UserInstallation to safely handle backslashes and spaces on Windows.
4. Tool result unwrapping (evaluation.py): Added defensive result.content unwrapping and guarded against None in getattr(block, "text") or "" before joining.
5. PEP 8 imports & usage formatting: Moved import platform / import tempfile to the top-level import blocks in soffice.py and accept_changes.py, and aligned package_skill.py's usage help with python -m scripts.package_skill.
6. Trigger text (claude-api/SKILL.md): The description length was trimmed to fit strictly within the 1024-character limit mandated by the specification while preserving all core trigger keywords (Claude, Anthropic, Fable, Opus, Sonnet, Haiku, us.anthropic.*) and the cross-provider skip heuristics.

All 19 skills passed validation via quick_validate.py.

[98zc5g5jyw-arch]

Re-review after 851a1a9: review feedback addressed — soffice UserInstallation now uses Path.as_uri(), tool_result.content handled via hasattr/getattr, empty text blocks guarded with 'or ""', UTF-8 encoding added to JSON writes, import ordering fixed. All changes correct. LGTM.