fix(search): explicit .Index() on IndexAsync so SUT writes don't bleed into the fixture index

[ANcpLua]

Summary

Follow-up to #23. Even after the ES TLS / parallel-algorithm / refresh fixes landed, MultipleDocuments_SearchCorrectly kept failing on CI with Expected collection to contain a single item, but the collection is empty. — same place every time.

Root cause: SearchIndexService.IndexDocumentAsync built its IndexRequestDescriptor without an explicit .Index(...), so the actual write target was whatever DefaultIndex the injected ElasticsearchClient was configured with — not the index the service's own ElasticsearchOptions point to.

In production this is a no-op (one client, one default, same value as options.Value.DefaultIndex). It is not a no-op when a test (here SearchIndexConcurrencyTests) builds a SUT with its own options but reuses the shared fixture's ElasticsearchClient. The SUT's InitializeAsync correctly creates its per-test index with the keyword/text/date mapping; then IndexAsync silently writes to the fixture's default index instead.

Alphabetical run order is GenAiIntegrationTests → OcrIntegrationTests → SearchIndexConcurrencyTests → SearchIndexIntegrationTests …. SearchIndexConcurrencyTests's SUT was the first writer to the fixture index. ES auto-created that index with dynamic mapping (id as text+.keyword subfield) before SearchIndexIntegrationTests.MultipleDocuments_SearchCorrectly got a chance to call its own InitializeAsync — which then short-circuited on ExistsAsync==true and never applied the proper p.Keyword("id") mapping. The downstream Filter(f => f.Term(t => t.Field(\"id\").Value(helloId.ToString()))) ran against a tokenised text field, never matched, and the test failed at the ContainSingle() assertion.

Fix

• PaperlessServices/Features/OcrProcessing/Infrastructure/Search/SearchIndexService.cs: add .Index(options.Value.DefaultIndex) so writes target the service-owned index, not the client default.
• PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs: the secondary assertion in InitializeAsync_WhenIndexPreCreated_SkipsCreateAndLogsNothing was relying on the bug (the doc landing in the fixture index so fixture.WaitForDocumentAsync could find it). With the production fix the doc now lives in the SUT's pre-created index, so the assertion now queries that index directly via ElasticClient.GetAsync(... g => g.Index(indexName) ...). The primary assertion (count of Created Elasticsearch index log lines) is unchanged.

Verification

• Local 13/13 PaperlessServices.Tests.Integration pass.
• This is the correctness fix; the earlier fix(tests): unblock CI by stopping Testcontainers' ES wait from probing HTTPS #23 commits (TLS off, conservative parallel, refresh, 30s budget) are still useful defence-in-depth on slow CI runners but the test now passes without needing the extra timeout headroom.

Test plan

• Local integration suite green
• CI backend job green (the only previously-flaky test should now pass)

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Tests

  • Improved indexing validation to verify documents land in the intended index.
  • Increased search polling timeout and made polling/cancellation more robust.
  • Fixed test environment startup hangs by disabling TLS probing for the test search service.
  • Hardened test teardown to avoid null-reference masking of initialization failures and to dispose containers more safely.

• Chores

  • Switched test runner parallelization strategy to be more conservative.
  • Updated test Elasticsearch image and ensured indexing explicitly targets the configured index.

Walkthrough

Service indexing now explicitly targets the configured default Elasticsearch index. Tests use per-test index verification. Test containers pinned to ES 9.1.3 with HTTP TLS disabled. Fixture disposal made best-effort; search polling adds upfront refresh, 30s overall timeout, and improved cancellation handling. xUnit runner set to conservative algorithm.

Changes

Elasticsearch reliability and index targeting

Layer / File(s)	Summary
Explicit index targeting in indexing request PaperlessServices/Features/OcrProcessing/Infrastructure/Search/SearchIndexService.cs	SearchIndexService.IndexDocumentAsync now explicitly sets the target index via .Index(options.Value.DefaultIndex) in the IndexAsync request.
Test verification of per-test index targeting PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs	InitializeAsync_WhenIndexPreCreated_SkipsCreateAndLogsNothing verifies indexing against the per-test index by using ElasticClient.GetAsync with explicit Index(indexName) instead of fixture default lookup.
Container image pinning and runner tuning .env.test, PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs, PaperlessServices.Tests/xunit.runner.json, PaperlessServices.Tests/Integration/WorkerTestBase.cs	ELASTIC_IMAGE fallback and .env.test updated to docker.elastic.co/elasticsearch/elasticsearch:9.1.3. xUnit parallelAlgorithm changed from aggressive to conservative.
Disable HTTP TLS in test container PaperlessServices.Tests/Integration/WorkerTestBase.cs	Adds container env xpack.security.http.ssl.enabled = false to avoid TLS probing when Elasticsearch serves plain HTTP.
Fixture disposal robustness PaperlessServices.Tests/Integration/WorkerTestBase.cs	DisposeAsync guards _host nullability and wraps Stop/Dispose calls in best-effort handling to avoid masking initialization errors.
Search polling and refresh robustness PaperlessServices.Tests/Integration/WorkerTestBase.cs	WaitForSearchResultsAsync<T>: 30s overall timeout with linked cancellation token, upfront Indices.RefreshAsync, reworked polling loop handling cancellation during search vs delay, final attempt uses caller token.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• ANcpLua/Paperless#23: Overlaps with test-fixture and polling/cancellation changes in WorkerTestBase.cs.

Suggested labels

area:infra

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and accurately describes the main fix: explicitly setting the index on IndexAsync to prevent writes from targeting the wrong index.
Description check	✅ Passed	The description thoroughly explains the root cause, the reproduction chain (alphabetical test order, dynamic mapping auto-creation, skipped proper mapping), and the fix applied.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ci-unblock-orphan-tests-xmlcontent

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/ci-unblock-orphan-tests-xmlcontent

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR fixes Elasticsearch indexing so SearchIndexService writes to the index configured in its own options instead of relying on the injected client’s default index, preventing test index contamination and mapping drift.

Changes:

• Adds an explicit .Index(options.Value.DefaultIndex) to document indexing.
• Updates the concurrency test to query the SUT-specific index directly.
• Includes integration-test stability updates for Elasticsearch container TLS waiting, xUnit scheduling, and search polling.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
PaperlessServices/Features/OcrProcessing/Infrastructure/Search/SearchIndexService.cs	Ensures indexed documents are written to the service-configured Elasticsearch index.
PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs	Adjusts the assertion to read from the per-test index after explicit indexing is fixed.
PaperlessServices.Tests/Integration/WorkerTestBase.cs	Adds Elasticsearch TLS env configuration and makes search polling more resilient in CI.
PaperlessServices.Tests/xunit.runner.json	Switches xUnit parallel scheduling from aggressive to conservative.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 8 complexity · -2 duplication

Metric	Results
Complexity	8
Duplication	-2

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

The PR successfully addresses index isolation issues by ensuring SearchIndexService uses the configured index rather than the client default. This fix, combined with the SSL configuration for Testcontainers and increased timeouts, significantly improves the reliability of the integration test suite. Codacy analysis indicates the changes are up to standards.

However, a logic gap remains in the search helper within WorkerTestBase.cs. While the PR enables custom index names to prevent data bleeding, the helper still targets the DefaultIndex during explicit refreshes. This should be addressed to ensure that tests using non-default indices remain consistent in restricted I/O environments like CI.

About this PR

• The systemic improvements to CI stability—specifically the manual container health check fixes and the 'final attempt' polling pattern—are high-quality additions that will reduce transient test failures.

Test suggestions

• Verify IndexDocumentAsync targets the index name provided in ElasticsearchOptions rather than the client default
• Verify SearchIndexConcurrencyTests queries the correct non-default index during verification
• Verify Testcontainers setup explicitly disables http.ssl to facilitate container health checks
• Verify the search wait-helper in WorkerTestBase implements a 30s timeout and idempotent index refresh

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: The explicit refresh targets the client's DefaultIndex. Since a primary goal of this PR is to ensure the System Under Test (SUT) can use specific index names to avoid bleeding into the shared fixture index, the DefaultIndex might not be the index where the document was just written. Using Indices.All ensures that the defensive refresh applies to all indices, covering cases where the test uses a custom index name.

Suggested change

	r => r.Indices(client.ElasticsearchClientSettings.DefaultIndex),
	await client.Indices.RefreshAsync(
	r => r.Indices(Indices.All),
	overallLinked.Token);

[codacy-production]

_{⚪ LOW RISK}

Nitpick: This 'final attempt' pattern is excellent for test reliability. By performing one last search using the caller's original cancellationToken after the internal polling loop finishes, you ensure that timeout failures provide specific details about what was missing rather than throwing an opaque OperationCanceledException.

[codacy-production]

_{⚪ LOW RISK}

Nitpick: Adding xpack.security.http.ssl.enabled set to false is an important fix. Without this, Testcontainers often defaults to an HTTPS health check even when xpack.security.enabled is false, which can cause the container startup to hang indefinitely in CI environments.

[coderabbitai]

♻️ Duplicate comments (1)

PaperlessServices.Tests/Integration/WorkerTestBase.cs (1)

215-217: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Defensive refresh may target the wrong index.

The refresh targets client.ElasticsearchClientSettings.DefaultIndex, but WaitForSearchResultsAsync accepts an arbitrary configureSearch action that can target any index. Given this PR's goal of allowing the SUT to use specific index names separate from the shared fixture index, if the caller searches a non-default index, the defensive refresh will not affect the searched index.

♻️ Proposed fix

 			await client.Indices.RefreshAsync(
-				r => r.Indices(client.ElasticsearchClientSettings.DefaultIndex),
+				r => r.Indices(Indices.All),
 				overallLinked.Token);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs` around lines 215 -
217, The defensive refresh in WaitForSearchResultsAsync currently hardcodes
client.ElasticsearchClientSettings.DefaultIndex when calling
client.Indices.RefreshAsync but the configureSearch action can target any index;
update WaitForSearchResultsAsync so the refresh targets the same index(es) as
the search: either extract the Indices from the SearchDescriptor built by the
configureSearch action and pass those into client.Indices.RefreshAsync, or (if
extraction is impractical) use r => r.Indices(Indices.All) so the refresh covers
the searched index; ensure you modify the RefreshAsync call (the lambda
currently using DefaultIndex) so it uses the extracted indices or Indices.All
instead.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs`:
- Around line 215-217: The defensive refresh in WaitForSearchResultsAsync
currently hardcodes client.ElasticsearchClientSettings.DefaultIndex when calling
client.Indices.RefreshAsync but the configureSearch action can target any index;
update WaitForSearchResultsAsync so the refresh targets the same index(es) as
the search: either extract the Indices from the SearchDescriptor built by the
configureSearch action and pass those into client.Indices.RefreshAsync, or (if
extraction is impractical) use r => r.Indices(Indices.All) so the refresh covers
the searched index; ensure you modify the RefreshAsync call (the lambda
currently using DefaultIndex) so it uses the extracted indices or Indices.All
instead.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4b459228-515a-4da9-8fa4-f424bfcee38a

📥 Commits

Reviewing files that changed from the base of the PR and between 910431b and 29584ce.

📒 Files selected for processing (3)

• .env.test
• PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs
• PaperlessServices.Tests/Integration/WorkerTestBase.cs

📜 Review details

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Build & Test (backend)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.cs

📄 CodeRabbit inference engine (.editorconfig)

**/*.cs: Do not qualify event, field, method, and property access with 'this.' in C# code
Use predefined types (like int, string) instead of BCL types (like Int32, String) in C#
Always use parentheses for clarity in arithmetic and relational binary operators in C#
Require explicit accessibility modifiers for non-interface members in C#
Prefer coalesce expressions, collection initializers, null propagation, and object initializers in C#
Prefer auto-properties and compound assignments in C#
Prefer conditional expressions over assignment and return statements in C#
Mark fields as readonly when possible in C#
Treat all unused parameters as code quality issues in C#
Use explicit types instead of 'var' in C# unless type is apparent
Prefer expression-bodied members for accessors, indexers, lambdas, methods, and properties in C#
Use pattern matching instead of 'as' with null checks and 'is' with cast checks in C#
Prefer switch expressions over traditional switch statements in C#
Use conditional delegate calls in C# to avoid null reference exceptions
Prefer static local functions over instance local functions in C#
Use modifier order: public, private, protected, internal, file, static, extern, new, virtual, abstract, sealed, override, readonly, unsafe, required, volatile, async in C#
Always use braces for code blocks in C#
Prefer file-scoped namespaces in C#
Prefer implicit object creation when type is apparent in C#
Prefer index and range operators in C#
Place 'using' directives outside of namespace declarations in C#
Always place opening brace on new line before catch, else, finally, and members in C#
Indent block contents and case contents in C#
Add space after comma, before and after binary operators, and after colon in inheritance clause in C#
Do not add space after dot, cast, or before open square brackets in C#
Name interfaces with 'I' prefix using PascalCase in C#
Name types (classes, structs, enums) using PascalCase in C#
Name non-field members (properties, events...

Files:

• PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs
• PaperlessServices.Tests/Integration/WorkerTestBase.cs

PaperlessServices.Tests/Integration/**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

Mock ITextSummarizer in integration tests with FakeTextSummarizer; do not use the real Gemini API even with placeholder keys

Files:

• PaperlessServices.Tests/Integration/WorkerTestBase.cs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs`:
- Around line 122-127: The cleanup currently calls _host.Dispose() without a
best-effort guard, which can throw and mask earlier InitializeAsync failures;
wrap the Dispose call in a try/catch (similar to the existing try for await
_host.StopAsync()) so any exception thrown by _host.Dispose() is swallowed and
does not overwrite the original exception — locate the block referencing _host,
StopAsync and Dispose and surround the Dispose invocation with a try {
_host.Dispose(); } catch { /* swallow to avoid masking init error */ }.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3946eac6-bff2-487b-9923-10319606f0cd

📥 Commits

Reviewing files that changed from the base of the PR and between 29584ce and 87da734.

📒 Files selected for processing (1)

• PaperlessServices.Tests/Integration/WorkerTestBase.cs

📜 Review details

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: copilot-pull-request-reviewer
• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Build & Test (backend)

🧰 Additional context used

📓 Path-based instructions (2)

PaperlessServices.Tests/Integration/**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

Mock ITextSummarizer in integration tests with FakeTextSummarizer; do not use the real Gemini API even with placeholder keys

Files:

• PaperlessServices.Tests/Integration/WorkerTestBase.cs

**/*.cs

📄 CodeRabbit inference engine (.editorconfig)

**/*.cs: Do not qualify event, field, method, and property access with 'this.' in C# code
Use predefined types (like int, string) instead of BCL types (like Int32, String) in C#
Always use parentheses for clarity in arithmetic and relational binary operators in C#
Require explicit accessibility modifiers for non-interface members in C#
Prefer coalesce expressions, collection initializers, null propagation, and object initializers in C#
Prefer auto-properties and compound assignments in C#
Prefer conditional expressions over assignment and return statements in C#
Mark fields as readonly when possible in C#
Treat all unused parameters as code quality issues in C#
Use explicit types instead of 'var' in C# unless type is apparent
Prefer expression-bodied members for accessors, indexers, lambdas, methods, and properties in C#
Use pattern matching instead of 'as' with null checks and 'is' with cast checks in C#
Prefer switch expressions over traditional switch statements in C#
Use conditional delegate calls in C# to avoid null reference exceptions
Prefer static local functions over instance local functions in C#
Use modifier order: public, private, protected, internal, file, static, extern, new, virtual, abstract, sealed, override, readonly, unsafe, required, volatile, async in C#
Always use braces for code blocks in C#
Prefer file-scoped namespaces in C#
Prefer implicit object creation when type is apparent in C#
Prefer index and range operators in C#
Place 'using' directives outside of namespace declarations in C#
Always place opening brace on new line before catch, else, finally, and members in C#
Indent block contents and case contents in C#
Add space after comma, before and after binary operators, and after colon in inheritance clause in C#
Do not add space after dot, cast, or before open square brackets in C#
Name interfaces with 'I' prefix using PascalCase in C#
Name types (classes, structs, enums) using PascalCase in C#
Name non-field members (properties, events...

Files:

• PaperlessServices.Tests/Integration/WorkerTestBase.cs

🔇 Additional comments (4)

PaperlessServices.Tests/Integration/WorkerTestBase.cs (4)

220-229: Refresh targets only the fixture's DefaultIndex.

The configureSearch delegate may target a different index (e.g., a per-test SUT index). Refreshing only DefaultIndex won't make documents in other indices searchable. Use Indices.All for the defensive refresh.

---

25-25: LGTM!

---

35-38: LGTM!

---

202-211: LGTM!

Also applies to: 231-259

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

_host.Dispose() not wrapped in best-effort handler.

The comment on lines 117-121 states the intent to "swallow per-container Dispose failures" to avoid masking init errors. StopAsync is wrapped but Dispose() is not. If Dispose() throws (e.g., a hosted service's disposal fails), it masks the original init failure exactly as described.

Proposed fix

 if (_host is not null)
 {
     try { await _host.StopAsync(); }
     catch { /* best-effort: don't mask the InitializeAsync exception */ }
-    _host.Dispose();
+    try { _host.Dispose(); }
+    catch { /* best-effort */ }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@PaperlessServices.Tests/Integration/WorkerTestBase.cs` around lines 122 -
127, The cleanup currently calls _host.Dispose() without a best-effort guard,
which can throw and mask earlier InitializeAsync failures; wrap the Dispose call
in a try/catch (similar to the existing try for await _host.StopAsync()) so any
exception thrown by _host.Dispose() is swallowed and does not overwrite the
original exception — locate the block referencing _host, StopAsync and Dispose
and surround the Dispose invocation with a try { _host.Dispose(); } catch { /*
swallow to avoid masking init error */ }.