chore: make PR validation evidence reliable

[Rana Singh (ranadeepsingh)]

Green PR checks can hide stale revisions, missing regression tests, or incomplete CI. External-contributor validation must also distinguish evidence from permission to edit, trigger CI, approve, or merge. This change makes those boundaries explicit, adds reusable validation helpers, and isolates credential-fetching CLI children from contributor-controlled Python configuration.

Related Issues/PRs

Follow-up to lessons from #2774 and recent external-contributor validation.

What changes are proposed in this pull request?

• Pin readiness to the expected PR head and live target. Report exact required checks, workflow approvals, evidence completeness and human approval separately.
• Collect build, ordered merge-parent, current job-attempt and named-test evidence. Bound Azure monitoring and response reads by the original deadline, with sanitized failures and explicit Azure/GitHub disagreement.
• Verify an already trusted guidance checkout, including ignored files and symlinks. Keep contributor permissions in the contributor skill and shared validation in the PR-loop skill.
• Remove Python-specific environment variables from native and Windows batch CLI children. An isolated parent alone does not prevent an inherited PYTHONPATH from loading contributor-controlled modules in a credential-fetching child.
• For jobs that intentionally publish no individual test results, require named local regression proof plus separate API-verified current-merge aggregate corroboration. Do not represent aggregate counts as named hosted outcomes or waive unexplained skips, expected publication, or CI-only hardware requirements.
• Require every changed file to serve the request. Keep the current lightweight review in the diff and preserve superseded feedback in verified hosted history.
• Exempt only complete diffs confined to .github/skills/ and reviews/ from the workflow's default Azure requirement; actual enforced GitHub policy remains separate. This PR changes tools/ci/tests/, so it is not exempt.

No product implementation, pipeline definition, release tooling, or dependency pin changes.

How is this patch tested?

• I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

Published head: 57f22136d7eacfc2e0d20d6722e3aae35938b87f, based on d34f762a785ee8a7c7c624806f8c112256168833. The current evidence index records the full 17-path source manifest, SHA-256 11aaf1ffda6ce9e088caeae3e7dc608dcd78e36f297997c04c296cc7db7a3ee5, and full diff, SHA-256 6214a751f6335039ffc0d85048dc978c6aa1d5d025c5666b1ce1a3a175aed944. The final diff has 17 source/test/documentation files and three review-evidence files. Superseded findings and dispositions remain in verified hosted history.

python -B -m pytest tools/ci/tests/test_collect_ci_evidence.py tools/ci/tests/test_watch_azure_pipeline.py -v -rs --tb=short

Passed on the current source: Windows Python 3.14.6: 88 tests and 285 subtests, zero skips, 20.34 seconds. Linux Python 3.10.20: 86 tests and 280 subtests, with two Windows-only batch-test skips, 21.12 seconds. The new native child-launcher case executed on both platforms; Windows also executed its batch variant.

The new regression invokes a real isolated parent and real Python CLI children with an untrusted shadow module on PYTHONPATH. Before the fix, both launcher cases failed; the shadow module's sentinel proved that contributor-controlled code executed. Afterward, the children return valid JSON without importing the shadow module, omit Python configuration variables, and preserve ordinary environment values. Tests use no real credentials or cloud operations.

python -B -m pytest tools/ci/tests/test_collect_ci_evidence.py::CiEvidenceTests::test_streaming_response_stops_at_the_original_deadline tools/ci/tests/test_get_pr_readiness.py::PrReadinessTests::test_missing_azure_aggregate_exposes_enforced_policy_source tools/ci/tests/test_get_pr_readiness.py::PrReadinessTests::test_classic_checks_only_require_present_successful_checks -v -rs --tb=short

Passed before the later child-environment fix: Windows Python 3.14.6: 3 tests and 10 subtests, zero skips, 9.61 seconds. Linux Python 3.10.20: the streaming test and its 3 subtests passed; the two readiness tests skipped because that local PATH lacked PowerShell. Windows executed those tests, and no readiness implementation changed afterward. The current full collector run above also covers the streaming case. CI fails rather than skips if PowerShell is missing under TF_BUILD.

The real socket-pair/urllib test checks completion, expiry between reads, and a stalled response with 0.2 seconds remaining against a two-second socket timeout; it requires timeout before 1.5 seconds and resource cleanup. Readiness tests execute the actual PowerShell entrypoint and cover ruleset/classic policy origins and missing/success/skipped/neutral classic checks-only outcomes. Four isolated mutations failed the intended behavioral assertions, rather than test setup. Runtime source was not modified for these regressions.

python -B -m pytest tools/ci/tests/test_collect_ci_evidence.py tools/ci/tests/test_get_pr_readiness.py tools/ci/tests/test_trusted_guidance.py tools/ci/tests/test_watch_azure_pipeline.py -v -rs --tb=short

Historical validation before the later follow-ups: Windows: 145 tests and 432 subtests, with two POSIX-only skips covered separately on Linux. This is not claimed as a full run of the current patch; the current collector/watcher run is reported above. Earlier cross-platform and before-fix results remain in the archived evidence.

black --check --required-version 22.3.0 .github/skills/synapseml-pr-loop/scripts/collect_ci_evidence.py tools/ci/tests/test_collect_ci_evidence.py
git diff --cached --check

Both passed. The complete changed-path set and every normalized source mode/blob were verified against the frozen manifest before publication. No local Spark build or Fabric E2E run was run for these tooling-only changes.

Review: at the maintainer's request, attempt 78 used one Low harness review (gpt-6-astra, high reasoning), not further six-lens passes. It passed with zero findings, valid transport, exact full input, matching runtime identity and persisted evidence. Earlier malformed-transport attempts remain failed and are preserved. No accepted unresolved finding remains in the local review.

The current-head Copilot review reports no open issues. CodeQL alert 86 was confirmed as a false positive: its SARIF classified a TemporaryDirectory() fixture path as a secret written into the synthetic batch launcher; no credential is involved. The original thread records the data-flow evidence and disposition. Python and JavaScript analysis jobs succeeded. The optional CodeQL aggregate remains neutral because a default API-upload configuration on master is missing from the PR; the identical warning exists on predecessor 0487209b131885d5b06b7036300acdc4ce37b09d. No security rule or workflow was disabled, and this warning is not represented as a fully green scan.

CI status: Azure build 241748094 is running for head 57f22136d7eacfc2e0d20d6722e3aae35938b87f. Its merge 5cbf7445453c3f2c0c42a7660bd33ef95d9de514 has ordered parents d34f762a785ee8a7c7c624806f8c112256168833 and that head; definition 17563, repository, PR merge ref and pullRequest reason were verified. This is not yet a passing result. The final CI-qualified lightweight review and human approval remain outstanding. Prior build 241745526 belongs to predecessor bb032091980d44fb7042f84571af5ebf40907b92 and cannot qualify the child-environment fix.

Build 241706733 succeeded for predecessor 0487209b131885d5b06b7036300acdc4ce37b09d, with 65 successful jobs. It does not qualify this follow-up.

Does this PR change any dependencies?

• No. You can skip this section.
• Yes. Make sure the dependencies are resolved correctly, and list changes here.

Does this PR add a new feature? If so, have you added samples on website?

• No. You can skip this section.
• Yes. Make sure you have added samples following below steps.

1. Find the corresponding markdown file for your new feature in website/docs/documentation folder.
   Make sure you choose the correct class estimators/transformers and namespace.
2. Follow the pattern in markdown file and add another section for your new API, including pyspark, scala (and .NET potentially) samples.
3. Make sure the DocTable points to correct API link.
4. Navigate to website folder, and run npm start to make sure the website renders correctly.
5. Don't forget to add <!--pytest-codeblocks:cont--> before each python code blocks to enable auto-tests for python samples.
6. Make sure the WebsiteSamplesTests job pass in the pipeline.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Hey Rana Singh (@ranadeepsingh) 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

• fix: Fix LightGBM crashes with empty partitions
• feat: Make HTTP on Spark back-offs configurable
• docs: Update Spark Serving usage
• build: Add codecov support
• perf: improve LightGBM memory usage
• refactor: make python code generation rely on classes
• style: Remove nulls from CNTKModel
• test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

[Copilot]

🟡 Changes recommended

Bootstrap import isolation and unresolved retry and warning-handling defects undermine reliable validation.

4 open issues

• Use isolated Python startup to prevent pre-verification imports · New
• Require current retry results per job and suite · New
• Keep gates unresolved until Azure warnings are dispositioned · New
• Apply completeness checks only to current attempts · New

What changed in this PR

Strengthens SynapseML’s PR validation tooling without changing product code or pipeline definitions.

Changes:

• Separates revision, workflow, check, and human-approval evidence.
• Adds Azure job-attempt and named-test collection.
• Verifies trusted guidance and clarifies validation-only permissions.

File	Description
tools/​ci/​tests/​test_watch_azure_pipeline.py	Tests timeline monitoring and retry handling.
tools/​ci/​tests/​test_trusted_guidance.py	Tests guidance integrity checks.
tools/​ci/​tests/​test_get_pr_readiness.py	Tests readiness and trigger safeguards.
tools/​ci/​tests/​test_collect_ci_evidence.py	Tests build and test-result evidence.
.github/​skills/​synapseml-pr-loop/​SKILL.md	Integrates shared validation helpers.
.github/​skills/​synapseml-pr-loop/​scripts/​watch_azure_pipeline.py	Adds Azure timeline diagnostics.
.github/​skills/​synapseml-pr-loop/​scripts/​verify_trusted_guidance.py	Verifies pinned guidance snapshots.
.github/​skills/​synapseml-pr-loop/​scripts/​Get-PrReadiness.ps1	Separates readiness gates and checks revisions.
.github/​skills/​synapseml-pr-loop/​scripts/​collect_ci_evidence.py	Collects merge, job, and test evidence.
.github/​skills/​synapseml-pr-loop/​references/​trusted-guidance.md	Documents trusted bootstrap procedures.
.github/​skills/​synapseml-pr-loop/​references/​review-gauntlet.md	Extracts existing review policy.
.github/​skills/​synapseml-pr-loop/​references/​readiness-gates.md	Clarifies independent readiness gates.
.github/​skills/​synapseml-pr-loop/​references/​loop-control.md	Adds retry and evidence scenarios.
.github/​skills/​synapseml-pr-loop/​references/​ci-triage.md	Documents timeline monitoring and bounded retries.
.github/​skills/​synapseml-pr-loop/​references/​ci-evidence.md	Defines evidence collection and interpretation.
.github/​skills/​synapseml-external-contributor-review/​SKILL.md	Clarifies validation without maintainer edits.
.github/​skills/​synapseml-external-contributor-review/​references/​ci-delegation.md	Connects delegated validation to shared evidence.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Workflow supersession can hide an unresolved approval gate when different PRs share the same head commit.

5 open issues

• Scope workflow runs to the requested PR before selecting newest · New
• Require current retry results per job and suite
• Use isolated Python startup to prevent pre-verification imports
• Apply completeness checks only to current attempts
• Keep gates unresolved until Azure warnings are dispositioned

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Security-sensitive validation changes still need human review, with an unresolved verifier defect and current-head CI unverified.

1 open issue

• Reject non-regular guidance files before reading to prevent hangs · New

5 resolved since last review

• Scope workflow runs to the requested PR before selecting newest
• Require current retry results per job and suite
• Use isolated Python startup to prevent pre-verification imports
• Apply completeness checks only to current attempts
• Keep gates unresolved until Azure warnings are dispositioned

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

The evidence collector can report successful validation after its original collection deadline expires.

2 open issues

• Reject non-regular guidance files before reading to prevent hangs
• Reject validation completed at or after the deadline · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Permission-sensitive validation tooling needs final human review, and authorized current-head CI clearance remains outstanding.

No open issues 🎉

2 resolved since last review

• Reject non-regular guidance files before reading to prevent hangs
• Reject validation completed at or after the deadline

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Streaming HTTP responses can exceed the original collector and watcher deadlines without interruption.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Enforce absolute timeout during response body reads

.github/​skills/​synapseml-pr-loop/​scripts/​collect_ci_evidence.py:173

The urllib timeout limits socket inactivity, not total response time. If bytes keep arriving within that timeout, response.read() can continue past the original deadline. The final deadline check prevents a late success but cannot interrupt this read, so both the collector and timeline watcher can exceed their waiting limits. A standard-library socket-pair check reproduced this: a 150 ms timeout allowed a trickling response to finish after 481 ms. Enforce the absolute deadline during response consumption, with cleanup on expiry, and add a slow-stream regression.

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Credential-sensitive validation tooling still lacks current-head full CI clearance and the final CI-qualified review.

No open issues 🎉

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

Addressed the suppressed streaming-response deadline finding in
2d4bf69.
Each raw response receive now uses the original remaining budget, and cleanup
closes the underlying socket reader. Deadline expiry and shorter query timeouts
also have distinct sanitized diagnostics.

The streaming regression fails before the fix and covers normal completion,
interruption before body completion, socket closure and sender cleanup. The
current collector/watcher suites pass on Windows (87 tests, 282 subtests) and
Linux Python 3.10 (85 tests, 278 subtests; two Windows-only skips). A verified
real-TLS probe independently confirms interruption and cleanup.

Build 241706733
is now validating the published head 0487209b131885d5b06b7036300acdc4ce37b09d;
its ordered merge parents match that head and the current target. The latest
snapshot has zero unresolved inline threads. Original review findings,
dispositions and the exact source manifest are retained in the
review index.
CI is still running; this is not a merge-readiness or approval claim.

[Copilot]

🔵 Needs a closer look

Security-sensitive validation tooling still needs confirmed current-head Azure evidence, final CI-qualified review, and human approval.

No open issues 🎉

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Child-process environment isolation remains unresolved, and current-revision CI evidence and final human review are still outstanding.

1 open issue

• Sanitize inherited PYTHONPATH before launching credential-fetching child proces… · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Supplied evidence leaves current-head CI and final review outstanding, and the trust and authorization changes warrant human review.

1 open issue

• Sanitize inherited PYTHONPATH before launching credential-fetching child proces…

_{Review effort: Balanced}

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Credential-handling and authorization-sensitive tooling requires confirmed current-head CI evidence and final human review.

No open issues 🎉

1 resolved since last review

• Sanitize inherited PYTHONPATH before launching credential-fetching child proces…

_{Review effort: Balanced}