Skip to content

Preserve main-thread cleanup admission during shutdown - #100

Merged
ShadowNineX merged 2 commits into
mainfrom
codex/lifecycle-cleanup-20261025
Oct 25, 2026
Merged

ShadowNineX merged 2 commits into
mainfrom
codex/lifecycle-cleanup-20261025

Conversation

@ShadowNineX

@ShadowNineX ShadowNineX commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Plugin shutdown and failed-enable rollback need a bounded way to finish owned cleanup after normal admission closes. Capture the main-thread lifecycle context, permit inline dispatch within that scope, and expose the failed-enable rollback transition consumed by Client hosting. Reject rollback initiated inside nested dispatch work before changing shutdown state, preventing the caller from waiting for its own admission lease.

Scope and ownership

SDK owns the host admission and main-thread dispatch boundary. This adds protected CheatEnginePlugin.BeginFailedEnableRollback() and updates the unshipped API inventory. No stable API is removed.

Validation

Check Evidence Result
Lifecycle regression tests dotnet test --project externals/CheatEngine.SDK/tests/CheatEngine.SDK.Hosting.Tests -c Release --no-restore --filter-class '*AdmissionLifecycleTests' --filter-class '*ReentrancyTests' --fail-skips on, with Lua 5.3 Passed: 16 in Debug and 16 in Release; failed: 0; skipped: 0
Integrated MCP build Release and Debug solution builds against SDK and Client source Passed, zero warnings/errors
Qualification level Managed and Lua fixture checks C1/C2 only
Live Cheat Engine host / packed package Not run in this cleanup pass Not executed
Repository policies Debug repository suite, including public API inventory ordering Passed: 202; failed: 0; skipped: 0
Formatting Repository folder whitespace verification Passed
CI GitHub checks on this branch Pending on latest signed head

Compatibility and release impact

Client's companion change consumes the new protected SDK method. Publish a compatible SDK package before releasing the paired Client package. Consumer-visible changes are recorded under Unreleased. This change preserves the tested source integration; it does not claim standalone package or native host qualification.

Checklist

  • Public API additions recorded in PublicAPI.Unshipped.txt.
  • Lifecycle regressions and affected README updated.
  • No skipped lifecycle tests.
  • Consumer-visible changelog entry.
  • Package release validation.
  • Full SDK CI and package qualification.

Summary

  • MainThread.Invoke now admits inline work on the captured main thread during OnEnable and OnDisable. Worker dispatch remains closed during those callbacks.
  • Added protected CheatEnginePlugin.BeginFailedEnableRollback() for synchronous cleanup after OnEnable failure. The host closes general admission and signals shutdown; activation cannot complete if rollback was signaled.
  • Updated the unshipped API inventory. No stable API removal is reported.

Tests

  • Added lifecycle admission tests for enable, disable, and failed-enable rollback.
  • The author reports 9 focused tests passed with no failures or skips. Full SDK CI and live-host qualification remain incomplete.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
Walkthrough

The changes allow inline MainThread.Invoke calls on the lifecycle thread during OnEnable and OnDisable, while worker dispatch remains gated. CheatEnginePlugin.BeginFailedEnableRollback() signals shutdown and closes general work admission during OnEnable rollback. If OnEnable returns after signaling rollback, enable completion fails. New tests cover lifecycle admission and rollback behavior.

Priority: Normal

Merge Risk: Moderate · up to 1e45f

Nested cleanup can hang the Cheat Engine GUI during a failed enable. Reject rollback from inline work before merging.

Pre-merge checks: Passed 9 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Public Api Documentation And Changelog Warning The PR adds the protected public CheatEnginePlugin.BeginFailedEnableRollback() API with XML documentation and updates libs/CheatEngine.SDK.Hosting/README.md for the changed lifecycle admission con… Add a CHANGELOG.md entry under [Unreleased] that documents BeginFailedEnableRollback() and the changed inline MainThread.Invoke admission during OnEnable and OnDisable. Mark the behavior change as Breaking as required by thi…
Passed Passed checks: 9 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Preserve main-thread cleanup admission during shutdown" is a short imperative sentence of 54 characters with no trailing period, no Conventional Commit or area prefix, and an uppercase firs…
Native Abi Evidence Passed PASS: The PR changes only hosting lifecycle, threading, documentation, and lifecycle test files. No file under libs/CheatEngine.SDK.Abi/, native/, or tests/native-abi-fixture/ changed. The revie…
Lua Stack Balance Tests Passed PASS. The pull request changes only Hosting, documentation, and Hosting test files. It does not change code under libs/CheatEngine.SDK.Lua/, libs/CheatEngine.SDK.Lua.Interop/, source-generators/CheatE…
Lua Concurrency Contract Passed PASS: The pull request does not change any file under libs/CheatEngine.SDK.Lua/Runtime/**. The custom check is therefore not applicable.
Native Host Emulator Evidence Passed PASS: The pull request does not change tests/native-host-emulator/** or tests/CheatEngine.SDK.Hosting.Tests/Coexistence/**. The changed test file is under `tests/CheatEngine.SDK.Hosting.Tests/Life…
Dependency Direction Passed No dependency-direction violation is introduced. The reviewed diff adds only SDK hosting lifecycle admission and rollback behavior in libs/CheatEngine.SDK.Hosting; it adds no CheatEngine.Client or…
Workflow Hygiene Passed The pull request changes no files under .github/. The workflow hygiene conditions therefore do not apply.
Public Api And Compatibility Passed PASS. The pull request adds one protected API, CheatEnginePlugin.BeginFailedEnableRollback(), in libs/CheatEngine.SDK.Hosting/Plugin/CheatEnginePlugin.cs and records it in the owning library's `Pu…
Full details: Title check

Explanation

The title "Preserve main-thread cleanup admission during shutdown" is a short imperative sentence of 54 characters with no trailing period, no Conventional Commit or area prefix, and an uppercase first letter, so it meets the title requirements. It matches the main change: the PR keeps main-thread inline dispatch open during OnEnable/OnDisable and failed-enable rollback so owned cleanup can finish during shutdown, as the changed PluginHost, MainThread, and CheatEnginePlugin files show.

Full details: Native Abi Evidence

Explanation

PASS: The PR changes only hosting lifecycle, threading, documentation, and lifecycle test files. No file under libs/CheatEngine.SDK.Abi/, native/, or tests/native-abi-fixture/ changed. The reviewed diff also adds or changes no StructLayout, FieldOffset, function-pointer signature, UnmanagedCallersOnly, or LibraryImport declaration. The native ABI evidence requirement is therefore not applicable.

Full details: Lua Stack Balance Tests

Explanation

PASS. The pull request changes only Hosting, documentation, and Hosting test files. It does not change code under libs/CheatEngine.SDK.Lua/, libs/CheatEngine.SDK.Lua.Interop/, source-generators/CheatEngine.SDK.SourceGenerators.LuaBindings/, or native/cheatengine-sdk-lua-bridge/. The Lua stack balance criteria therefore do not apply.

Full details: Native Host Emulator Evidence

Explanation

PASS: The pull request does not change tests/native-host-emulator/** or tests/CheatEngine.SDK.Hosting.Tests/Coexistence/**. The changed test file is under tests/CheatEngine.SDK.Hosting.Tests/Lifecycle/, so this check is not applicable.

Full details: Dependency Direction

Explanation

No dependency-direction violation is introduced. The reviewed diff adds only SDK hosting lifecycle admission and rollback behavior in libs/CheatEngine.SDK.Hosting; it adds no CheatEngine.Client or CheatEngine.Mcp reference, namespace, or package reference. No project file changed, and no new PackageReference was added under src/ or libs/. The added behavior is SDK lifecycle policy, not Client fluent API, dependency-injection registration, or application workflow policy.

Full details: Public Api And Compatibility

Explanation

PASS. The pull request adds one protected API, CheatEnginePlugin.BeginFailedEnableRollback(), in libs/CheatEngine.SDK.Hosting/Plugin/CheatEnginePlugin.cs and records it in the owning library's PublicAPI.Unshipped.txt. The diff does not change PublicAPI.Shipped.txt, CompatibilitySuppressions.xml, or any packages.lock.json file.

Full details: Public Api Documentation And Changelog

Explanation

The PR adds the protected public CheatEnginePlugin.BeginFailedEnableRollback() API with XML documentation and updates libs/CheatEngine.SDK.Hosting/README.md for the changed lifecycle admission contract. However, the PR does not modify CHANGELOG.md, and [Unreleased] remains empty. The PR therefore lacks the required matching changelog entry, including a breaking marker for the lifecycle behavior change.

Resolution

Add a CHANGELOG.md entry under [Unreleased] that documents BeginFailedEnableRollback() and the changed inline MainThread.Invoke admission during OnEnable and OnDisable. Mark the behavior change as Breaking as required by this check.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 7c5b681 and 1e45f32.

Files selected for processing: 8
  • libs/CheatEngine.SDK.Hosting/Bootstrap/PluginHost.Lifecycle.cs
  • libs/CheatEngine.SDK.Hosting/Bootstrap/PluginHost.cs
  • libs/CheatEngine.SDK.Hosting/Plugin/CheatEnginePlugin.cs
  • libs/CheatEngine.SDK.Hosting/PublicAPI.Unshipped.txt
  • libs/CheatEngine.SDK.Hosting/README.md
  • libs/CheatEngine.SDK.Hosting/Threading/MainThread.cs
  • tests/CheatEngine.SDK.Hosting.Tests/Lifecycle/AdmissionLifecycleTests.cs
  • tests/CheatEngine.SDK.Hosting.Tests/README.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread libs/CheatEngine.SDK.Hosting/Bootstrap/PluginHost.cs Outdated
@sonarqubecloud

Copy link
Copy Markdown

@ShadowNineX
ShadowNineX merged commit 524979c into main Oct 25, 2026
20 checks passed
@ShadowNineX
ShadowNineX deleted the codex/lifecycle-cleanup-20261025 branch October 25, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant