diff --git a/.codex/agents/atomic-executor.toml b/.codex/agents/atomic-executor.toml new file mode 100644 index 00000000..d6bb4b7f --- /dev/null +++ b/.codex/agents/atomic-executor.toml @@ -0,0 +1,53 @@ +name = "atomic-executor" +description = "Execute an atomic plan exactly as written, verify acceptance criteria task-by-task, and stop only for preflight-blocking issues." + +developer_instructions = """ +You are an execution-only agent. + +Primary role: +- Execute a plan produced by the atomic-planner exactly as written. +- Preserve phase headings, task IDs, checkbox format, and task order. +- Verify each task’s acceptance criteria before checking it off. +- Do not re-plan. + +Use the following repo-local skills as the canonical workflow source: +- atomic-executor +- policy-compliance-order +- atomic-plan-contract +- acceptance-criteria-tracking + +Hard constraints: +- Do not invent new phases or tasks. +- Do not reorder tasks. +- Do not replace the plan with a different approach. +- Do not use an in-session todo list as a substitute for the plan file. +- The plan file on disk is the source of truth. + +Policy and preflight rules: +- Repository policy files are authoritative over this agent. +- Before execution, perform preflight validation against repo policy and plan-format requirements. +- Blocking is allowed only during preflight, before the first task executes. +- If the plan is invalid, non-atomic, non-verifiable, or conflicts with repo policy, stop at preflight and provide a precise plan delta for correction. +- After execution begins, do not block mid-plan; continue to completion within the allowed scope of the current tasks. + +Execution behavior: +- Execute tasks one by one, in order. +- Only perform micro-actions necessary to complete the current task. +- If completing the current task would require a new independent outcome not described in the plan, stop and request a plan revision only if still in preflight; otherwise stay within the task’s scope. +- After a task passes verification, immediately check it off in the canonical plan file on disk. +- If the completed work satisfies acceptance criteria in the resolved requirements source files, check those off as well per repo rules. + +Verification discipline: +- Prefer repo-defined commands and the canonical toolchain loop. +- Never claim success without verification. +- If the plan affects code or tests, ensure the final QA phase runs the repo-standard toolchain loop for all impacted languages. +- Treat expect-fail tasks according to the plan’s explicit acceptance criteria. + +Resume behavior: +- On resume or continue, load the plan-of-record, find the next unchecked task, and continue from there without replanning. + +Communication discipline: +- Be concise and exact. +- Report commands or tasks run and summarize results. +- End progress updates with the updated checklist or the current phase plus upcoming tasks. +""" \ No newline at end of file diff --git a/.codex/agents/atomic-planner.toml b/.codex/agents/atomic-planner.toml new file mode 100644 index 00000000..a7ecd759 --- /dev/null +++ b/.codex/agents/atomic-planner.toml @@ -0,0 +1,56 @@ +name = "atomic-planner" +description = "Generate deterministic phased implementation plans with atomic checkbox tasks and mandatory atomic-executor preflight clearance before finalization." + +developer_instructions = """ +You are a planning-only agent. + +Primary role: +- Produce phased implementation plans composed of atomic checkbox tasks. +- Design work that can be executed deterministically by an executor without replanning. +- You may read code, docs, plans, and repo policy for context, but you do not implement changes. + +Use the following repo-local skills as the canonical workflow source: +- atomic-planner +- policy-compliance-order +- atomic-plan-contract + +Hard constraints: +- Do not implement code, tests, configuration, CI, or docs beyond plan files. +- Do not execute the plan. +- Your only write scope is creating or updating a Markdown plan document when explicitly requested or when the calling workflow provides an authoritative plan target path. + +Plan requirements: +- Output a brief overview followed by phases and atomic tasks. +- Every task must be binary, atomic, and independently verifiable. +- Every task must use stable checkbox IDs in the canonical form [P#-T#]. +- If the plan changes code or tests, include Phase 0 baseline capture tasks and a final QA phase. +- Use executor-compatible formatting exactly. +- Reject placeholders, vague bucket tasks, and multi-outcome tasks. +- Prefer machine-verifiable acceptance criteria only. + +Mode and policy behavior: +- Repository policy files are authoritative over this agent. +- Resolve work mode from issue.md using repo rules. +- Fail closed when the mode marker is missing or malformed if repo policy requires that behavior. +- For minor-audit mode, require explicit evidence-capture tasks, targeted verification tasks, and end-state evidence tasks. + +Mandatory orchestration rule: +- Before finalizing any plan, explicitly spawn the `atomic-executor` subagent in preflight-validation-only mode. +- Pass the current draft plan file as the plan of record for validation. +- Wait for one of exactly two outcomes: + - `PREFLIGHT: ALL CLEAR` + - `PREFLIGHT: REVISIONS REQUIRED` +- If `atomic-executor` returns `PREFLIGHT: REVISIONS REQUIRED`, revise the same plan file and then explicitly spawn `atomic-executor` again in preflight-validation-only mode. +- Repeat this planner → executor-preflight loop until `atomic-executor` returns `PREFLIGHT: ALL CLEAR`. +- Do not finalize the plan before that result is obtained. + +Preflight loop behavior: +- Preserve the caller-provided target path when one is supplied. +- Do not create extra sibling plan files when an authoritative target file exists. +- Treat executor preflight findings as binding plan defects to be corrected before finalization. + +Self-checking: +- Before each executor preflight pass, confirm zero placeholders, atomic tasks only, canonical phase headings, canonical checkbox syntax, machine-verifiable acceptance criteria, and policy compatibility. +- Before final completion, confirm that `atomic-executor` has returned `PREFLIGHT: ALL CLEAR`. +- If the request is outside planning, refuse execution and direct the workflow to the executor. +""" \ No newline at end of file diff --git a/.codex/agents/feature-reviewer.toml b/.codex/agents/feature-reviewer.toml new file mode 100644 index 00000000..52cfaa41 --- /dev/null +++ b/.codex/agents/feature-reviewer.toml @@ -0,0 +1,55 @@ +name = "feature-reviewer" +description = "Review a feature branch relative to a base branch, generate policy/code/feature audit artifacts, and trigger remediation planning when required." + +developer_instructions = """ +You are a feature-branch reviewer. + +Primary role: +- Review the current feature branch relative to a specified base branch. +- Produce audit-grade review artifacts, not code changes. +- Do not ask clarifying questions unless execution is impossible; make best-effort assumptions and record them explicitly. + +Use the following repo-local skills as the canonical workflow source: +- feature-review +- policy-compliance-order +- evidence-and-timestamp-conventions +- policy-audit-template-usage +- pr-context-artifacts +- acceptance-criteria-tracking +- remediation-handoff-atomic-planner + +Core behavior: +- Treat repository policy files as authoritative over this agent. +- Use PR context artifacts as the primary source of truth for scope and baseline evidence. +- Read artifacts/pr_context.summary.txt thoroughly first. +- Use artifacts/pr_context.appendix.txt as secondary evidence for exact diff anchoring. +- If PR context artifacts are missing or stale, refresh them using the repo’s canonical mechanism before continuing. +- Determine the active feature folder deterministically from the repo’s scoping docs and PR context. +- Write timestamped review artifacts into the active feature folder. + +Required outputs: +- policy-audit..md +- code-review..md +- feature-audit..md +- remediation-inputs..md when remediation is required +- remediation-plan..md when remediation is required + +Review constraints: +- Do not modify policy documents. +- Prefer check-only / no-mutation verification commands. +- Do not silently fix code during review. +- If tooling cannot be run, mark the relevant sections UNVERIFIED or PARTIAL with a concrete reason. +- Continue until all required review artifacts exist on disk. + +Acceptance and remediation rules: +- Trigger remediation if policy audit has FAIL or meaningful PARTIAL findings, toolchain checks fail, blockers exist in code review, or required acceptance criteria are not fully met. +- When remediation is triggered, generate remediation-inputs first. +- Then delegate remediation planning to the atomic-planner agent or equivalent workflow. +- Remediation planning must treat remediation-inputs as the primary requirements source. + +Final response contract: +- Report every artifact path created or updated. +- Provide a one-paragraph go/no-go recommendation for PR readiness. +- If remediation was triggered, confirm the remediation planning handoff occurred. +- Do not claim completion unless all required reported artifacts exist on disk. +""" \ No newline at end of file diff --git a/.codex/codex-web-setup.plan.md b/.codex/codex-web-setup.plan.md index ffc23afe..8a02603a 100644 --- a/.codex/codex-web-setup.plan.md +++ b/.codex/codex-web-setup.plan.md @@ -15,9 +15,12 @@ Fix `.codex/codex-web-setup.sh` so a Linux-based Codex Web bootstrap does not fa 1. Refactor the verification path in `.codex/codex-web-setup.sh` so general prerequisite checks remain mandatory. 2. Detect non-Windows PowerShell hosts and downgrade the Visual Studio-specific task verification from a hard failure to a warning. 3. Update the script's repo notes so they describe partial verification on Linux instead of an expected non-zero exit. +4. Update `.github/workflows/codex-web-setup-test.yml` so CI expects the new Linux warning-and-success behavior rather than the previous hard failure. +5. Add non-default-branch CI triggers so the setup workflow can run from branch pushes and pull requests, not only from `workflow_dispatch` on the default branch. ## Verification 1. Run a syntax check for `.codex/codex-web-setup.sh`. 2. Run the repository PowerShell quality commands required by policy. 3. If practical, run a focused command path that exercises the non-Windows verification branch without reinstalling dependencies. +4. Run `actionlint` against the updated workflow definition. diff --git a/.codex/codex-web-setup.sh b/.codex/codex-web-setup.sh index 82eaeb06..5d0e553f 100755 --- a/.codex/codex-web-setup.sh +++ b/.codex/codex-web-setup.sh @@ -196,7 +196,7 @@ install_dotnet_coverage() { mkdir -p "${dotnet_tools_dir}" export PATH="${dotnet_tools_dir}:${PATH}" - append_if_missing "${HOME}/.bashrc" 'export PATH="$HOME/.dotnet/tools:$PATH"' + append_if_missing "${HOME}/.bashrc" "export PATH=\"\$HOME/.dotnet/tools:\$PATH\"" if command -v dotnet-coverage >/dev/null 2>&1; then log "dotnet-coverage is already available; skipping." diff --git a/.codex/prompts/feature-review-remediate.md b/.codex/prompts/feature-review-remediate.md new file mode 100644 index 00000000..3f03896b --- /dev/null +++ b/.codex/prompts/feature-review-remediate.md @@ -0,0 +1,10 @@ +--- +description: Run feature review and, if needed, remediation planning with executor preflight +--- + +$feature-review +Use pr_context as authoritative if present and valid. Only fall back to an explicit base branch if pr_context is missing, stale, or ambiguous. +If remediation is required, explicitly spawn atomic-planner to create or revise the remediation plan. +Before atomic-planner finalizes the plan, it must explicitly spawn atomic-executor in preflight-validation-only mode against that same file. +If atomic-executor returns PREFLIGHT: REVISIONS REQUIRED, atomic-planner must revise the same file and repeat until PREFLIGHT: ALL CLEAR. +Only then finalize the remediation plan and report all generated artifacts. \ No newline at end of file diff --git a/.github/workflows/codex-web-setup-test.yml b/.github/workflows/codex-web-setup-test.yml index 804629fb..b372229d 100644 --- a/.github/workflows/codex-web-setup-test.yml +++ b/.github/workflows/codex-web-setup-test.yml @@ -2,6 +2,14 @@ name: Codex Web Setup Test on: workflow_dispatch: + push: + paths: + - ".codex/codex-web-setup.sh" + - ".github/workflows/codex-web-setup-test.yml" + pull_request: + paths: + - ".codex/codex-web-setup.sh" + - ".github/workflows/codex-web-setup-test.yml" permissions: contents: read @@ -79,26 +87,19 @@ jobs: EOF chmod +x "${HOME}/.dotnet/tools/dotnet-coverage" - - name: Execute script and assert expected Linux failure + - name: Execute script and assert expected Linux warning shell: bash run: | set -euo pipefail export PATH="${GITHUB_WORKSPACE}/.github/test-bin:${PATH}" - set +e bash .codex/codex-web-setup.sh > codex-web-setup.log 2>&1 - exit_code=$? - set -e cat codex-web-setup.log - if [ "${exit_code}" -eq 0 ]; then - echo "Expected .codex/codex-web-setup.sh to fail on Linux verification, but it succeeded." - exit 1 - fi - - grep -F "Windows-only Visual Studio task requirements" codex-web-setup.log + grep -F "Skipping Windows-only Visual Studio task verification because this host is not Windows." codex-web-setup.log + grep -F "[codex-web-setup] Setup complete." codex-web-setup.log - name: Upload script log if: always() diff --git a/QuickFiler.Test/Controllers/EfcHomeControllerTests.cs b/QuickFiler.Test/Controllers/EfcHomeControllerTests.cs new file mode 100644 index 00000000..852d2805 --- /dev/null +++ b/QuickFiler.Test/Controllers/EfcHomeControllerTests.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using QuickFiler.Interfaces; +using UtilitiesCS; + +namespace QuickFiler.Controllers.Tests +{ + [TestClass] + public class EfcHomeControllerTests + { + private Mock _mockGlobals; + private Mock _mockParentCleanup; + + [TestInitialize] + public void Setup() + { + _mockGlobals = new Mock(MockBehavior.Loose); + _mockParentCleanup = new Mock(); + } + + /// + /// Creates an EfcHomeController via the private (globals, parentCleanup) constructor, + /// which does not allocate sub-components such as the data model or stop-watch. + /// + private EfcHomeController CreateMinimalController() + { + var ctor = typeof(EfcHomeController).GetConstructor( + BindingFlags.NonPublic | BindingFlags.Instance, + null, + new[] { typeof(IApplicationGlobals), typeof(System.Action) }, + null + ); + ctor.Should().NotBeNull("private (globals, parentCleanup) constructor must exist"); + return (EfcHomeController) + ctor.Invoke(new object[] { _mockGlobals.Object, _mockParentCleanup.Object }); + } + + private static void SetField(object target, string fieldName, object value) + { + var field = target + .GetType() + .GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Instance); + field.Should().NotBeNull($"field '{fieldName}' must exist on EfcHomeController"); + field.SetValue(target, value); + } + + // Regression test for: + // System.NullReferenceException at EfcHomeController.ExecuteMovesAsync line 346 + // Root cause: re-entrant invocation could call Cleanup() while MoveToFolderAsync was + // awaited, nulling _globals. The second continuation then dereferenced null _globals. + [TestMethod] + public async Task ExecuteMovesAsync_WhenAlreadyExecuting_ReturnsWithoutAccessingNullFields() + { + // Arrange + var controller = CreateMinimalController(); + + // Simulate a concurrent invocation already in progress. + SetField(controller, "_isExecuting", true); + + // _formController is null from the private constructor. + // If the guard were absent, the very first line of ExecuteMovesAsync would throw + // NullReferenceException on _formController.SelectedFolder. + + // Act & Assert: must complete without exception. + Func act = () => controller.ExecuteMovesAsync(); + await act.Should() + .NotThrowAsync("a re-entrant call must be dropped via the _isExecuting guard"); + } + + // Regression test for the inverted guard in QuickFileMetrics_WRITE. + // The original condition was `moved.Count == 0`, which entered the metrics-writing + // block only when the list was empty and would immediately throw DivideByZeroException + // on `Duration /= moved.Count`. The fix changes the condition to `moved.Count > 0`. + [TestMethod] + public void QuickFileMetrics_WRITE_WithEmptyList_SkipsBodyAndDoesNotThrow() + { + // Arrange + var controller = CreateMinimalController(); + // _stopWatch is null in a controller created via the private constructor. + // If the body were entered (old bug: Count == 0), accessing _stopWatch.Elapsed + // would throw NullReferenceException before even reaching DivideByZeroException. + var emptyMoved = new List(); + + // Act & Assert + Action act = () => + controller.QuickFileMetrics_WRITE("session.csv", @"Inbox\Projects", emptyMoved); + act.Should() + .NotThrow( + "an empty moved list must cause the metrics body to be skipped (Count > 0 guard)" + ); + } + + [TestMethod] + public void QuickFileMetrics_WRITE_WithNullList_SkipsBodyAndDoesNotThrow() + { + // Arrange + var controller = CreateMinimalController(); + + // Act & Assert + Action act = () => + controller.QuickFileMetrics_WRITE("session.csv", @"Inbox\Projects", null); + act.Should().NotThrow("a null moved list must be handled by the null guard"); + } + } +} diff --git a/QuickFiler.Test/Controllers/QfcItemControllerTests.cs b/QuickFiler.Test/Controllers/QfcItemControllerTests.cs new file mode 100644 index 00000000..02ff66cb --- /dev/null +++ b/QuickFiler.Test/Controllers/QfcItemControllerTests.cs @@ -0,0 +1,165 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using QuickFiler.Controllers; +using QuickFiler.Helper_Classes; + +namespace QuickFiler.Controllers.Tests +{ + /// + /// Regression tests for the cancellation-flow bug in QfcItemController. + /// + /// Root cause: LoadConversationResolverAsync caught all exceptions including + /// OperationCanceledException and suppressed them, leaving ConversationResolver null. + /// PopulateConversationAsync then dereferenced the null property and crashed with + /// NullReferenceException instead of propagating the OperationCanceledException. + /// + [TestClass] + public class QfcItemControllerTests + { + // --------------------------------------------------------------------------- + // Test double: subclass that overrides the static-call seam so tests do not + // require WinForms infrastructure (ItemViewer, MailItem, etc.). + // --------------------------------------------------------------------------- + private sealed class TestableQfcItemController : QfcItemController + { + private readonly Func> _loadCore; + + /// + /// Delegate executed in place of ConversationResolver.LoadAsync. + /// Throw OperationCanceledException to simulate a mid-load cancellation. + /// Throw any other exception to simulate a non-cancellation load failure. + /// + internal TestableQfcItemController(Func> loadCore) + : base() + { + _loadCore = loadCore; + } + + protected override Task DoLoadConversationResolverCoreAsync( + CancellationTokenSource tokenSource, + CancellationToken token, + bool loadAll + ) => _loadCore(); + } + + // --------------------------------------------------------------------------- + // LoadConversationResolverAsync tests + // --------------------------------------------------------------------------- + + [TestMethod] + public async Task LoadConversationResolverAsync_WhenLoadThrowsOperationCanceled_PropagatesCancellation() + { + // Arrange + // Simulates OperationCanceledException thrown from inside ConversationResolver.LoadAsync + // (e.g., from TimeOutTask.RunWithTimeout -> GetConversationDfAsync) while the token + // was canceled during the async operation. + // A separate CTS is used for the method call so the pre-guard passes; the seam + // throws unconditionally to reproduce the bug scenario. + var callCts = new CancellationTokenSource(); + var callToken = callCts.Token; // not canceled; pre-guard passes + + var controller = new TestableQfcItemController(() => + throw new OperationCanceledException(callToken) + ); + + // Act + Func act = () => + controller.LoadConversationResolverAsync(callCts, callToken, false); + + // Assert — before the fix this call completed silently (exception was suppressed) + await act.Should() + .ThrowAsync( + because: "cancellation during load must propagate, not be swallowed" + ); + + callCts.Dispose(); + } + + [TestMethod] + public async Task LoadConversationResolverAsync_WhenLoadThrowsNonCancellation_DoesNotThrow() + { + // Arrange — non-cancellation exceptions (e.g., COM errors) must still be + // suppressed and logged so the overall populate flow can continue gracefully. + var callCts = new CancellationTokenSource(); + var callToken = callCts.Token; + + var controller = new TestableQfcItemController(() => + throw new InvalidOperationException("simulated non-cancel load failure") + ); + + // Act + Func act = () => + controller.LoadConversationResolverAsync(callCts, callToken, false); + + // Assert — non-OCE must still be swallowed; behaviour is unchanged from before fix + await act.Should() + .NotThrowAsync( + because: "non-cancellation load failures must be suppressed and logged" + ); + + callCts.Dispose(); + } + + // --------------------------------------------------------------------------- + // PopulateConversationAsync tests + // --------------------------------------------------------------------------- + + [TestMethod] + public async Task PopulateConversationAsync_WhenLoadCanceledDuringAsync_ThrowsOperationCanceledNotNullRef() + { + // Arrange + // This is the exact regression scenario: token cancelled during LoadAsync, + // OCE was swallowed, ConversationResolver was null, and PopulateConversationAsync + // crashed with NullReferenceException on ConversationResolver.Count.SameFolder. + var callCts = new CancellationTokenSource(); + var callToken = callCts.Token; + + var controller = new TestableQfcItemController(() => + throw new OperationCanceledException(callToken) + ); + + // Act + Func act = () => controller.PopulateConversationAsync(callCts, callToken, false); + + // Assert — before the fix this threw NullReferenceException; + // after the fix it propagates OperationCanceledException. + await act.Should() + .ThrowAsync( + because: "a mid-load cancellation must surface as OperationCanceledException, " + + "not crash with NullReferenceException on the null ConversationResolver" + ); + + callCts.Dispose(); + } + + [TestMethod] + public async Task PopulateConversationAsync_WhenLoadFailsWithNonCancellation_ReturnsWithoutCrash() + { + // Arrange — verifies the null guard added to PopulateConversationAsync: + // if a non-cancellation exception causes the resolver to remain null, + // the method must return cleanly rather than dereference null. + var callCts = new CancellationTokenSource(); + var callToken = callCts.Token; + + var controller = new TestableQfcItemController(() => + throw new InvalidOperationException("simulated non-cancel load failure") + ); + + // Act + Func act = () => controller.PopulateConversationAsync(callCts, callToken, false); + + // Assert — ConversationResolver is null after the suppressed load failure; + // the null guard must prevent a NullReferenceException. + await act.Should() + .NotThrowAsync( + because: "when load fails silently and ConversationResolver is null, " + + "PopulateConversationAsync must return without crashing" + ); + + callCts.Dispose(); + } + } +} diff --git a/QuickFiler.Test/QuickFiler.Test.csproj b/QuickFiler.Test/QuickFiler.Test.csproj index d5320f4d..0524bb22 100644 --- a/QuickFiler.Test/QuickFiler.Test.csproj +++ b/QuickFiler.Test/QuickFiler.Test.csproj @@ -75,8 +75,10 @@ bin\x86\Release\ + + Form diff --git a/QuickFiler/Controllers/EfcHomeController.cs b/QuickFiler/Controllers/EfcHomeController.cs index 348675b5..8a24cad6 100644 --- a/QuickFiler/Controllers/EfcHomeController.cs +++ b/QuickFiler/Controllers/EfcHomeController.cs @@ -285,6 +285,8 @@ public System.Diagnostics.Stopwatch StopWatch get => _stopWatch; } + private volatile bool _isExecuting; + public bool Loaded => throw new NotImplementedException(); internal void CreateCancellationToken() @@ -319,35 +321,49 @@ public SynchronizationContext UiSyncContext async public Task ExecuteMovesAsync() { - var selectedFolder = _formController.SelectedFolder; - var moveConversation = _formController.MoveConversation; - var convInfo = DataModel.ConversationResolver.ConversationInfo.SameFolder; - if (!moveConversation) + if (_isExecuting) + return; + + _isExecuting = true; + try { - convInfo = convInfo - .Where(itemInfo => itemInfo.EntryId == DataModel.Mail.EntryID) - .ToList(); - } + var selectedFolder = _formController.SelectedFolder; + var moveConversation = _formController.MoveConversation; + var convInfo = DataModel.ConversationResolver.ConversationInfo.SameFolder; + if (!moveConversation) + { + convInfo = convInfo + .Where(itemInfo => itemInfo.EntryId == DataModel.Mail.EntryID) + .ToList(); + } - var result = await _dataModel.MoveToFolderAsync( - selectedFolder, - _formController.SaveAttachments, - _formController.SaveEmail, - _formController.SavePictures, - moveConversation - ); + // Capture _globals before the await: Cleanup() may null the field while + // MoveToFolderAsync is in flight, causing NullReferenceException on resume. + var globals = _globals; + var result = await _dataModel.MoveToFolderAsync( + selectedFolder, + _formController.SaveAttachments, + _formController.SaveEmail, + _formController.SavePictures, + moveConversation + ); - if (!result) - { - MessageBox.Show($"Cannot move to folderpath {selectedFolder}"); + if (!result) + { + MessageBox.Show($"Cannot move to folderpath {selectedFolder}"); + } + else + { + QuickFileMetrics_WRITE( + globals.FS.Filenames.EmailSession, + selectedFolder, + convInfo + ); + } } - else + finally { - QuickFileMetrics_WRITE( - _globals.FS.Filenames.EmailSession, - selectedFolder, - convInfo - ); + _isExecuting = false; } } @@ -367,7 +383,7 @@ public void QuickFileMetrics_WRITE( List moved ) { - if (moved is not null && moved.Count == 0) + if (moved is not null && moved.Count > 0) { var curDateText = DateTime.Now.ToString("MM/dd/yyyy"); var curTimeText = DateTime.Now.ToString("hh:mm"); diff --git a/QuickFiler/Controllers/QfcHomeController.cs b/QuickFiler/Controllers/QfcHomeController.cs index 45db44ba..9002189e 100644 --- a/QuickFiler/Controllers/QfcHomeController.cs +++ b/QuickFiler/Controllers/QfcHomeController.cs @@ -522,15 +522,22 @@ out Folder OlEmailCalendar "Email Time", Globals.Ol.App.Session ); - OlAppointment = (AppointmentItem)OlEmailCalendar.Items.Add(); + if (OlEmailCalendar is null) { - OlAppointment.Subject = $"Quick Filed {emailsLoaded} emails"; - OlAppointment.Start = OlStartTime; - OlAppointment.End = OlEndTime; - OlAppointment.Categories = "@ Email"; - OlAppointment.ReminderSet = false; - OlAppointment.Sensitivity = OlSensitivity.olPrivate; - OlAppointment.Save(); + OlAppointment = null; + } + else + { + OlAppointment = (AppointmentItem)OlEmailCalendar.Items.Add(); + { + OlAppointment.Subject = $"Quick Filed {emailsLoaded} emails"; + OlAppointment.Start = OlStartTime; + OlAppointment.End = OlEndTime; + OlAppointment.Categories = "@ Email"; + OlAppointment.ReminderSet = false; + OlAppointment.Sensitivity = OlSensitivity.olPrivate; + OlAppointment.Save(); + } } } diff --git a/QuickFiler/Controllers/QfcItemController.cs b/QuickFiler/Controllers/QfcItemController.cs index 0ddbba54..7a0c7921 100644 --- a/QuickFiler/Controllers/QfcItemController.cs +++ b/QuickFiler/Controllers/QfcItemController.cs @@ -30,7 +30,7 @@ internal class QfcItemController : IQfcItemController, INotifyPropertyChanged, I #region ctor - private QfcItemController() { } + protected QfcItemController() { } public QfcItemController( IApplicationGlobals appGlobals, @@ -597,15 +597,17 @@ bool loadAll try { - ConversationResolver = await ConversationResolver.LoadAsync( - _globals, - ItemHelper, + ConversationResolver = await DoLoadConversationResolverCoreAsync( tokenSource, token, - loadAll, - SetTopicThread + loadAll ); } + catch (OperationCanceledException) + { + // Cancellation is an expected flow; propagate so callers can observe it. + throw; + } catch (System.Exception e) { logger.Error($"Error in PopulateConversationAsync: {e.Message}", e); @@ -613,6 +615,24 @@ bool loadAll } } + /// + /// Seam for the static ConversationResolver.LoadAsync call. Override in tests to + /// inject controlled behaviour without requiring WinForms infrastructure. + /// + protected virtual Task DoLoadConversationResolverCoreAsync( + CancellationTokenSource tokenSource, + CancellationToken token, + bool loadAll + ) => + ConversationResolver.LoadAsync( + _globals, + ItemHelper, + tokenSource, + token, + loadAll, + SetTopicThread + ); + public async Task PopulateConversationAsync( CancellationTokenSource tokenSource, CancellationToken token, @@ -620,6 +640,9 @@ bool loadAll ) { await LoadConversationResolverAsync(tokenSource, token, loadAll); + token.ThrowIfCancellationRequested(); + if (ConversationResolver is null) + return; await RenderConversationCountAsync( ConversationResolver.Count.SameFolder, token, diff --git a/TaskMaster/AppGlobals/AppAutoFileObjects.cs b/TaskMaster/AppGlobals/AppAutoFileObjects.cs index a6e6b8ce..f1d3f413 100644 --- a/TaskMaster/AppGlobals/AppAutoFileObjects.cs +++ b/TaskMaster/AppGlobals/AppAutoFileObjects.cs @@ -588,15 +588,20 @@ internal void SubjectMap_CollectionChanged( NotifyCollectionChangedEventArgs e ) { - SubjectMapSco map = (SubjectMapSco)sender; - - if (e.Action == NotifyCollectionChangedAction.Add) + // Read the newly added entry from the event args rather than from the collection. + // Calling map.Last() (or any collection read) here would trigger DoBaseRead, which + // tries to acquire a read lock on the same ReaderWriterLockSlim that DoBaseWrite + // is already holding on this thread. ReaderWriterLockSlim (NoRecursion policy) + // throws LockRecursionException. e.NewItems[0] is safe because it does not + // re-enter the collection lock. + if (e.Action == NotifyCollectionChangedAction.Add && e.NewItems?.Count > 0) { - var entry = map.Last(); + var entry = (SubjectMapEntry)e.NewItems[0]; entry.Encode(Encoder); } else if (e.Action == NotifyCollectionChangedAction.Reset) { + var map = (SubjectMapSco)sender; Encoder.RebuildEncoding(map); } } diff --git a/TaskMaster/Ribbon/RibbonExplorer.xml b/TaskMaster/Ribbon/RibbonExplorer.xml index a385557d..3693e147 100644 --- a/TaskMaster/Ribbon/RibbonExplorer.xml +++ b/TaskMaster/Ribbon/RibbonExplorer.xml @@ -1,5 +1,5 @@  - + @@ -167,7 +167,7 @@ /> - +