Add signing operation coordination infrastructure - #1066
Open
dtivel wants to merge 1 commit into
Open
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: be91288d-5106-4c59-a1fa-c5353dccb828
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It adds substantial new concurrency coordination and snapshot-lifetime logic that warrants careful human review despite strong unit test coverage.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
test/Sign.Core.Test/TestInfrastructure/TestDirectory.cs — CreateFile takes a parameter named relativePath but currently does not enforce that it is actually… |
 |
src/Sign.Core/SigningOperationCoordinator.cs — The artifact-unavailable detection mixes || and && without parentheses, which is easy to misread… |
What changed in this PR
This PR introduces new internal coordination infrastructure in Sign.Core to canonicalize signing-source identities and to deduplicate concurrent signing operations by sharing a single owner operation/result (plus snapshot-based materialization). It also adds focused unit tests to validate identity semantics, concurrency behavior, snapshot materialization, and disposal/cleanup.
Changes:
- Added
SigningSourceIdentityto provide canonical identities for physical paths and recursively nested container entries. - Added
SigningOperationCoordinator+SigningOperationResultto coordinate same-identity operations and materialize immutable snapshot outputs safely. - Added new unit-test coverage (including concurrency and disposal scenarios) and a small temp-directory test helper.
| File | Description |
|---|---|
| test/Sign.Core.Test/TestInfrastructure/TestDirectory.cs | Adds a disposable temp directory helper for coordinator/identity tests. |
| test/Sign.Core.Test/SigningSourceIdentityTests.cs | Adds unit tests for physical path canonicalization and container entry normalization. |
| test/Sign.Core.Test/SigningOperationCoordinatorTests.cs | Adds extensive coverage for coordination, shared failures, self-await prevention, snapshot materialization, and cleanup. |
| src/Sign.Core/SigningSourceIdentity.cs | Implements canonical, comparable identities for physical files and nested container entries. |
| src/Sign.Core/SigningOperationResult.cs | Implements snapshot-based materialization plus snapshot lifetime management and cleanup behavior. |
| src/Sign.Core/SigningOperationCoordinator.cs | Implements invocation-scoped deduplication/coordination for same-identity signing operations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+25
to
+27
| string path = Path.Combine(FullPath, relativePath); | ||
| Directory.CreateDirectory(Path.GetDirectoryName(path)!); | ||
| File.WriteAllText(path: path, contents: contents); |
Comment on lines
+82
to
+89
| Exception primaryException = | ||
| exception is FileNotFoundException || | ||
| exception is DirectoryNotFoundException && | ||
| !File.Exists(artifact.FullName) | ||
| ? new InvalidOperationException( | ||
| message: "The signed artifact is unavailable.", | ||
| innerException: exception) | ||
| : exception; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behavior
This infrastructure remains dormant. It is not registered with dependency injection or integrated with dispatch, signing formats, containers, or CLI behavior. Activation is deferred to a later PR in the ClickOnce implementation sequence.
Testing
dotnet build .\sign.sln --no-restoredotnet test .\sign.sln --no-build --no-restore993 tests passed and 1 test was skipped.