Add proposal conflict detector for review UI (#1022) by Chris0Jeky · Pull Request #1040 · Chris0Jeky/Taskdeck

Chris0Jeky · 2026-04-25T20:48:52Z

Summary

Add ConflictTone enum (Warn, Info, Ok) and ConflictRow value object to Domain layer
Add IProposalConflictDetector interface and ProposalConflictDetector service that analyzes proposals and produces tone-classified conflict rows
Add GET /api/automation/proposals/{id}/conflicts endpoint with authorization
Register service via DI in ApplicationServiceRegistration

Conflict detection rules

Warn scenarios:

Target card was modified since proposal was generated (stale data)
Target column is at WIP limit
Another pending proposal targets the same card
Proposal risk level is High or Critical
Target card no longer exists (deleted)

Info scenarios:

Board has active outbound webhooks that will fire
Target card has active comments/discussion
Multiple operations in the proposal affect the same card

Ok scenarios (positive signals):

No conflicts detected (single status row)
Target column has available capacity
Card data is current (fresh)

Tests

Domain: 12 tests for ConflictRow (construction, validation, equality, hashing)
Application: 31 tests for ProposalConflictDetector covering all warn/info/ok scenarios, authorization, sorting, combinations, edge cases (invalid GUIDs, null targets, malformed JSON), and expired proposal handling
Existing: All 27 existing ArchiveConflictDetector tests continue to pass

Adversarial self-review findings (fixed)

Positive signal tracking bug (fixed): Original implementation tried to detect already-flagged cards by searching for card IDs in warning message text. This silently failed because stale-data messages contain card titles, not IDs, causing fresh-data OK rows to appear alongside stale-data warnings for the same card. Fixed by using explicit HashSet<Guid> tracking for flagged card and column IDs.
Test timing sensitivity (fixed): Entity constructors set CreatedAt/UpdatedAt to DateTimeOffset.UtcNow, so creating a Card entity after a Proposal entity in test code would always make card.UpdatedAt > proposal.CreatedAt, falsely triggering stale-data warnings. Fixed by restructuring tests to create card entities before proposals.
Moq setup order sensitivity (fixed): SetupEmptySecondaryChecks used It.IsAny<string>() for GetLatestByOperationTargetAsync, which overrode specific card-ID setups made earlier. Fixed by inlining secondary check setup in the duplicate-proposal test.

Remaining risks (acceptable)

N+1 queries: The detector makes individual DB calls per card/column target. For proposals with many operations, this could be slow. Acceptable for now since proposals typically have 1-5 operations; batch loading can be added if needed.
Stale data comparison precision: The card.UpdatedAt > proposal.CreatedAt comparison uses DateTimeOffset with tick-level precision. In production, proposal creation and card fetching happen at clearly different times, so sub-tick races are not a concern.

Closes #1022

Test plan

dotnet build backend/Taskdeck.sln -c Release passes
dotnet test --filter "FullyQualifiedName~Conflict" -- all 147 tests pass (37 domain + 83 application + 27 api)
Existing archive conflict tests unaffected
CI pipeline passes

Part of #1022: tone-classified conflict rows for proposal review. ConflictTone has Warn/Info/Ok values mapping to frontend color semantics. ConflictRow is an immutable value object with Tone, Key, and Value.

Part of #1022: IProposalConflictDetector interface and implementation. Detects warn/info/ok conditions for proposals including stale data, WIP limits, duplicate proposals, high risk, webhooks, comments, and multi-op conflicts. Includes ConflictRowDto for API serialization.

Part of #1022: wire ProposalConflictDetector into API layer. New endpoint delegates to conflict detector with auth from JWT claims. DI registration added in ApplicationServiceRegistration.

Part of #1022: 12 tests covering construction validation, equality, hashing, toString, and invalid tone rejection.

Part of #1022: 31 tests covering all conflict detection scenarios. Tests: authorization (5), no-conflicts ok (1), stale data (2), WIP limit (2), duplicate proposals (2), high risk (4), webhooks (3), comments (2), multi-op (2), positive signals (2), sorting (1), combinations (1), expired proposals (1), edge cases (3).

Adversarial self-review fix: the original implementation tried to detect already-flagged cards by searching for card IDs in warning message text. This failed because stale-data messages contain card titles, not IDs. Now uses explicit HashSet<Guid> tracking for flagged card and column IDs.

Chris0Jeky · 2026-04-25T20:49:12Z

Adversarial Self-Review

Issues Found and Fixed

1. Positive signal tracking bug (FIXED in f1b407d)

What: AddPositiveSignalsAsync tried to detect already-flagged cards by searching for card IDs or formatted GUIDs within warning message text (e.g., staleCardKeys.Any(r => r.Value.Contains(cardId.ToString()))).
Why it failed: Stale-data warning messages contain card titles ("Card 'My Card' was modified..."), not card IDs. The text-based check never matched, so fresh-data OK rows appeared alongside stale-data warnings for the same card.
Fix: Replaced text-based detection with explicit HashSet<Guid> tracking for flagged card and column IDs, passed through the detection pipeline.

2. Test timing false positives (FIXED in 923e21c)

What: Tests creating Card entities after Proposal entities always triggered stale-data warnings because both constructors use DateTimeOffset.UtcNow, with the card's timestamp being nanoseconds later.
Fix: Restructured all tests to create card entities before proposals, ensuring card.UpdatedAt <= proposal.CreatedAt.

3. Moq setup order override (FIXED in 923e21c)

What: SetupEmptySecondaryChecks used It.IsAny<string>() for GetLatestByOperationTargetAsync, which silently overrode earlier specific card-ID setups (Moq uses last-matching-setup semantics).
Fix: Inlined the secondary check setup in the duplicate-proposal test, setting the specific mock last.

Authorization Review

Owner always has access (checked first, avoids unnecessary DB calls)
Non-owner with board-scoped proposal: checks CanReadBoardAsync
Non-owner without board scope: always denied
Non-owner with board access denied: returns 403

Graceful Degradation

Missing/deleted target cards: produces missing-target warning instead of failing
Invalid GUIDs in TargetId: silently skipped (tested)
Null TargetId: silently skipped (tested)
Malformed JSON in parameters: skipped with silent catch (tested)
No operations on proposal: returns clean Ok status row

Remaining Acceptable Risks

N+1 query pattern for per-card/column checks (acceptable for typical 1-5 operation proposals)
DateTimeOffset tick-precision comparison for stale data (not a concern in production where times are clearly separated)

gemini-code-assist

Code Review

This pull request introduces a conflict detection system for automation proposals, featuring a new API endpoint and a service that analyzes proposals for risks like stale data and WIP limit violations. Feedback identifies a logic error in target column extraction where operations lacking parameters are skipped and suggests caching entities to eliminate redundant database lookups within the detection process.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1b407db8c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Chris0Jeky · 2026-04-25T23:27:41Z

Adversarial Review: PR #1040 — All 5 bot findings addressed

Reviewed all changed files and addressed every bot comment. Fixes pushed.

Findings and fixes applied

1. HIGH (Gemini): Column detection skipped for parameterless ops (line 343)
Confirmed. The continue on string.IsNullOrWhiteSpace(op.Parameters) ran before the TargetType == "column" check, so column-targeted operations with no JSON parameters were silently dropped.
Fix: Moved the TargetType check above the parameters guard. Added test DetectConflictsAsync_ColumnTargetWithNoParameters_StillDetected.

2. MEDIUM (Gemini): Redundant entity fetching in AddPositiveSignalsAsync (line 277)
Confirmed. CheckStaleDataAsync, CheckWipLimitAsync, and AddPositiveSignalsAsync all independently fetched the same cards and columns from the DB.
Fix: Introduced Dictionary<Guid, Card?> and Dictionary<Guid, Column?> caches in DetectConflictsAsync, passed through to all sub-methods via GetOrFetchCardAsync / GetOrFetchColumnAsync helper methods. Each entity is now fetched at most once per detection run.

3. P1 (Codex): False warnings for create-card operations (line 327)
Confirmed. GetDistinctCardTargetIds included create operations that pre-assign IDs. This caused CheckStaleDataAsync to emit missing-target warnings for cards that don't exist yet (by design).
Fix: Added !op.ActionType.Equals("create", ...) filter to GetDistinctCardTargetIds. Added test DetectConflictsAsync_CreateCardWithPreAssignedId_NoStaleOrMissingWarning.

4. P2 (Codex): Only checks latest proposal for duplicates (line 177)
Confirmed. GetLatestByOperationTargetAsync returns a single proposal regardless of status. If the latest proposal targeting a card was already applied/rejected, an older pending proposal would be missed.
Fix: Added new GetPendingByOperationTargetAsync method to IAutomationProposalRepository (returns all pending proposals for a target), implemented in AutomationProposalRepository, and updated CheckDuplicatePendingProposalsAsync to use it. Updated all 4 test stubs.

5. P2 (Codex): Unsafe JSON parsing (line 360)
Confirmed. GetString() on a non-string JsonElement (e.g., "columnId": 12345) throws InvalidOperationException, which is not caught by catch (JsonException).
Fix: Added colProp.ValueKind == JsonValueKind.String guard before GetString() for both columnId and targetColumnId properties. Added test DetectConflictsAsync_NonStringColumnIdInJson_DoesNotThrow.

Verification

dotnet build backend/Taskdeck.sln -c Release — 0 errors
dotnet test --filter "FullyQualifiedName~Conflict" — 150 tests pass (37 Domain + 86 Application + 27 Api)

- Move TargetType check before Parameters guard in GetTargetColumnIds so column-targeted ops without parameters are detected - Add entity caches (cardCache, columnCache) to avoid redundant DB lookups across CheckStaleData, CheckWipLimit, and AddPositiveSignals - Exclude create actions from GetDistinctCardTargetIds to prevent false stale/missing warnings for not-yet-created cards - Add GetPendingByOperationTargetAsync to find ALL pending proposals for a target card, not just the latest - Check ValueKind before GetString() on JSON properties to avoid InvalidOperationException on non-string columnId values

New method returns all pending-review proposals targeting a given entity, replacing the single-result GetLatestByOperationTargetAsync for duplicate-proposal detection.

…itory Queries for all PendingReview proposals that have operations targeting a given entity type and ID. Filters by status at the DB level.

- Switch duplicate-proposal mocks to GetPendingByOperationTargetAsync returning lists instead of single entities - Add test for create-card operations not producing false warnings - Add test for column-targeted ops with no parameters being detected - Add test for non-string columnId in JSON not throwing

All IAutomationProposalRepository concrete fakes in Api.Tests now implement the new interface method (throws NotSupportedException).

Update STATUS.md, IMPLEMENTATION_MASTERPLAN.md, and TESTING_GUIDE.md to reflect the 10 Paper backend gap PRs (#1031-#1040, issues #1015-#1024): ~460 new backend tests, 10 new API endpoints, 3 EF Core migrations, and two rounds of adversarial review per PR.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d76ce47f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Chris0Jeky · 2026-04-26T20:06:47Z

Fresh Adversarial Review (Round 3)

IMPORTANT

1. CheckActiveCommentsAsync loads full entity graphs just to count (Confidence: 85)
GetByCardIdAsync includes AuthorUser + Mentions + MentionedUser, but detector only calls .Count(). For N cards, this materializes N full entity graphs unnecessarily.

Fix: Add CountByCardIdAsync to ICardCommentRepository with SELECT COUNT(*).

2. Authorization duplicated between controller and service (Confidence: 83)
Every other proposal endpoint uses controller's AuthorizeProposalAsync. This endpoint embeds authorization in the service's AuthorizeAccessAsync. Creates divergence risk if controller-level auth is updated.

Fix: Use AuthorizeProposalAsync at controller level, pass authorized proposal to detector.

3. WIP limit uses current state, not projected post-execution state (Confidence: 80)
Column at WipLimit - 1 with a proposal that adds a card: no warning raised even though execution will hit the limit. Acceptable for advisory system but worth documenting.

Verified Non-Issues

7 detection rules are sound and properly classified (Warn/Info/Ok)
Entity caching eliminates redundant DB lookups across sub-methods
Create-card false-positive fix verified correct
GetPendingByOperationTargetAsync properly queries ALL pending proposals
Safe JSON parsing with ValueKind guards
Security: functionally correct authorization
46 tests with good coverage

ProposalConflictDetector.CheckActiveCommentsAsync was calling GetByCardIdAsync which includes AuthorUser + Mentions + MentionedUser navigation properties, but only used .Count() on the result. For N cards this materialized N full entity graphs unnecessarily. Add CountByCardIdAsync(cardId) to ICardCommentRepository that executes SELECT COUNT(*) without includes, and use it in the conflict detector.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 390b966490

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Update STATUS.md, TESTING_GUIDE.md, IMPLEMENTATION_MASTERPLAN.md, MANUAL_TEST_CHECKLIST.md, GOLDEN_PRINCIPLES.md, and ISSUE_EXECUTION_GUIDE.md to reflect round 3 adversarial review findings and fixes across all 10 Paper backend gap PRs (#1031-#1040): 3 critical bugs fixed (seal-day race, tomorrow-note phantom data, card-history case mismatch), 8 important issues resolved, test counts updated (~460→~465), manual test checklist expanded with Today dossier and Review deep-dive API spot checks.

Resolve conflicts from #1032 (streak), #1033 (side-effects), #1038 (similar-past) merges: - AutomationProposalsController: keep conflicts endpoint alongside side-effects, diff, and similar-past endpoints; constructor accepts all services - ApplicationServiceRegistration: register both ProposalConflictDetector and SideEffectAnalyzer - WorkerResilienceTests: fake repo implements both GetPendingByOperationTargetAsync and GetTerminalByActionTypeAsync - smoke.spec.ts: take main's more robust addColumn helper (waits for API response and dialog close)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12d09c9594

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Separate mangled GetProposalConflicts and GetProposalHistory methods into distinct endpoints.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e55fc6b38b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d97861d1b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40dba4f847

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62f202ce13

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Chris0Jeky added 6 commits April 25, 2026 21:34

Add ConflictTone enum and ConflictRow value object to Domain layer

8f21613

Part of #1022: tone-classified conflict rows for proposal review. ConflictTone has Warn/Info/Ok values mapping to frontend color semantics. ConflictRow is an immutable value object with Tone, Key, and Value.

Add GET /api/automation/proposals/{id}/conflicts endpoint

e116daf

Part of #1022: wire ProposalConflictDetector into API layer. New endpoint delegates to conflict detector with auth from JWT claims. DI registration added in ApplicationServiceRegistration.

Add ConflictRow domain tests

de0ac4d

Part of #1022: 12 tests covering construction validation, equality, hashing, toString, and invalid tone rejection.

github-project-automation Bot added this to Taskdeck Execution Apr 25, 2026

github-project-automation Bot moved this to Pending in Taskdeck Execution Apr 25, 2026

gemini-code-assist Bot reviewed Apr 25, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs

chatgpt-codex-connector Bot reviewed Apr 25, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Chris0Jeky added 5 commits April 26, 2026 00:27

Add GetPendingByOperationTargetAsync to IAutomationProposalRepository

03f0bb6

New method returns all pending-review proposals targeting a given entity, replacing the single-result GetLatestByOperationTargetAsync for duplicate-proposal detection.

Implement GetPendingByOperationTargetAsync in AutomationProposalRepos…

43872fb

…itory Queries for all PendingReview proposals that have operations targeting a given entity type and ID. Filters by status at the DB level.

Add GetPendingByOperationTargetAsync stubs to test repository fakes

6d76ce4

All IAutomationProposalRepository concrete fakes in Api.Tests now implement the new interface method (throws NotSupportedException).

chatgpt-codex-connector Bot reviewed Apr 25, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

chatgpt-codex-connector Bot reviewed Apr 26, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Chris0Jeky added 2 commits April 27, 2026 00:25

Normalize line endings to match .gitattributes

8ef4af8

github-advanced-security AI found potential problems Apr 26, 2026

View reviewed changes

Comment thread frontend/taskdeck-web/scripts/demo-director.mjs Fixed

chatgpt-codex-connector Bot reviewed Apr 26, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs

Comment thread backend/src/Taskdeck.Infrastructure/Repositories/AutomationProposalRepository.cs

Merge origin/main into paper/1022-conflicts

e55fc6b

Separate mangled GetProposalConflicts and GetProposalHistory methods into distinct endpoints.

chatgpt-codex-connector Bot reviewed Apr 26, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Infrastructure/Repositories/AutomationProposalRepository.cs Outdated

Comment thread backend/src/Taskdeck.Infrastructure/Repositories/CardCommentRepository.cs Outdated

Address proposal conflict review findings

d97861d

chatgpt-codex-connector Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Align conflict webhook edge cases

40dba4f

chatgpt-codex-connector Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Chris0Jeky added 2 commits April 27, 2026 01:55

Project proposal WIP conflict checks

c6f6927

Launch demo director Playwright without shell

62f202c

chatgpt-codex-connector Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Comment thread backend/src/Taskdeck.Application/Services/ProposalConflictDetector.cs Outdated

Refine projected WIP conflict detection

1d75ae9

Conversation

Chris0Jeky commented Apr 25, 2026

Summary

Conflict detection rules

Tests

Adversarial self-review findings (fixed)

Remaining risks (acceptable)

Test plan

Uh oh!

Chris0Jeky commented Apr 25, 2026

Adversarial Self-Review

Issues Found and Fixed

Authorization Review

Graceful Degradation

Remaining Acceptable Risks

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Chris0Jeky commented Apr 25, 2026

Adversarial Review: PR #1040 — All 5 bot findings addressed

Findings and fixes applied

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Chris0Jeky commented Apr 26, 2026

Fresh Adversarial Review (Round 3)

IMPORTANT

Verified Non-Issues

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!