test: keep harness relay logs when a test fails

[afrind]

A relay that exits non-zero at teardown prints its sanitizer report, from the first ERROR or WARNING line, or its log tail if it has none. A failed run saves the relay logs, configs and actor output to .scratch/moq_harness_logs/[_picoquic] without --save-logs, and CI uploads that directory as an artifact when the test step fails.

---

This change is 

Summary by CodeRabbit

• New Features

  • Failed test runs now save diagnostic logs by default, even when log saving was not explicitly requested. Relay failures display a sanitizer report when available, or a tail of the log otherwise.
  • CI uploads logs from failed builds as downloadable artifacts, retained for seven days. Log directories distinguish runs using the picoquic QUIC stack.

• Documentation

  • Updated test-harness guidance to describe log levels, saved log naming, and the diagnostics shown for relay failures.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openmoq/moqx/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e9acfe95-ac10-4973-ba23-107d13fad560

📥 Commits

Reviewing files that changed from the base of the PR and between d71b2ea and 787627d.

📒 Files selected for processing (5)

• .github/workflows/ci-main.yml
• .github/workflows/ci-pr.yml
• docs/dev/test-harness.md
• test/lib/moq_harness.py
• test/test_relay_chain.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The harness now prints sanitizer reports or log tails for abnormal relay exits and saves logs by default for failed runs. Saved-log directories distinguish non-default QUIC stacks. Both CI workflows upload available harness logs from failed build jobs.

Changes

Harness Failure Logs

Layer / File(s)	Summary
Harness log handling test/lib/moq_harness.py, docs/dev/test-harness.md, test/test_relay_chain.py	The harness prints up to 400 lines from the first sanitizer report, or the last 40 log lines when no report is found. Failed runs save logs by default. Saved-log paths include a QUIC stack suffix except for mvfst. Documentation and usage text describe the logging behavior and path.
Failed-job log artifacts .github/workflows/ci-main.yml, .github/workflows/ci-pr.yml	Each workflow uploads available harness logs when a build job fails. Artifact names include the matrix lane, and retention is seven days.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other

Suggested reviewers: gmarzot

Merge Risk: ⚪ Minimal · up to 78762

Failed relay runs retain harness logs, and the CI workflows upload available logs on failure. No actionable merge-blocking defect is established; the change appears ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 78762

Failure artifacts make debugging easier, but they also retain and publish more test output. No sensitive-data exposure was established; the main remaining question is who can read the artifacts and what the output may contain.

Retained concerns

• Low · security · inferred: Failure-only uploads take unfiltered relay logs, actor output, and YAML from a persistent directory. The source does not establish an artifact audience or a content-sensitivity rule, so the confidentiality consequence of publishing those files remains unresolved.

Security review details

Security Blast Radius

• inferred — The changed exposure is confined in the inspected source to test diagnostics and CI artifacts, not a production service entrypoint. A failing PR test can affect the artifact contents for its CI build job; artifact reader permissions are not established here.

Security Findings and Attack Paths

• inferred — Relay and actor subprocess output now reaches a downloadable failure artifact without content filtering. No credential-bearing output or unauthorized artifact reader was established, so this is a conditional disclosure path rather than a verified leak.

Trust Boundaries and Controls

• observed — Failure gating, seven-day retention, a dedicated harness artifact path, and separation of the build token from the test step limit the new flow. The workflows do not specify artifact-read policy or redact retained subprocess output.

Resilience and Maintainability Implications

• inferred — A first handled termination reaches finalization, but a second signal is deliberately fatal. Copy failure or repeated use of the same saved directory can leave incomplete or mixed diagnostics; the evidence does not establish that CI schedules an identical test twice in one workspace.

Hardening Proposals

• proposed — Confirm artifact readership and permitted diagnostic content; if either requires stricter isolation, filter sensitive output and use an invocation-owned upload directory.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (3 skipped: 3…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving harness relay logs when tests fail.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[gmarzot]

@gmarzot reviewed 5 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on afrind).