BatchPersistence discovery-cache leaks across tests (TestList_WhenNoCachedDiscoveryExists)

[Scaler0222]

Background

Spun off from issue #1 (Unity 6.5 EditMode test failures). The Phase 2 consensus deferred this specific failure to keep PR scopes tight; the adversarial debate gate (see .claude/embrace-gate-define-develop-2026-05-28.md in the host project) confirmed it's a real test-isolation defect worth a follow-up.

The failing test

UnitySkills.Tests.Core.TestInfrastructureTests.TestList_WhenNoCachedDiscoveryExists_StartsAsyncDiscovery (SkillsForUnity/Tests/Editor/Core/TestInfrastructureTests.cs:259)

Expected: False
But was:  True

The test asserts that TestSkills.TestList() returns success=false when no discovery cache exists, but success=true was returned — meaning a cached discovery from a sibling test (or a prior CLI test pass) is still in BatchPersistence.

In PR#1 (the quick-win), this test is Assert.Ignore-quarantined with a link to this issue. The quarantine is a holding pattern, not a fix.

Root cause (per Codex Phase 1 analysis)

BatchPersistence keeps batch state as a static cache (SkillsForUnity/Editor/Skills/BatchPersistence.cs:16-22), and EnsureLoaded early-returns once _state exists (BatchPersistence.cs:32-40). BatchJobService auto-loads and resumes jobs on editor load (SkillsForUnity/Editor/Skills/BatchJobService.cs:22-27, 116-124).

Net effect: across-test cache leakage. Tests can pass or fail based on ordering and prior Library state.

Why deferred from PR#2

Phase 2 consensus (multi-LLM): "BatchPersistence design (Issue Besty0728#10) is entirely deferred. To prevent an undesigned hook from introducing cross-cutting system regressions, absolutely zero BatchPersistence or cache reset changes will be introduced across any of the three PRs."

The risk is real but the design space is larger than a single test fix:

• Should BatchPersistence expose a public ResetForTests() (mirrors SkillsModeManager.ResetForTests())?
• Should the test fixture use a transient scope (BatchPersistence.BeginTransientScope() already exists per BatchGovernanceTests)?
• Should TestSkills.TestList itself become idempotent re: cached discovery?

Suggested resolution shape (non-blocking)

The cleanest minimal change:

1. Add internal static void ResetDiscoveryCache() to BatchPersistence — removes only discovery jobs from the in-memory state.
2. Wire it into TestInfrastructureTests.[SetUp] for the affected test only.
3. Remove the Assert.Ignore quarantine from PR#1.

But that's design work, not a one-liner. Hence this follow-up issue.

Acceptance criteria

• The quarantine on TestList_WhenNoCachedDiscoveryExists_StartsAsyncDiscovery is removed.
• The test passes deterministically with no other tests in the cache OR after other tests have populated it.
• No regression in the 110 currently-passing UnitySkills.Tests.Core tests.

Linked

• Parent: Unity 6.5 EditMode: 14 pre-existing test failures (triage, none caused by EntityId rename) #1
• Multi-LLM debate gate that surfaced this (host project): .claude/embrace-gate-define-develop-2026-05-28.md (Finding SkillsModeManager fresh-install default: revert Auto→Approval (deferred from #1 PR#2 — needs test-env setup first) #4)