COR-173 - Add IMAP command interruption - #103
Closed
dbezverkhnii wants to merge 3 commits into
Closed
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated Windows download script uses a PowerShell flag (Invoke-WebRequest -UseBasicParsing) that can fail under pwsh/PowerShell 6+ and should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a new IMAP “interrupt current command” capability (to abort a blocked socket read and unblock queued teardown work) and wires it through the C/C++ core, C wrapper, and Swift bindings, alongside a revamped Windows prebuilt distribution flow keyed by a source digest and a CI gate that verifies the corresponding prebuilt archive exists.
Changes:
- Add
interrupt()/interruptCurrentCommand()plumbing acrossOperationQueue, IMAP async connection/operation, the C API, and Swift (MCOIMAPBaseOperation). - Add a Darwin-only Swift unit test (
IMAPInterruptCurrentCommandTests) that simulates a hung TCP read and validates interruption behavior. - Introduce digest-based Windows prebuilt scripts (
Prebuilt-Common.ps1, publish/check scripts), update documentation, and add a PR workflow job to verify the Windows prebuilt is published.
File summaries
| File | Description |
|---|---|
| windows-build-pins.json | Adds pinned toolchain/dependency revisions that participate in the Windows source digest. |
| unittest/IMAPInterruptCurrentCommandTests.swift | New Darwin-only test exercising interruption of a hung socket read. |
| src/swift/imap/IMAPBaseOperation.swift | Exposes interruptCurrentCommand() on the Swift IMAP base operation wrapper. |
| src/include/MailCore/MCOperationQueue.h | Public API: adds interruptRunningOperation() and a running-op pointer. |
| src/include/MailCore/MCOperation.h | Public API: adds default interrupt() hook for operations. |
| src/include/MailCore/MCIMAPSession.h | Public API: adds IMAPSession::interruptCurrentCommand(). |
| src/include/MailCore/MCIMAPOperation.h | Public API: adds IMAP operation interruption APIs (interrupt*). |
| src/include/MailCore/MCIMAPAsyncConnection.h | Public API: adds async-connection interruption entry point. |
| src/include/MailCore/CIMAPBaseOperation.h | C API: exposes interruptCurrentCommand() for Swift/C callers. |
| src/core/imap/MCIMAPSession.h | Core header mirrors new session interruption API. |
| src/core/imap/MCIMAPSession.cpp | Implements stream cancellation under the session lock. |
| src/core/basetypes/MCOperationQueue.h | Core header mirrors queue interruption API/state. |
| src/core/basetypes/MCOperationQueue.cpp | Tracks the currently-running operation and implements interruptRunningOperation(). |
| src/core/basetypes/MCOperation.h | Core header mirrors new Operation::interrupt() virtual. |
| src/core/basetypes/MCOperation.cpp | Adds default no-op Operation::interrupt() implementation. |
| src/c/imap/CIMAPBaseOperation.h | C wrapper header mirrors new exported interrupt function. |
| src/c/imap/CIMAPBaseOperation.cpp | Implements the C wrapper scalar call-through for interruption. |
| src/async/imap/MCIMAPOperation.h | Async IMAP operation: adds interruption APIs. |
| src/async/imap/MCIMAPOperation.cpp | Implements interrupt forwarding to the owning async connection/session. |
| src/async/imap/MCIMAPAsyncConnection.h | Async connection header mirrors the new interruption entry point. |
| src/async/imap/MCIMAPAsyncConnection.cpp | Implements interruption by invoking the queue’s “interrupt running op” helper. |
| README.md | Updates Windows distribution docs to digest-based release assets and new scripts. |
| Package.swift | Adds the new interruption test file to the SwiftPM unittest target sources. |
| build-windows-5.10/Publish-Mailcore2Prebuilt.ps1 | New one-command Windows prebuilt build+verify+upload script. |
| build-windows-5.10/Prebuilt-Common.ps1 | New shared helpers for digest computation and release asset discovery. |
| build-windows-5.10/Get-Mailcore2.ps1 | Switches Windows prebuilt download from S3/versioning to digest-based release assets. |
| build-windows-5.10/Check-PrebuiltPublished.ps1 | New CI-friendly script to check whether the digest-named prebuilt exists. |
| build-windows-5.10/Build-Mailcore2.ps1 | Moves dependency/toolchain handling toward pins + release-provided deps archive. |
| build-windows-5.10/Build-Helpers.ps1 | Adds minimal stand-ins for internal RD build modules used by Windows scripts. |
| AGENTS.md | Documents the Windows prebuilt publishing workflow and constraints for agents. |
| .gitignore | Ignores SwiftPM local state (Package.resolved, .swiftpm/). |
| .github/workflows/pull-request-check.yml | Adds a “Windows prebuilt” job that verifies the prebuilt is published for the merge result. |
Review details
- Files reviewed: 31/32 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
interruptRunningOperation was documented as "the check and the call happen under the queue's lock, so the operation cannot finish in between". It can: main() runs without the lock, so it may return while the caller is still waiting for it. The clause in MCOperationQueue.cpp said the same thing. What the lock does guarantee is the half that matters for the method's purpose - no other operation can become the running one between the identity check and interrupt(), so the interrupt cannot land on whatever started next. The residual race costs a harmless interrupt on an idle stream, one reconnect, and no rearrangement of these two functions closes it. Comments only; the code is byte-identical with comments stripped. Reported by the Copilot reviewer on PR #103, once per copy of the header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7215260. Configure here.
| $cmakeBin = "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin" | ||
| $ninjaBin = "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja" | ||
| $env:Path = "$swiftBin;$msvcBin;$windowsSdkBin;$cmakeBin;$ninjaBin;$env:Path" | ||
| } |
There was a problem hiding this comment.
Pinned Swift toolchain not actually used
Medium Severity
Initialize-Toolchain puts the first clang-cl.exe under the Swift root on PATH, Invoke-VsDevCmd always loads MSVC 14.39, and the install step picks dispatch.dll via Get-Command. None of these consult the versions in windows-build-pins.json or SDKROOT, so a machine with more than one Swift or toolset can compile and package the wrong toolchain while the archive is still named after the pins.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 7215260. Configure here.
dbezverkhnii
deleted the
feature/COR-173-imap-command-interruption-support
branch
September 1, 2026 16:56
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.
Note
Medium Risk
IMAP interrupt touches async queue locking and live IMAP streams (teardown-only by design, but misuse could drop connections); Windows pipeline changes are broad but mostly release tooling—missed prebuilt upload or script/content mismatch could block Spark Windows builds until publish.
Overview
Adds
interruptCurrentCommand()on IMAP operations (C++, C, Swift) so a caller can abort the currently running command on that connection viamailstream_cancel, unblocking the queue for disconnect/teardown instead of waiting on socket timeout. The operation queue tracks the running operation and only interrupts when the caller’s operation matches; queued or finished operations are no-ops. Darwin SPM tests cover blocked-connect and queued-vs-running behavior.Separately, Windows C/C++ delivery moves from versioned S3 (
mailcore2-all-N) to content-addressed GitHub release assets (mailcore2-windows-<digest>.zip), withwindows-build-pins.json, shared PowerShell helpers (Prebuilt-Common.ps1, publish/check scripts), optional public dependency archive, RD-module fallbacks inBuild-Helpers.ps1, and a Linux PR job that verifies the merge-result digest has a published prebuilt. README and AGENTS.md document the manual publish flow.Merge note: C/C++ changes change the Windows digest — someone must run
Publish-Mailcore2Prebuilt.ps1(and may need-Forceif install-script changes altered archive contents without a digest change).Reviewed by Cursor Bugbot for commit 7215260. Bugbot is set up for automated code reviews on this repo. Configure here.