SDK: finalize runtime ownership and scan contracts - #83
Merged
Merged
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe pull request updates ownership and cleanup behavior for symbol registration, callbacks, Auto Assembler patches, and memory scans. Memory scans gain target qualification outcomes, release outcomes, bounded result paging, and cancellation support. Address-resolution compatibility members are removed. Native bridge parity and packaged Native AOT consumer tests are expanded. Several files also receive formatting, typing, analyzer-suppression, and test-infrastructure updates. Priority: ➖ Normal Merge Risk: 🔵 Low · up to The current Client pin remains compatible, but its test must be migrated when it adopts this SDK. Document the new Adopt failure mode so SDK consumers can handle target-qualification failures correctly. Comment |
AriusII
force-pushed
the
sdk/functional-adjustments
branch
3 times, most recently
from
September 22, 2026 13:37
934a576 to
b349713
Compare
AriusII
force-pushed
the
sdk/reformat-and-ci
branch
from
September 22, 2026 13:44
2f58c67 to
2d3988e
Compare
AriusII
force-pushed
the
sdk/functional-adjustments
branch
from
September 22, 2026 13:46
b349713 to
de663bf
Compare
AriusII
force-pushed
the
sdk/reformat-and-ci
branch
from
September 22, 2026 13:56
2d3988e to
77f7ed9
Compare
AriusII
force-pushed
the
sdk/functional-adjustments
branch
from
September 22, 2026 13:57
de663bf to
44ccbd4
Compare
AriusII
force-pushed
the
sdk/reformat-and-ci
branch
from
September 22, 2026 14:01
77f7ed9 to
1240598
Compare
AriusII
force-pushed
the
sdk/functional-adjustments
branch
from
September 22, 2026 14:01
44ccbd4 to
7bdbc0c
Compare
AriusII
force-pushed
the
sdk/reformat-and-ci
branch
from
September 22, 2026 14:12
1240598 to
912e826
Compare
AriusII
force-pushed
the
sdk/functional-adjustments
branch
from
September 22, 2026 14:13
7bdbc0c to
0b1782e
Compare
AriusII
force-pushed
the
sdk/reformat-and-ci
branch
from
September 22, 2026 14:31
912e826 to
de5f204
Compare
Carry the prepared runtime, inspection, memory-scan, symbol-registration, Lua, native bridge, analyzer, generator, and packaging/test changes as one functional batch. The changes close ownership hand-offs, expose explicit scan outcomes, harden lifecycle and bridge behavior, and keep the managed tests aligned with the generated/native contracts.
AriusII
force-pushed
the
sdk/functional-adjustments
branch
from
September 22, 2026 14:39
0b1782e to
4fbf51d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: CheatEngineNet/CheatEngine.SDK/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 608119e4-fba8-436e-820a-4c7812150ec3
⛔ Files ignored due to path filters (1)
native/cheatengine-sdk-lua-bridge/runtimes/win-x64/native/cheatengine-sdk-lua-bridge.dllis excluded by!**/*.dll,!**/*.dll
📒 Files selected for processing (44)
CHANGELOG.mdanalyzers/CheatEngine.SDK.Analyzers/Diagnostics/DiagnosticDescriptors.cslibs/CheatEngine.SDK.Abi/Native/ClassicDebugEventDispatcher.cslibs/CheatEngine.SDK.Engine/AddressList/AddressListMutations.cslibs/CheatEngine.SDK.Engine/Assembly/AutoAssemblerPatcher.cslibs/CheatEngine.SDK.Engine/Inspection/AddressResolutionOptions.cslibs/CheatEngine.SDK.Engine/Inspection/EngineInspection.cslibs/CheatEngine.SDK.Engine/Inspection/SymbolRegistrationHandoffException.cslibs/CheatEngine.SDK.Engine/Inspection/SymbolRegistrationLeaseFactory.cslibs/CheatEngine.SDK.Engine/Inspection/SymbolRegistrationLeasePublisher.cslibs/CheatEngine.SDK.Engine/Inspection/SymbolRegistry.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanCreationOutcome.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanCreationStatus.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanMaterializationStatus.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanReleaseOutcome.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanSession.cslibs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanSessions.cslibs/CheatEngine.SDK.Lua/Callbacks/LuaCallback.cslibs/CheatEngine.SDK.Lua/Runtime/LuaRuntime.cslibs/CheatEngine.SDK.Lua/State/LuaState.Callbacks.csnative/cheatengine-sdk-lua-bridge/cheatengine_sdk_lua_bridge.csource-generators/CheatEngine.SDK.SourceGenerators.Shared/Shapes/PluginShape.cstests/CheatEngine.SDK.Abi.Tests/Fixture/NativeAbiFixtureManagedComparisonTests.cstests/CheatEngine.SDK.Analyzers.Tests/Architecture/LuaDirectApiBoundaryGuardTests.cstests/CheatEngine.SDK.Benchmarks/CallbackBenchmarks.cstests/CheatEngine.SDK.Engine.Tests/Assembly/InstructionOperationsTests.cstests/CheatEngine.SDK.Engine.Tests/Inspection/EngineInspectionTests.cstests/CheatEngine.SDK.Engine.Tests/Inspection/SymbolRegistryTests.cstests/CheatEngine.SDK.Engine.Tests/Scanning/MemoryScanSessionFactoryTests.cstests/CheatEngine.SDK.Engine.Tests/Scanning/MemoryScanSessionTests.cstests/CheatEngine.SDK.Engine.Tests/Support/FakeHost.cstests/CheatEngine.SDK.LivePlugin.Coexistence/PluginA/CoexistencePluginA.cstests/CheatEngine.SDK.LivePlugin.Coexistence/PluginB/CoexistencePluginB.cstests/CheatEngine.SDK.LivePlugin/CheatEngineSdkLivePlugin.cstests/CheatEngine.SDK.LiveProbe/LiveProbeAuthorization.cstests/CheatEngine.SDK.Lua.Interop.Tests/RoundTrips/CallbackTests.cstests/CheatEngine.SDK.Lua.Tests/Callbacks/LuaCallbackTests.cstests/CheatEngine.SDK.SourceGenerators.LuaBridgeContract.Tests/CheatEngine.SDK.SourceGenerators.LuaBridgeContract.Tests.csprojtests/CheatEngine.SDK.SourceGenerators.LuaBridgeContract.Tests/Generator/NativeBridgeParityTests.cstests/CheatEngine.SDK.SourceGenerators.LuaBridgeContract.Tests/Infrastructure/ProductionNativeBridge.cstests/CheatEngine.SDK.Tests/Infrastructure/PackagedUmbrellaFixture.cstests/CheatEngine.SDK.Tests/Infrastructure/ThrowawayConsumer.cstests/CheatEngine.SDK.Tests/Packaging/NativeBridgePeAuditTests.cstests/CheatEngine.SDK.Tests/Packaging/PackedConsumerBindingTests.cs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
CheatEngineNet/CheatEngine.Client(manual)
💤 Files with no reviewable changes (1)
- libs/CheatEngine.SDK.Engine/Inspection/EngineInspection.cs
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Comment on lines
+11
to
+12
| /// <summary>Creates options for the optional CE <c>shallow</c> argument.</summary> | ||
| public AddressResolutionOptions(bool Shallow = false) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n "AddressResolutionOptions|CheatEngine.SDK" ../CheatEngine.Client 2>/dev/null | head -100Repository: CheatEngineNet/CheatEngine.SDK
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- call site and nearby test ---'
sed -n '55,85p' tests/CheatEngine.Client.Core.Tests/Domains/InspectionClientBehaviorTests.cs
printf '%s\n' '--- package pin ---'
sed -n '1,20p' Directory.Packages.props
printf '%s\n' '--- SDK references and AddressResolutionOptions usages ---'
rg -n -C 2 'CheatEngine\.SDK|AddressResolutionOptions' --glob '*.{cs,props,targets,proj}' .Repository: CheatEngineNet/CheatEngine.Client
Length of output: 41937
Update the linked Client test when it upgrades the SDK.
CheatEngine.Client currently pins CheatEngine.SDK to 1.0.0, so this PR does not break its current dependency selection. When the Client upgrades, new AddressResolutionOptions(true, true) will fail to compile because this SDK exposes only AddressResolutionOptions(bool Shallow = false). Update the test call and record the migration in CHANGELOG.
Comment on lines
+275
to
+280
| MemoryScanSessionContext context = MemoryScanSessionContext.Capture(operation.State); | ||
| if (!context.TargetObservation.IsQualified) | ||
| { | ||
| throw new MemoryScanException(MemoryScanFailureKind.TargetIdentityUnavailable, "MemoryScan.Adopt", | ||
| "The memory scan session cannot adopt target-dependent owners without a qualified target incarnation."); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '245,290p' libs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanSession.cs
find .. -name AGENTS.md -o -name CONTRIBUTING.md -o -name .editorconfig | head -40Repository: CheatEngineNet/CheatEngine.SDK
Length of output: 2562
🏁 Script executed:
printf '%s\n' '--- AGENTS.md ---'
cat -n AGENTS.md
printf '%s\n' '--- CONTRIBUTING.md relevant documentation rules ---'
rg -n -i -C 3 'document|xml|exception|public|api' CONTRIBUTING.md AGENTS.md .editorconfig
printf '%s\n' '--- MemoryScanSession.Adopt and surrounding public members ---'
cat -n libs/CheatEngine.SDK.Engine/Scanning/Values/MemoryScanSession.cs | sed -n '190,310p'
printf '%s\n' '--- MemoryScanException declarations and usages ---'
rg -n -C 5 'class MemoryScanException|record MemoryScanException|MemoryScanException\\(' libs tests
printf '%s\n' '--- TargetObservation qualification and context capture ---'
rg -n -C 6 'TargetObservation|IsQualified|MemoryScanSessionContext' libs/CheatEngine.SDK.Engine/Scanning/ValuesRepository: CheatEngineNet/CheatEngine.SDK
Length of output: 32001
🏁 Script executed:
printf '%s\n' '--- MemoryScanException definitions and public documentation ---'
rg -n -F -C 8 'MemoryScanException' libs/CheatEngine.SDK.Engine libs/CheatEngine.SDK.Annotations tests || true
printf '%s\n' '--- TargetSelectionObservation definition ---'
rg -n -F -C 12 'record struct TargetSelectionObservation' libs || true
rg -n -F -C 8 'struct TargetSelectionObservation' libs || true
printf '%s\n' '--- IsQualified declarations and documentation ---'
rg -n -F -C 8 'IsQualified' libs/CheatEngine.SDK.Engine libs/CheatEngine.SDK.Annotations || trueRepository: CheatEngineNet/CheatEngine.SDK
Length of output: 42386
Document the new MemoryScanException on Adopt.
Adopt can throw MemoryScanException when the current target observation is not qualified. Add the exception to the public XML contract.
📝 Suggested documentation addition
/// <exception cref="ArgumentNullException">Either ownership wrapper is <see langword="null" />.</exception>
/// <exception cref="ObjectDisposedException">Either ownership wrapper was already released or disposed.</exception>
+ /// <exception cref="MemoryScanException">
+ /// The current target observation is not qualified as a target-process incarnation, so target-dependent owners are not adopted.
+ /// </exception>
AriusII
added a commit
that referenced
this pull request
Sep 23, 2026
Add the second required check, "PR policy" (shared-contracts 1.2, 1.12; audit register PR-CQ-34). The title and CHANGELOG rules were advisory (CONTRIBUTING, CodeRabbit warning mode) and only the tag run enforced a CHANGELOG section. pull-request-ci.yml does not run on "edited", so a title fix would need a full Windows run: the check is a separate, cheap Ubuntu workflow that also runs on title and description edits, drafts included, with no path filter and no job condition (a skipped job would satisfy a required check). - eng/ci/PullRequestPolicy.psm1 holds pure rules with stable ids: TitleLength (1-72 text elements), TitleNoTrailingPeriod, TitleNoConventionalPrefix (also rejects "Area:" prefixes), TitleStartsUppercase, TitleImperative (-ed/-ing/third-person heuristic with an allowlist of real verbs) and ChangelogEntry (libs/, src/, analyzers/, source-generators/, native/ changes, lock files excluded, unless CHANGELOG.md changes or the description carries <!-- changelog: not-needed -->). Matching is ordinal, case-sensitive, with regex timeouts. Pull requests authored by dependabot[bot] are exempt; the repository-specific values sit in one constants block so the CheatEngine.Client twin changes only those lines. - eng/ci/Test-PullRequestPolicy.ps1 reads the pull request only from environment variables, diffs base...head with --no-renames (moving a file out of libs/ still counts), writes a rule table to the job summary, emits one escaped ::error annotation per failed rule and never prints the description. The repository tests now start pwsh for this: PullRequestPolicyFixture evaluates 35 vectors (real titles of #10, #83 and #86 included) in one process through JSON files, and the entry script is run end to end with the GitHub file-command variables removed from its environment. The project README and comment record this single exception to "never starts a process".
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
1 participant
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.
Context
This dependent branch contains the functional source changes that were already present in the SDK worktree after the formatting/CI baseline. It is intentionally isolated from the mechanical reformatting and from the deletion-only cleanup.
Changes
Review notes
This is the behavior/API batch. It should be reviewed after PR #82 and merged after the deletion and formatting/CI baselines. No public SDK handles are leaked by the new ownership paths; the added tests cover managed/native parity and resource hand-off behavior.
Validation
Integration order
Merge PR #81, then PR #82, then this PR.
Summary
AddressResolutionOptions.UseHostSymbolTableand its compatibility API. Host-symbol resolution now usesEngineInspection.ResolveHostAddress.Validation includes
git diff --check. Release build and test gates run through the pull-request workflow.