Skip to content

Fix(store-wrapper): guard StoreWrapperController.Launch() against unavailable store model (#240) - #241

Merged
drmoisan merged 5 commits into
mainfrom
TaskMaster-wt-2026-07-06-06-35
Jul 6, 2026
Merged

Fix(store-wrapper): guard StoreWrapperController.Launch() against unavailable store model (#240)#241
drmoisan merged 5 commits into
mainfrom
TaskMaster-wt-2026-07-06-06-35

Conversation

@drmoisan

@drmoisan drmoisan commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Fix(store-wrapper): guard StoreWrapperController.Launch() against unavailable store model (#240)

Summary

  • Fixes an unhandled System.NullReferenceException in StoreWrapperController.Launch() that reached the user when the store-settings dialog was opened before async store initialization had populated Globals.Ol.StoresWrapper.
  • Introduces an explicit readiness evaluation (EvaluateLaunchReadiness()) that classifies store state as Ready, ModelUnavailable, or StoresUnavailable, and gates Launch() on it.
  • On a not-ready state, Launch() now fails gracefully with a clear user-facing message and returns without opening a broken dialog, instead of dereferencing a null model.
  • Adds a deterministic MSTest regression suite (Moq-based, no live Outlook, no temporary files) that reproduces the pre-fix crash path and verifies the guarded behavior.
  • Splits the oversized StoreWrapperController_Tests.cs into cohesive partial-class files to satisfy the repository 500-line file limit, preserving every test method and assertion.

Why

Globals.Ol.StoresWrapper is populated asynchronously during add-in startup: ThisAddIn queues _globals.LoadAsync(false) on the IdleAsyncQueue, and AppOlObjects.LoadStoresAsync() fills the wrapper. The ribbon entry point RibbonController.FolderStoresSettings() invoked Launch() with no gating on whether that load had completed or succeeded.

Two null states could therefore reach Launch():

  1. StoresWrapper (Model) is null — the async load has not finished, or the config-missing branch left it null.
  2. StoresWrapper is non-null but its Stores list is transiently null — [OnDeserialized] RewireOlObjects fires-and-forgets the rewire, and Stores ??= [] runs only inside that async path.

Launch() guarded neither, and dereferenced Model.Stores.Select(...), producing the unhandled NullReferenceException reported in issue #240.

What Changed

Core fix (UtilitiesCS/OutlookObjects/Store/StoreWrapperController.cs)

  • Added EvaluateLaunchReadiness(), which reads Globals?.Ol?.StoresWrapper, classifies the readiness state, and — when ready — returns the model plus the store display names needed to populate the dialog.
  • Added the internal StoreLaunchReadinessState enum (Ready, ModelUnavailable, StoresUnavailable) and the internal readonly struct StoreLaunchReadiness result type with NotReady(...) / Ready(...) factories.
  • Rewired Launch() to consult readiness first and short-circuit with a MyBox.ShowDialog(...) warning when not ready, binding Viewer.DisplayName.DataSource from the pre-computed display names only on the ready path.
  • The null sentinel inside NotReady(...) is wrapped in a narrow #pragma warning disable CS8625 / restore because this project has no #nullable annotation context; a ? annotation would emit new CS8632 warnings during normal builds.

Tests (UtilitiesCS.Test/OutlookObjects/Store/)

  • Added StoreWrapperController_Tests.Launch.cs and StoreWrapperController_Tests.ButtonAndPopulate.cs as partial classes; reduced StoreWrapperController_Tests.cs accordingly. This is a mechanical split with no behavior change, driven by the 500-line file-size limit.
  • Added regression tests covering the null-model and null-stores paths (fail-before / pass-after).
  • Updated UtilitiesCS.Test.csproj to include the new partial files.

Documentation / evidence

  • Feature-folder artifacts under docs/features/active/2026-07-06-store-wrapper-launch-npe-240/: research, plan, issue, remediation inputs/plan, QA-gate evidence, regression evidence, and the code/feature/policy audits.

Architecture / How It Fits Together

  • Entry point: ribbon action -> RibbonController.FolderStoresSettings() -> StoreWrapperController.Launch().
  • Launch() now delegates the null-state decision to EvaluateLaunchReadiness(), keeping the readiness policy in one testable method separated from the WinForms dialog wiring.
  • EvaluateLaunchReadiness() is internal (not [ExcludeFromCodeCoverage]), so it is exercised directly by unit tests; Launch() itself remains the thin, host-bound WinForms wiring and is [ExcludeFromCodeCoverage].
  • The readiness result (StoreLaunchReadiness) carries the model and precomputed display names, so the ready path does not re-dereference the model.

Verification

Completed (from context evidence)

  • Formatting (csharpier): pass — evidence/qa-gates/qa-01-format.md.
  • .NET analyzers build: pass, EXIT_CODE 0evidence/qa-gates/qa-02-analyzers.md.
  • Nullable / TreatWarningsAsErrors build: solution-wide EXIT_CODE 1 attributable to pre-existing, unrelated nullable debt in vendored projects; touched files add zero new nullable diagnostics — evidence/qa-gates/qa-03-nullable.md.
  • Tests: 4170/4170 passed — evidence/qa-gates/qa-04-test-coverage.md, evidence/regression-testing/pass-after-240.md.
  • Regression test fails before the fix (EXIT_CODE 1) and passes after — evidence/regression-testing/fail-before-240.md, evidence/regression-testing/pass-after-240.md.
  • Coverage: EvaluateLaunchReadiness 100% line coverage on changed lines; UtilitiesCS.dll at 85.88% with no regression — evidence/qa-gates/qa-05-coverage-delta.md.

Recommended

  • dotnet tool run csharpier .
  • msbuild TaskMaster.sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
  • msbuild TaskMaster.sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:Nullable=enable /p:TreatWarningsAsErrors=true
  • vstest.console.exe UtilitiesCS.Test/bin/Debug/UtilitiesCS.Test.dll /EnableCodeCoverage /InIsolation

Backward Compatibility / Migration Notes

  • No public API changes. The new StoreLaunchReadinessState enum and StoreLaunchReadiness struct are internal; EvaluateLaunchReadiness() is internal.
  • User-visible behavior change: opening store settings before store state is available now shows a warning dialog and returns, instead of crashing with an unhandled exception.
  • Test files were split into partial classes; no test was removed or renamed.

Risks and Mitigations

  • Risk: the warning-dialog path is host-bound (WinForms MyBox.ShowDialog) and not unit-covered. Mitigation: the decision logic lives in the covered EvaluateLaunchReadiness(); Launch() remains thin wiring.
  • Risk: the solution-wide nullable build reports EXIT_CODE 1. Mitigation: this is pre-existing vendored-project debt present in the baseline; the touched files introduce zero new nullable diagnostics (documented in evidence/qa-gates/qa-03-nullable.md).
  • Rollback: revert the three commits in range; the change is localized to one production file plus its tests and feature docs.

Review Guide

Suggested review order:

  1. UtilitiesCS/OutlookObjects/Store/StoreWrapperController.cs — the readiness type, EvaluateLaunchReadiness(), and the guarded Launch().
  2. UtilitiesCS.Test/OutlookObjects/Store/StoreWrapperController_Tests.Launch.cs — the null-model / null-stores regression tests.
  3. evidence/regression-testing/fail-before-240.md and pass-after-240.md — the red-before-green evidence.
  4. The StoreWrapperController_Tests.cs / .ButtonAndPopulate.cs split — mechanical move, no behavior change (verify against evidence/qa-gates/split-*-verification.md).

Follow-ups

  • AC6 (all required PR CI checks green against the PR head SHA) is verified after the PR is opened.
  • A canonical, merged repo-wide C# coverage artifact (artifacts/csharp/coverage.xml across all first-party test projects) does not yet exist in-repo; producing it is a pre-existing, maintainer-owned item tracked under feature/csharp-coverage-uplift, not attributable to this change.

GitHub Auto-close

drmoisan added 5 commits July 6, 2026 07:29
…is unavailable

- Add EvaluateLaunchReadiness() guard to detect null or incomplete store model before Launch() opens dialog
- Handle transient post-deserialize state where Globals.Ol.StoresWrapper is null or its Stores list is null
- Show user-facing message when model is not ready, preventing unhandled exception
- Add deterministic MSTest regression tests with Moq for both null-model and null-stores-list paths
- Include feature-folder lifecycle artifacts and QA gate evidence

Refs: #240
- Split 781-line test file into three partial-class files, each under the 500-line limit
- Trimmed original StoreWrapperController_Tests.cs to 181 lines
- New ButtonAndPopulate partial: 396 lines; new Launch partial: 234 lines
- Added two Compile Include entries to UtilitiesCS.Test.csproj
- All 39 test methods preserved; no behavior change; production code unchanged
- Includes feature-review audit artifacts and remediation evidence

Refs: #240
…e scope

- Add reaudit policy-audit/code-review/feature-audit and remediation-inputs (2026-07-06T13-00) confirming the 500-line blocking finding is resolved
- Annotate AC5 to scope the repository-coverage claim to the measured UtilitiesCS testable denominator and flag the repo-wide coverage artifact as maintainer-owned (feature/csharp-coverage-uplift)

Refs: #240
- Document the enforce-pr-author-skill.ps1 preflight blocker (missing scripts.dev_tools.validate_orchestration_artifacts module) that denies gh pr create
- Provide two resolution options: provision/repoint the hook validator, or create the PR manually
- Add human-exception-runbook agent memory on the missing MCP docs tool and the hook/MCP-validator reference

Refs: #240
@drmoisan drmoisan changed the title TaskMaster wt 2026 07 06 06 35 Fix(store-wrapper): guard StoreWrapperController.Launch() against unavailable store model (#240) Jul 6, 2026
@drmoisan
drmoisan merged commit 961a768 into main Jul 6, 2026
2 checks passed
@drmoisan
drmoisan deleted the TaskMaster-wt-2026-07-06-06-35 branch July 18, 2026 20:40
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.

Bug: store-wrapper-launch-npe

1 participant