fix: batch quick-fixes (4 issues)

[andreasronge]

Summary

Batch fix for quick-fix issues from recent PR reviews.

Issues Fixed

• Closes [From PR #906] signature: "any" + output_schema mutual exclusivity edge case #907: signature: "any" + output_schema mutual exclusivity edge case
• Closes [From PR #914] Add HTTP upstream metadata test for parse_http_upstream/3 #915: Add HTTP upstream metadata test for parse_http_upstream/3
• Closes [From PR #914] Clean up repeated tmp_dir setup in metadata tests #916: Clean up repeated tmp_dir setup in metadata tests
• Closes [From PR #917] Extract shared server header helper in CatalogDescription #918: Extract shared server header helper in CatalogDescription

Issues Skipped

• [From PR #919] check_configured/2 hardcodes "catalog/list-tools" in error message #921 and [From PR #919] catalog all_server_info/1 missing defensive try/catch around GenServer.call #922: Both reference catalog_builtins.ex which only exists on the unmerged PR feat(mcp): catalog/ PTC-Lisp namespace with 4 builtins #919 branch (claude/911-catalog-namespace-builtins). These should be fixed when that branch is merged.

Changes

#907 — tools.ex: validate_output_contract/1 was checking sig_present using a simple nil-check, which treated signature: "any" as present. But validate_signature/1 normalizes "any" to {:ok, nil} (absent). Fixed the presence check to exclude "any" values using the same String.trim/1 logic. Added a regression test in output_schema_arg_test.exs.

#915 — application_phase1b_test.exs: Added "http entry preserves description and capabilities in metadata" test to the existing upstream metadata preservation describe block, completing coverage for the HTTP path of extract_upstream_metadata/1.

#916 — application_phase1b_test.exs: Refactored the 3 new metadata tests and the handshake_timeout_ms test to accept %{tmp_dir: tmp_dir} from the existing setup block and use the write_config/2 helper instead of creating their own inline tmp directories.

#918 — catalog_description.ex: Extracted render_server_header/1 (3 clauses: nil/empty/list tools) to replace the 6 duplicated clauses across render_inline_server/1 and render_lazy_server/1. The lazy path now delegates entirely to render_server_header/1; the inline path calls it and appends the tool listing for non-empty tool lists.

---

🤖 Generated with Claude Code

[claude]

PR Review: fix: batch quick-fixes (4 issues)

Summary

Clean batch PR that fixes four independent quick-fix issues from recent reviews. All changes are well-scoped, correct, and each issue is properly addressed with good test coverage.

What's Good

• [From PR #906] signature: "any" + output_schema mutual exclusivity edge case #907 fix is precise: The validate_output_contract update correctly mirrors the String.trim(v) == "any" logic from validate_signature/1, keeping the two functions consistent. The case expression is clearer than the old match? one-liner.
• [From PR #917] Extract shared server header helper in CatalogDescription #918 extraction is clean: render_server_header/1 elegantly unifies the 6 duplicated clauses (3 in render_inline_server + 3 in render_lazy_server) into 3, and the render_inline_server catch-all clause correctly falls through for both nil and [] cases thanks to the when is_list(tools) and tools != [] guard.
• [From PR #914] Clean up repeated tmp_dir setup in metadata tests #916 refactoring is thorough: All four tests now use the shared setup block's tmp_dir and write_config/2 helper, eliminating the manual mkdir_p!/File.write!/on_exit boilerplate.
• [From PR #914] Add HTTP upstream metadata test for parse_http_upstream/3 #915 HTTP test: Good coverage addition — the test verifies both that metadata is preserved and that description/capabilities are stripped from config (with both atom and string key checks).
• Regression test for [From PR #906] signature: "any" + output_schema mutual exclusivity edge case #907 is well-named and tests the exact edge case.

Issues (Must Fix)

None.

Suggestions (Optional)

1. File size of tools.ex — mcp_server/lib/ptc_runner_mcp/tools.ex is at 1111 lines. This PR only adds 9 lines (a minor edit), so no action needed here, but it's worth tracking for future work that adds more logic to this module.

Security

No concerns. The String.trim(v) != "any" comparison is purely on the server-side argument validation path with no user-facing input risk.

Documentation

No updates needed. The changes are internal implementation fixes with no public API impact.

Verdict

Approve. All four issues are correctly addressed with proper tests. The code is clean and the refactoring reduces duplication without changing behavior.

[claude]

Auto-Triage Summary

Issue	Decision	Action
tools.ex at 1111 lines — worth tracking for future refactor	DEFER_ISSUE	Created #930

No FIX_NOW items — all must-fix issues from review are clean.