TST-36: Golden path integration test for capture-to-board pipeline

[Chris0Jeky]

Summary

• Adds CaptureToBoardGoldenPathIntegrationTests — 7 integration tests exercising the full capture → triage → proposal → review → board mutation pipeline
• Uses TestWebApplicationFactory with real SQLite and Mock LLM provider for deterministic behavior
• Tests cover: happy path, multi-operation, rejection, cross-user isolation, audit trail, provenance integrity, and triage failure

Closes #703

Test plan

• All 7 new tests pass locally
• Full backend test suite passes (2361 tests, 0 failures)
• Tests use the same TestWebApplicationFactory infrastructure as existing API tests
• No flaky timing issues — uses established PollUntilAsync helper with 40 attempts x 250ms

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Adversarial Self-Review

Does the test actually prove the full pipeline?

Yes, with caveats. The happy path test (HappyPath_CaptureTriageApproveExecute_ShouldCreateCardOnBoard) exercises every step: capture creation -> triage enqueue -> worker processing -> proposal generation -> proposal review -> approval -> execution -> board card creation -> capture status transition to Converted. It verifies the card title matches the capture text and the board ID is correct.

Gaps identified

1. No proposal expiry test — The issue requested a test for expired proposals. This was omitted because the ProposalHousekeepingWorker timing is hard to control deterministically in a shared IClassFixture<TestWebApplicationFactory> without fast-forwarding clocks. The expired proposal UX is already covered by dedicated tests in AutomationProposalsApiTests and the #678/#690 PR.

2. No retry-on-failure test — The issue requested testing capture triage retry behavior. This is already covered by the existing Triage_ShouldPersistRedactedFailureMessage_WhenWorkerFailureContainsSensitiveDetails test in CaptureApiTests which uses a custom SensitiveFailureCaptureWebApplicationFactory. Adding another here would duplicate coverage.

3. Unused imports — System.Text.Json is imported but not used in the test file. Minor.

4. user variable unused in some tests — In MultiOperation_CaptureWithMultipleTasks_ShouldCreateMultipleCards, the user variable is assigned but never referenced. Same in CrossUserIsolation for userB. Harmless but slightly noisy.

Is the setup realistic?

Yes. The test uses the same TestWebApplicationFactory with real SQLite, real EF Core migrations, and the Mock LLM provider that all other API tests use. The worker actually processes the queue items asynchronously, and the test waits via polling — exactly how the real system behaves.

Edge cases covered

• Multi-operation atomicity (3 cards from one capture)
• Rejection leaves board empty
• Cross-user isolation (read, approve, execute all blocked)
• Audit trail records card creation
• Provenance chain verified at API and persistence layers
• Triage failure for missing board

Verdict

The core pipeline coverage is solid. The missing expiry and retry scenarios are already tested elsewhere and would add complexity without meaningful new coverage. No action items.

[gemini-code-assist]

Code Review

This pull request introduces a new integration test suite, CaptureToBoardGoldenPathIntegrationTests, which validates the end-to-end pipeline from item capture to board mutation. The tests cover various scenarios including successful conversions, multi-card creation, proposal rejections, and security isolation. Review feedback suggests refactoring the test setup logic into helper methods to reduce code duplication and refining audit log assertions for better precision.

[gemini-code-assist]

There is a lot of repeated code for setting up a user, board, and column across multiple tests in this file (e.g., HappyPath..., MultiOperation..., Rejection...). To improve maintainability and reduce duplication, consider extracting this setup logic into a private helper method.

For example, you could create a helper method that takes an HttpClient and returns the created BoardDto:

private async Task<BoardDto> CreateBoardWithColumnAsync(HttpClient client, string boardName)
{
    var board = await ApiTestHarness.CreateBoardAsync(client, boardName);
    var columnResponse = await client.PostAsJsonAsync(
        $`/api/boards/{board.Id}/columns`,
        new CreateColumnDto(board.Id, "Backlog", null, null));
    columnResponse.StatusCode.Should().Be(HttpStatusCode.Created);
    return board;
}

Then, the setup in each test would be simplified to:

using var client = _factory.CreateClient();
await ApiTestHarness.AuthenticateAsync(client, "golden-happy");
var board = await CreateBoardWithColumnAsync(client, "golden-happy-board");

[gemini-code-assist]

The current assertion Should().Contain(...) checks that at least one audit log for card creation exists. Since this test creates exactly one card, the assertion can be made more specific to ensure that exactly one such audit log is created. This will make the test more robust against regressions that might cause duplicate logs.

Using ContainSingle from FluentAssertions would be more precise here.

        auditLogs!.Should().ContainSingle(log =>
            string.Equals(log.EntityType, "card", StringComparison.OrdinalIgnoreCase) &&
            log.Action == AuditAction.Created);

[Copilot]

Pull request overview

Adds a new “golden path” integration test suite that exercises the full capture → triage → proposal → review/approval → execution → board mutation workflow end-to-end using TestWebApplicationFactory with SQLite + Mock LLM for deterministic behavior.

Changes:

• Introduces CaptureToBoardGoldenPathIntegrationTests with 7 end-to-end integration tests covering happy path, multi-op execution, rejection, cross-user isolation, audit logging, provenance integrity, and deterministic triage failure.
• Adds polling helper usage to reliably await async worker processing and capture state transitions.
• Validates persisted provenance linkage by reading TaskdeckDbContext payloads for consistency.

---

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

[Copilot]

System.Text.Json is not used in this test file. Removing the unused using will avoid compiler warnings and keep imports minimal.

[Copilot]

The triage request response is not asserted here. If the endpoint returns a non-2xx status (auth issue, validation change, etc.), the subsequent poll will time out and the failure will be harder to diagnose. Consider asserting the triage response status code (and optionally body) like the other integration tests do.

Suggested change

	await client.PostAsync($"/api/capture/items/{capture!.Id}/triage", null);
	var triageResponse = await client.PostAsync($"/api/capture/items/{capture!.Id}/triage", null);
	triageResponse.IsSuccessStatusCode.Should().BeTrue();

[Copilot]

The approve/execute calls aren’t asserting their HTTP status codes or parsing the response. Adding explicit status assertions (and optionally validating the returned proposal status) will make failures more actionable and consistent with the rest of the test suite.

Suggested change

	await client.PostAsync($"/api/automation/proposals/{proposalId}/approve", null);
	var executeRequest = new HttpRequestMessage(HttpMethod.Post, $"/api/automation/proposals/{proposalId}/execute");
	executeRequest.Headers.Add("Idempotency-Key", Guid.NewGuid().ToString());
	await client.SendAsync(executeRequest);
	var approveResponse = await client.PostAsync($"/api/automation/proposals/{proposalId}/approve", null);
	approveResponse.StatusCode.Should().Be(HttpStatusCode.OK);
	var executeRequest = new HttpRequestMessage(HttpMethod.Post, $"/api/automation/proposals/{proposalId}/execute");
	executeRequest.Headers.Add("Idempotency-Key", Guid.NewGuid().ToString());
	var executeResponse = await client.SendAsync(executeRequest);
	executeResponse.StatusCode.Should().Be(HttpStatusCode.OK);

[Chris0Jeky]

Adversarial Review (Round 2)

1. Missing column placement assertion (medium)

File: CaptureToBoardGoldenPathIntegrationTests.cs, lines ~93-96 (happy path card assertions)

CardDto exposes a ColumnId property (see CardDto.cs:8), but after the full pipeline executes, the test only asserts Title and BoardId:

cards[0].Title.Should().Be("Fix login bug");
cards[0].BoardId.Should().Be(board.Id);

It never checks that the card was placed in the expected column (the "Backlog" column created at the start). This is a meaningful gap: the mock LLM provider could produce a proposal targeting a non-existent column or the wrong column, and the test would still pass. The column assertion should be present for both the happy path and the multi-operation test.

2. ProvenanceIntegrity test queries internal persistence layer (low-medium)

File: CaptureToBoardGoldenPathIntegrationTests.cs, lines ~331-345

The ProvenanceIntegrity_BackwardTraceable_CardToProposalToCapture test accesses TaskdeckDbContext directly to query db.LlmRequests and then calls CaptureRequestContract.ParsePayload(). This is reaching below the API layer into infrastructure internals:

using var scope = _factory.Services.CreateScope();
var db = scope.ServiceProvider.GetRequiredService<TaskdeckDbContext>();
var persistedItem = await db.LlmRequests.SingleAsync(r => r.Id == capture.Id);

While this proves persistence-level provenance integrity, it couples the test to the internal DB schema. If LlmRequests is ever renamed or the capture-to-LlmRequest ID mapping changes, this test breaks for structural reasons, not behavioral ones. Consider whether the API-level provenance assertions (which are already thorough) are sufficient, or add a comment acknowledging the intentional coupling.

3. IClassFixture sharing means shared database across all 7 tests (low)

All 7 tests share a single TestWebApplicationFactory instance (via IClassFixture), which means they share one SQLite database. Each test creates users and boards with unique names (via Guid suffixes in AuthenticateAsync), so they won't collide on identity. However:

• The CrossUserIsolation test could potentially be affected if another test's async worker is still running and has mutated shared state.
• The audit trail test (AuditTrail_ShouldRecordBoardMutationAfterExecution) uses Should().Contain(...) rather than exact-match, which correctly handles the shared DB. Good.
• The TriageFailure_NoBoardId_ShouldFailDeterministically test polls for Failed status. If the worker is under load from other tests' triage requests, the 40-attempt / 250ms timeout (10 seconds) could be tight on slow CI runners.

This is not a bug, but the shared state deserves a brief class-level comment noting that test isolation relies on per-test unique users/boards.

4. Multi-operation test does not verify card-to-column assignment or card ordering (low)

File: CaptureToBoardGoldenPathIntegrationTests.cs, lines ~148-153

cards.Select(c => c.Title).Should().BeEquivalentTo(
    new[] { "Implement user authentication", "Write API integration tests", "Update deployment docs" });

BeEquivalentTo is order-insensitive, which is fine for titles. But like finding #1, ColumnId is not checked. Additionally, card Position values are not validated -- the mock provider might assign arbitrary positions.

5. WaitForCaptureStatusAsync early-exit on Failed status (acceptable but notable)

File: CaptureToBoardGoldenPathIntegrationTests.cs, lines ~387-389

item => item.Status == expectedStatus ||
        (item.Status == CaptureStatus.Failed && expectedStatus != CaptureStatus.Failed),

This early-exits polling if the status becomes Failed when we were waiting for something else. This is good defensive behavior -- it prevents 10-second hangs when triage fails unexpectedly. But it means if a worker bug causes sporadic Failed status, the test error message will say "did not complete" rather than "unexpectedly failed". Consider logging or asserting on the ErrorMessage field in the failure diagnostics lambda.

6. Rejection test does not verify capture item status after rejection (low)

File: CaptureToBoardGoldenPathIntegrationTests.cs, lines ~175-190

The Rejection_ShouldLeaveBoardUnchanged test verifies the board is empty and the proposal is Rejected, but does not check what happened to the capture item itself. After a proposal rejection, the capture item should presumably revert to a specific status (e.g., remain at ProposalCreated or transition to some other state). Asserting on this would strengthen the test.

7. No double-execute idempotency assertion (nice-to-have)

The happy path test sends a single execute request with an Idempotency-Key. But it never sends a second execute with the same key to verify idempotent behavior. This is arguably a separate concern, but since the idempotency key is already part of the test setup, a follow-up assertion would be low-cost.

Summary

The test suite is well-structured and covers the core pipeline thoroughly. The most actionable finding is #1 (missing column assertion) -- without it, the test proves a card was created but not that it was placed correctly on the board. Findings #2-#7 range from structural observations to nice-to-have improvements.

CI is green. The self-review comment already acknowledged the absence of expiry and retry tests with reasonable justification.

[Chris0Jeky]

Adversarial Review - Final Status

Action taken

Pushed commit 141f7ab4 to address finding #1 (missing column assertion):

• Happy path test: Now captures ColumnDto from column creation response and asserts cards[0].ColumnId.Should().Be(column!.Id) to verify the card lands in the correct Backlog column.
• Multi-operation test: Same column capture plus cards.Should().OnlyContain(c => c.ColumnId == column!.Id) to verify all 3 cards are placed in the Backlog column.

This is meaningful because the triage service deterministically selects the first column by position. Without this assertion, a bug in column targeting would pass silently.

Findings not acted on (acceptable as-is)

• Add testing readiness review and action plan #2 (DB-layer provenance query): Intentional coupling for deep provenance verification; already noted in self-review.
• Add comprehensive Application layer test suite #3 (shared fixture DB): Test isolation via unique users/boards is sound.
• Add label validation coverage for CardService #4 (card position ordering): Order-insensitive title check is appropriate since mock provider position assignment is non-deterministic.
• Review recent changes and documentation #5 (Failed status early-exit): Defensive behavior works correctly; diagnostics lambda already logs status.
• Add PR review notes #6 (rejection capture status): Would strengthen the test but rejection→capture status transition is a separate concern from the golden path.
• Claude/review changes docs 01 djam aovn k6 c2jye s2apiih #7 (idempotency double-execute): Belongs in a dedicated idempotency test, not the golden path.

Bot comments reviewed

• Codex: Hit usage limits, no findings.
• Gemini Code Assist: Suggested helper method refactoring and tighter audit assertions -- cosmetic.
• Copilot: Overview only, no actionable findings.

CI status

CI triggered on new commit, awaiting results. Previous run was all green.