Repository navigation
Prototype generated .NET SDK review artifact handoff - #47625
Sameeksha Vaity (samvaity) wants to merge 15 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Next Steps to Merge⌛ Please wait. Next steps to merge this PR are being evaluated by automation. ⌛ Still evaluating
Comment generated by summarize-checks workflow run. |
Fail closed on incomplete capture, validate package-scoped patch contents, bound artifact resources, and distinguish missing artifacts from retrieval failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Type the bounded stream reader explicitly and cover the aggregate changed-file schema limit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Bind replay to the trusted SDK Validation pipeline, resolve artifacts once, and verify complete patch applicability before publishing validated evidence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
JoshLove-msft
left a comment
There was a problem hiding this comment.
Found three issues in the bundle validation path: effective patch paths are not checked, independent snapshots can conflict when concatenated, and multiple empty patches fail applicability validation. Details are inline.
--generated by Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
JoshLove-msft
left a comment
There was a problem hiding this comment.
Reviewed specifically for complexity and unnecessary configuration. The scope reduction helps: manual backfill, SDK recloning/applicability checks, and checksums are gone.
The main remaining design decision is where the .NET review skill runs. This artifact contains generated SDK diffs, not a completed review. Capturing those diffs before resetGitRepo() is necessary. Moving them from Azure Pipelines to GitHub is necessary only if GitHub is the chosen review host; it is not inherently required for SDK review. The original issue proposes GitHub for orchestration and comment permissions, but this PR does not yet invoke the review skill or demonstrate its runtime/context requirements.
If GitHub is the review host, the smaller production path is: SDK Validation captures/publishes once; one GitHub review job downloads and validates the bundle, then runs the skill directly on that local input; a separate write-capable publisher posts structured findings after checking the PR head. No intermediate GitHub copy of the input bundle is required. If the review can instead run in a trusted, isolated Azure Pipelines job, publish its findings and have GitHub only validate identity/freshness and post the comment. Do not put model or PR-write credentials into the PR-controlled generation job.
I would validate the chosen review host with one representative generated package before expanding the transport contract further. The canonical .NET skill expects surrounding SDK project/repository context, not just changed lines, so copying a patch successfully does not yet prove the actual integration. Inline comments identify concrete reductions available now.
--generated by Copilot
|
Clarifying the review-input requirement: we need to publish the generated library source/project files, not just a patch or compiled package. The .NET review skill needs to inspect the resulting library, including unchanged surrounding code and project metadata; a diff alone is not sufficient. The diff is still useful to identify what changed, but should accompany the generated library rather than replace it. Please capture each generated library before the SDK checkout is reset, retain its SDK baseline identity, and make the generated source/project context available to the review runner. Shared SDK repository context can be obtained from the matching baseline; it does not all need to be duplicated in the artifact. The prototype should demonstrate running the actual .NET review skill against this input before settling the artifact contract. --generated by Copilot |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
d0db429 to
169e019
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
JoshLove-msft
left a comment
There was a problem hiding this comment.
The revision now captures generated package source and reconstructs the SDK baseline before invoking the canonical .NET review skill, so it addresses my earlier concern about reviewing only a diff. Reusing the existing generation artifacts and removing unused helper modes also reduces the configuration surface. The GitHub re-upload now has an actual downstream consumer rather than being transport-only proof output.
I found three remaining correctness issues: the ZIP header comparison rejects Azure Pipelines' streamed ZIP format; the re-upload drops hidden package files; and review-only limits can make the existing SDK status/label consumers reject otherwise successful generation artifacts. Details and reproductions are inline.
--generated by Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Jorge Rangel (jorgerangel-msft)
left a comment
There was a problem hiding this comment.
One question about the intended issue-publication behavior and three reproduced correctness issues: sparse-checkout reconstruction, the remaining shared-schema changed-file limit, and disabled .NET generation. Details are inline.
🤖 Generated by Jorge's Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5bac55-2046-4793-9296-579376c32524
Purpose
Prototype the handoff proposed in Azure/azure-sdk-tools#17269 between .NET SDK Validation and a GitHub-hosted review workflow.
Boundaries
• ADO: produce the generated state while it exists.
• GitHub handoff: establish that the state is trusted.
• GitHub Agentic Workflow: reconstruct the real change and perform the review.
• Future publisher: optionally publish validated, non-stale findings to the PR.
Conceptual Diagram
flowchart TD PR["Developer updates spec PR"] subgraph PIPE["1. Azure Pipelines: generate and preserve"] GEN["Generate and build .NET SDK"] CAP["Save package source and metadata<br/>before the next checkout reset"] ADO["Publish existing artifacts<br/>summary JSON + packages"] GEN --> CAP --> ADO end subgraph HAND["2. GitHub Actions: validate review input"] CHECK["Receive completed .NET check"] VALID["Check event/build identity,<br/>current PR head and snapshot contents"] INPUT["Upload validated review input<br/>and write job summary"] CHECK --> VALID --> INPUT end subgraph REVIEW["3. Report-only Copilot workflow: review"] START["Successful handoff run<br/>with review input available"] RESTORE["Reconstruct SDK changes<br/>on each exact SDK baseline"] SKILL["Fetch and pin canonical<br/>.NET code-review skill"] AI["Copilot reviews generated changes<br/>with package and repository context"] OUT["Upload findings reports<br/>JSON + Markdown + review context"] START --> RESTORE --> SKILL --> AI --> OUT end PR --> GEN ADO -. "check_run completed" .-> CHECK INPUT -. "workflow_run completed" .-> START OUT --> END["Developer reads results<br/>No automatic PR comments or changes"] classDef generation fill:#dbeafe,stroke:#2563eb,color:#172554 classDef validation fill:#fef3c7,stroke:#b45309,color:#78350f classDef review fill:#dcfce7,stroke:#15803d,color:#14532d class GEN,CAP,ADO generation class CHECK,VALID,INPUT validation class START,RESTORE,SKILL,AI,OUT reviewOut of scope
This PR invokes the canonical .NET code-review skill in a report-only workflow. Posting PR comments, failing SDK Validation, and adding a merge gate remain out of scope.