v0.1.10: skip imports + pre-existing fights + Import/Merge zone

Two reported bugs:

1. Importing a log file made the plugin auto-upload the historical
   fights it brought in. New rule: skip any encounter whose StartTime
   is more than InstanceStartGraceSeconds (= 5 min) before the
   plugin's own startup. Catches three scenarios with one check:
     * user enabled the plugin mid-session (pre-existing fights)
     * user imported a log file after enabling
     * ACT auto-loaded logs at startup before plugin init

   5-minute grace accommodates an in-progress fight that started just
   before plugin-enable. The manual right-click upload path
   intentionally does NOT enforce this — it's the escape hatch for
   "yes actually, please upload that older one".

2. Merged/edited encounters in ACT's "Import/Merge" zone were
   uploadable. New rule: never upload from Import/Merge — in the auto
   path it's skipped with a visible reason, in the right-click menu
   it's greyed out, and Plugin.OnManualUploadRequested re-checks
   defensively in case a fast click races the Opening handler.

   Merged parses are user-customised (combining two real fights into
   an aggregate that doesn't correspond to any single in-game event)
   and would pollute the leaderboard with parses that didn't happen
   as shown.

Implementation:
  * New src/Core/EncounterZone.cs with IsImportOrMerge(zoneName) —
    same pure-Core-testable-without-ACT pattern as EncounterTitle.
  * EncounterCapture.Poll: two new skip branches above the existing
    placeholder-title one. All three now share a tiny
    MarkProcessedNoCapture helper so the lock+queue+cap eviction
    isn't duplicated.
  * ActMenuExtension.OnMenuOpening: greys out the menu item for
    Import/Merge in addition to the existing "no real encounter
    selected" condition.
  * Plugin.OnManualUploadRequested: defensive Import/Merge re-check
    surfaced as "manual upload skipped (Import/Merge zone — ...)".

CLAUDE.md updated: the manual-upload gate matrix now lists both new
gates explicitly, and EncounterCapture's Key files entry calls out
all three skip reasons. 13 new tests in EncounterZoneTests pinning
the synthetic-zone string. Total 144/144.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: VortexUK

CLAUDE.md

@@ -25,9 +25,10 @@ The split also unlocks GitHub Actions CI: the workflow at `.github/workflows/ci.
 |---|---|
 | `src/Plugin.cs` | `IActPluginV1` entry point. ACT calls `InitPlugin`/`DeInitPlugin`. Resolves the config path via `ActHelpers.GetConfigPath()` and passes it to `PluginConfig.Load`/`Save`. Owns lifecycles of `PluginConfig`, `SettingsPanel`, `EncounterCapture`, `UploadClient`. |
 | `src/SettingsPanel.cs` | WinForms settings UI hosted in ACT's plugin tab. Dark-theme card layout (`T` static class holds every colour). Three cards: Configuration / Logging As / Last Captured. Persistence happens via the `_onSave` callback so the panel stays path-free. |
-| `src/EncounterCapture.cs` | 2 s polling timer over `ActGlobals.oFormActMain.ActiveZone`. Detects "settled" encounters (no `EndTime` update for `SettleSeconds=15`), converts to `EncounterSnapshot` via the public static `CaptureSnapshot` (reused by the manual-upload path), then calls `PayloadBuilder.BuildPayload` → `SanitizePayload` → `SerializeJson`. Defers placeholder-titled encounters via `EncounterTitle.IsPlaceholder` for up to `MaxPlaceholderWaitSeconds=60` (then raises `OnSkipped` with a user-visible reason). |
+| `src/EncounterCapture.cs` | 2 s polling timer over `ActGlobals.oFormActMain.ActiveZone`. Detects "settled" encounters (no `EndTime` update for `SettleSeconds=15`), converts to `EncounterSnapshot` via the public static `CaptureSnapshot` (reused by the manual-upload path), then calls `PayloadBuilder.BuildPayload` → `SanitizePayload` → `SerializeJson`. Skip reasons (all raise `OnSkipped` with a user-visible message): Import/Merge zone, encounter started before `_instanceStartedAt - InstanceStartGraceSeconds` (= log imports + pre-existing fights), placeholder title that didn't resolve within `MaxPlaceholderWaitSeconds`. The three skip branches share `MarkProcessedNoCapture` so the encid eviction logic isn't duplicated. |
 | `src/ActMenuExtension.cs` | Adds "Upload to EQ2 Lexicon" to ACT's right-click encounter menu. See "ACT UI extension" below for the (undocumented) wiring details. |
 | `src/Core/EncounterTitle.cs` | `IsPlaceholder(title)` — the shared predicate used by both the polling and manual-upload paths to reject encounters ACT hasn't named yet ("Encounter", "Unknown", empty/whitespace). Lives in Core so it's testable without ACT. |
+| `src/Core/EncounterZone.cs` | `IsImportOrMerge(zoneName)` — the shared predicate used by the polling path, the right-click menu's Opening handler (greys out the item), and Plugin's manual-upload handler (defensive re-check). The Import/Merge bucket holds imported logs and merged/edited encounters that we must never upload. |
 | `src/ActHelpers.cs` | Reads the logging character name by parsing the active log file path (`eq2log_<name>.txt`). `ActGlobals.charName` returns `"YOU"` in EQ2 so it can't be used. Also owns `GetConfigPath()` — the `%APPDATA%\Advanced Combat Tracker\Config\` resolver kept here so `PluginConfig` stays ACT-free. |
 | `src/Core/PayloadBuilder.cs` | Pure transform: `EncounterSnapshot` → ingest-JSON dict shape. Owns `OutgoingGroupToSwingType`, `FormatTime` (emits ISO-8601 + Z), `SanitizePayload` (replaces NaN/∞ with 0), and `SerializeJson` (JavaScriptSerializer with an 8 MB ceiling). |
 | `src/Core/Snapshots.cs` | Plain DTOs (`EncounterSnapshot`, `CombatantSnapshot`, `DamageTypeAggregate`, `AttackTypeSnapshot`) that mirror the slice of ACT's data model the payload builder reads. |
@@ -161,6 +162,8 @@ Different from the polling path — bypasses user opt-in gates because the click
 |---|---|---|
 | Blacklist (don't-upload-as) | **bypassed** | enforced |
 | "Enable automatic upload" checkbox | **bypassed** | enforced |
+| Import/Merge zone (`EncounterZone.IsImportOrMerge`) | enforced (menu greyed + defensive re-check) | enforced (skipped with reason) |
+| Pre-plugin-startup encounter | **bypassed** (manual is the escape hatch) | enforced (skipped with reason) |
 | Placeholder title (`EncounterTitle.IsPlaceholder`) | enforced (rejects with "rename in ACT first") | deferred up to 60s, then skipped |
 | API token configured | enforced | enforced |
 | Version not too old | enforced | enforced |

src/ActMenuExtension.cs

@@ -128,7 +128,14 @@ private void OnMenuOpening(object sender, CancelEventArgs e)
             // Gray out when there's no real encounter selected (zone
             // node, empty area, etc.) so the user gets the right
             // affordance instead of clicking a no-op item.
-            _menuItem.Enabled = GetSelectedEncounter() != null;
+            //
+            // Also greyed out for the Import/Merge zone — those are
+            // user-customised parses (merged fights, imported logs)
+            // that we never want uploaded. The click handler re-checks
+            // defensively in case a fast click somehow races the
+            // Opening event's enable computation.
+            var enc = GetSelectedEncounter();
+            _menuItem.Enabled = enc != null && !EncounterZone.IsImportOrMerge(enc.ZoneName);
         }
 
         private void OnMenuClick(object sender, EventArgs e)

src/Core/EQ2Lexicon.ACTPlugin.Core.csproj

@@ -18,7 +18,7 @@
       Build metadata is mirrored from the UI csproj so this assembly's
       version tracks the user-facing plugin version.
     -->
-    <Version>0.1.9</Version>
+    <Version>0.1.10</Version>
     <Company>EQ2 Lexicon</Company>
     <Product>EQ2 Lexicon Uploader (Core)</Product>
     <Description>Pure payload / config / HTTP layer shared by EQ2Lexicon.ACTPlugin.</Description>

src/Core/EncounterZone.cs

@@ -0,0 +1,38 @@
+using System;
+
+namespace EQ2Lexicon.ACTPlugin
+{
+    /// <summary>
+    /// Classifies ACT zone names. Pure (no ACT references) so the test
+    /// project can exercise the predicate set without ACT installed.
+    /// </summary>
+    public static class EncounterZone
+    {
+        /// <summary>
+        /// True when the zone is ACT's synthetic "Import/Merge" bucket —
+        /// where imported logs and merged/edited encounters end up. The
+        /// plugin must never upload from this zone because:
+        ///
+        ///   * Merged encounters are USER-CUSTOMISED — combining two real
+        ///     fights produces an aggregate that doesn't correspond to
+        ///     any single in-game event. Uploading it would pollute the
+        ///     leaderboard with parses that didn't actually happen as
+        ///     shown.
+        ///   * Imported parses are old log replays — the user already
+        ///     had the chance to upload them when they happened. Doing
+        ///     it on import would back-date arbitrary historical fights.
+        ///
+        /// Applied in both directions:
+        ///   * EncounterCapture.Poll skips with a visible reason.
+        ///   * ActMenuExtension.OnMenuOpening greys out the right-click
+        ///     menu item.
+        ///   * Plugin.OnManualUploadRequested re-checks defensively (in
+        ///     case a fast click races the menu's greyed-out state).
+        /// </summary>
+        public static bool IsImportOrMerge(string? zoneName)
+        {
+            if (string.IsNullOrWhiteSpace(zoneName)) return false;
+            return string.Equals(zoneName.Trim(), "Import/Merge", StringComparison.OrdinalIgnoreCase);
+        }
+    }
+}

src/EQ2Lexicon.ACTPlugin.csproj

@@ -22,7 +22,7 @@
     <ActInstallDir Condition="'$(ActInstallDir)' == ''">C:\Program Files (x86)\Advanced Combat Tracker</ActInstallDir>
 
     <!-- Build metadata shown in ACT's plugin list -->
-    <Version>0.1.9</Version>
+    <Version>0.1.10</Version>
     <Company>EQ2 Lexicon</Company>
     <Product>EQ2 Lexicon Uploader</Product>
     <Description>Uploads ACT-parsed encounters to the EQ2 Lexicon site.</Description>

src/EncounterCapture.cs

@@ -49,6 +49,25 @@ internal class EncounterCapture : IDisposable
         // UI so the user knows we didn't quietly drop a real fight.
         private const double MaxPlaceholderWaitSeconds = 60.0;
 
+        // Grace window for "is this encounter pre-existing or imported?".
+        // An encounter whose StartTime is more than this far before
+        // _instanceStartedAt is treated as not-ours: either it was already
+        // in zone.Items when the plugin loaded (user enabled mid-session),
+        // or the user just imported a log file with old fights. Either
+        // way the auto path skips it — the manual right-click path stays
+        // as the escape hatch.
+        //
+        // 5 minutes accommodates an in-progress fight that started
+        // shortly before plugin-enable (we don't want to drop the current
+        // raid pull just because the plugin loaded 30s into it).
+        private const double InstanceStartGraceSeconds = 300.0;
+
+        // Stamped once in the ctor — used to draw the "before me" line.
+        // DateTime.Now (local) because every other timestamp from ACT in
+        // this class is also local (StartTime/EndTime are ACT-emitted
+        // local-clock values).
+        private readonly DateTime _instanceStartedAt = DateTime.Now;
+
         // Last captured artefact — read by the settings panel for display.
         public string LastCapturedEncId { get; private set; } = "";
         public string LastCapturedTitle { get; private set; } = "";
@@ -118,6 +137,32 @@ private void Poll()
                 // Not settled yet — ACT may still be appending actions.
                 if ((now - enc.EndTime).TotalSeconds < SettleSeconds) continue;
 
+                // Import/Merge zone is ACT's bucket for imported logs
+                // and merged/edited encounters. Never auto-upload — those
+                // are user-customised, not authoritative parses. The
+                // manual right-click path enforces this too (greyed out).
+                if (EncounterZone.IsImportOrMerge(enc.ZoneName))
+                {
+                    MarkProcessedNoCapture(encid);
+                    OnSkipped?.Invoke(
+                        $"skipped ({encid}: Import/Merge zone — customised parses aren't uploaded)");
+                    continue;
+                }
+
+                // Pre-existing or freshly-imported encounter — the user
+                // didn't actively play this one under our watch. Either
+                // they enabled the plugin mid-session (these were already
+                // here), or they just imported a log file. Auto path
+                // skips; manual right-click upload still works as the
+                // escape hatch for "yes actually, please upload that".
+                if (enc.StartTime < _instanceStartedAt.AddSeconds(-InstanceStartGraceSeconds))
+                {
+                    MarkProcessedNoCapture(encid);
+                    OnSkipped?.Invoke(
+                        $"skipped ({encid}: started before plugin enabled — likely import or pre-existing)");
+                    continue;
+                }
+
                 // Placeholder title: defer in the hope ACT fills it in on
                 // a later poll. Past MaxPlaceholderWaitSeconds, give up
                 // and surface a skip reason so the user doesn't think the
@@ -129,15 +174,7 @@ private void Poll()
                         // Don't add to _processed — retry next tick.
                         continue;
                     }
-                    lock (_lock)
-                    {
-                        _processed.Add(encid);
-                        _processedQueue.Enqueue(encid);
-                        while (_processedQueue.Count > ProcessedCap)
-                        {
-                            _processed.Remove(_processedQueue.Dequeue());
-                        }
-                    }
+                    MarkProcessedNoCapture(encid);
                     OnSkipped?.Invoke(
                         $"skipped ({encid}: title never resolved past '{enc.Title}')");
                     continue;
@@ -147,6 +184,26 @@ private void Poll()
             }
         }
 
+        /// <summary>
+        /// Add an encid to the processed set without producing a capture.
+        /// Used by the three "skip reasons" branches above (Import/Merge
+        /// zone, pre-plugin-startup encounter, placeholder title that
+        /// never resolved) — all of them need to evict the encid from
+        /// future poll iterations, none want to fire OnCaptured.
+        /// </summary>
+        private void MarkProcessedNoCapture(string encid)
+        {
+            lock (_lock)
+            {
+                _processed.Add(encid);
+                _processedQueue.Enqueue(encid);
+                while (_processedQueue.Count > ProcessedCap)
+                {
+                    _processed.Remove(_processedQueue.Dequeue());
+                }
+            }
+        }
+
         // The placeholder-title predicate lives in Core
         // (EncounterTitle.IsPlaceholder) so the test project can
         // exercise it without ACT installed. Both the polling path

src/Plugin.cs

@@ -145,6 +145,18 @@ private void OnManualUploadRequested(EncounterData enc)
         {
             if (_settingsPanel == null || _config == null || _uploadClient == null) return;
 
+            // Defensive re-check of the Import/Merge gate even though
+            // the menu item is greyed out for these encounters — a
+            // fast click could in principle race the Opening handler's
+            // enable computation.
+            if (EncounterZone.IsImportOrMerge(enc.ZoneName))
+            {
+                _settingsPanel.SetUploadStatus(
+                    "manual upload skipped (Import/Merge zone — customised parses can't be uploaded)",
+                    success: false);
+                return;
+            }
+
             // Placeholder check uses the SAME predicate as the
             // automatic path so the rule stays consistent. The
             // message tells the user how to fix it themselves.

tests/EQ2Lexicon.ACTPlugin.Tests/EncounterZoneTests.cs

@@ -0,0 +1,37 @@
+using Xunit;
+
+namespace EQ2Lexicon.ACTPlugin.Tests
+{
+    /// <summary>
+    /// Pins the synthetic-zone set. The auto-upload path AND the
+    /// right-click manual path both call this predicate; a silent
+    /// change here changes user-visible behaviour in both flows.
+    /// </summary>
+    public class EncounterZoneTests
+    {
+        [Theory]
+        [InlineData("Import/Merge")]      // exact ACT spelling
+        [InlineData("import/merge")]      // case-insensitive
+        [InlineData("IMPORT/MERGE")]
+        [InlineData("  Import/Merge  ")]  // whitespace tolerant
+        public void IsImportOrMerge_TrueForTheSyntheticBucket(string zoneName)
+        {
+            Assert.True(EncounterZone.IsImportOrMerge(zoneName));
+        }
+
+        [Theory]
+        [InlineData("Great Divide")]
+        [InlineData("The Peat Bog")]
+        [InlineData("The Sinking Sands")]
+        [InlineData("Import/Merge zone something")] // not a bare match
+        [InlineData("Import")]                       // partial isn't enough
+        [InlineData("Merge")]
+        [InlineData("")]                             // empty zone name → real fight without zone metadata, allow
+        [InlineData("   ")]
+        [InlineData(null)]
+        public void IsImportOrMerge_FalseForRealZonesAndEmpty(string? zoneName)
+        {
+            Assert.False(EncounterZone.IsImportOrMerge(zoneName));
+        }
+    }
+}