feat(tests): disputed state + fix timestamps on reopen
Some checks failed
Aegis CI / lint-and-test (push) Has been cancelled
Some checks failed
Aegis CI / lint-and-test (push) Has been cancelled
1. New 'disputed' state — one lead approved, the other rejected:
- Both approved → validated (unchanged)
- Both rejected → rejected (unchanged)
- One approves + one rejects → disputed (new)
- DB: ALTER TYPE teststate ADD VALUE 'disputed'
- Notification sent to the approving lead explaining the conflict
with the rejection notes
2. Disputed UI in TestDetailHeader:
- Amber banner showing conflict + rejection reason from notes
- 'Change Vote to Rejected' button for the lead who approved
- Validation indicators shown for disputed state too
3. Fix timestamps on reopen (rejected → draft):
- Keep red_started_at, blue_started_at etc. as historical record
- Only clear paused_at defensively
- Timestamps naturally update when test is re-executed
4. disputed badge (amber) added to all badge color maps
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
22
backend/alembic/versions/b046_add_disputed_test_state.py
Normal file
22
backend/alembic/versions/b046_add_disputed_test_state.py
Normal file
@@ -0,0 +1,22 @@
|
|||||||
|
"""Add 'disputed' value to teststate enum.
|
||||||
|
|
||||||
|
Revision ID: b046
|
||||||
|
Revises: b045
|
||||||
|
Create Date: 2026-06-03
|
||||||
|
"""
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
revision = "b046"
|
||||||
|
down_revision = "b045"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.execute("ALTER TYPE teststate ADD VALUE IF NOT EXISTS 'disputed'")
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
# PostgreSQL does not support removing enum values; downgrade is a no-op.
|
||||||
|
pass
|
||||||
@@ -24,6 +24,7 @@ class TestState(str, enum.Enum):
|
|||||||
in_review = "in_review"
|
in_review = "in_review"
|
||||||
validated = "validated"
|
validated = "validated"
|
||||||
rejected = "rejected"
|
rejected = "rejected"
|
||||||
|
disputed = "disputed" # one lead approved, the other rejected
|
||||||
|
|
||||||
|
|
||||||
class TeamSide(str, enum.Enum):
|
class TeamSide(str, enum.Enum):
|
||||||
|
|||||||
@@ -314,21 +314,21 @@ class TestEntity:
|
|||||||
def check_dual_validation(self) -> None:
|
def check_dual_validation(self) -> None:
|
||||||
"""Evaluate both leads' votes and advance state if appropriate.
|
"""Evaluate both leads' votes and advance state if appropriate.
|
||||||
|
|
||||||
|
Rules (v2 — consensus required):
|
||||||
- Both **approved** -> ``validated``
|
- Both **approved** -> ``validated``
|
||||||
- Either **rejected** -> ``rejected``
|
- Both **rejected** -> ``rejected``
|
||||||
- Otherwise no change (waiting for the other lead).
|
- One approved + one rejected -> ``disputed`` (conflict, needs discussion)
|
||||||
|
- Otherwise (one or both still pending) -> no change
|
||||||
|
|
||||||
Called automatically by :meth:`validate_red` and :meth:`validate_blue`.
|
Called automatically by :meth:`validate_red` and :meth:`validate_blue`.
|
||||||
Also available as a standalone entry point for backward compatibility
|
|
||||||
when validation fields are set externally.
|
|
||||||
"""
|
"""
|
||||||
self._check_dual_validation()
|
self._check_dual_validation()
|
||||||
|
|
||||||
def _assert_in_review(self, side: str) -> None:
|
def _assert_in_review(self, side: str) -> None:
|
||||||
if self.state != TestState.in_review:
|
if self.state not in (TestState.in_review, TestState.disputed):
|
||||||
raise InvalidOperationError(
|
raise InvalidOperationError(
|
||||||
f"Cannot validate {side} side while test is in "
|
f"Cannot validate {side} side while test is in "
|
||||||
f"'{self.state.value}' state (must be in_review)"
|
f"'{self.state.value}' state (must be in_review or disputed)"
|
||||||
)
|
)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
@@ -339,11 +339,19 @@ class TestEntity:
|
|||||||
)
|
)
|
||||||
|
|
||||||
def _check_dual_validation(self) -> None:
|
def _check_dual_validation(self) -> None:
|
||||||
"""If both leads have voted, advance to validated or rejected."""
|
"""Advance the test state once both leads have voted."""
|
||||||
r, b = self.red_validation_status, self.blue_validation_status
|
r, b = self.red_validation_status, self.blue_validation_status
|
||||||
if r == "rejected" or b == "rejected":
|
|
||||||
self.state = TestState.rejected
|
if r == "approved" and b == "approved":
|
||||||
self._events.append(DomainEvent("dual_validation_rejected"))
|
|
||||||
elif r == "approved" and b == "approved":
|
|
||||||
self.state = TestState.validated
|
self.state = TestState.validated
|
||||||
self._events.append(DomainEvent("dual_validation_approved"))
|
self._events.append(DomainEvent("dual_validation_approved"))
|
||||||
|
|
||||||
|
elif r == "rejected" and b == "rejected":
|
||||||
|
# Full consensus to reject
|
||||||
|
self.state = TestState.rejected
|
||||||
|
self._events.append(DomainEvent("dual_validation_rejected"))
|
||||||
|
|
||||||
|
elif (r == "approved" and b == "rejected") or (r == "rejected" and b == "approved"):
|
||||||
|
# Conflict: one approves, one rejects → needs discussion
|
||||||
|
self.state = TestState.disputed
|
||||||
|
self._events.append(DomainEvent("dual_validation_disputed"))
|
||||||
|
|||||||
@@ -37,7 +37,8 @@ VALID_TRANSITIONS: dict[TestState, list[TestState]] = {
|
|||||||
TestState.draft: [TestState.red_executing],
|
TestState.draft: [TestState.red_executing],
|
||||||
TestState.red_executing: [TestState.blue_evaluating],
|
TestState.red_executing: [TestState.blue_evaluating],
|
||||||
TestState.blue_evaluating: [TestState.in_review],
|
TestState.blue_evaluating: [TestState.in_review],
|
||||||
TestState.in_review: [TestState.validated, TestState.rejected],
|
TestState.in_review: [TestState.validated, TestState.rejected, TestState.disputed],
|
||||||
|
TestState.disputed: [TestState.validated, TestState.rejected],
|
||||||
TestState.rejected: [TestState.draft],
|
TestState.rejected: [TestState.draft],
|
||||||
TestState.validated: [], # terminal state
|
TestState.validated: [], # terminal state
|
||||||
}
|
}
|
||||||
@@ -529,6 +530,60 @@ def _dispatch_dual_validation_effects(
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.warning("Jira push failed for test %s: %s", test.id, e, exc_info=True)
|
logger.warning("Jira push failed for test %s: %s", test.id, e, exc_info=True)
|
||||||
|
|
||||||
|
elif event.name == "dual_validation_disputed":
|
||||||
|
# Notify the lead who APPROVED asking them to review the rejection
|
||||||
|
_notify_validation_conflict(db, test, actor)
|
||||||
|
|
||||||
|
|
||||||
|
def _notify_validation_conflict(db: Session, test: Test, actor: User | None) -> None:
|
||||||
|
"""Notify the lead who APPROVED about the other lead's rejection.
|
||||||
|
|
||||||
|
Tells them: 'The other lead rejected. Review their notes and either
|
||||||
|
change your vote to rejected or discuss with them to resolve.'
|
||||||
|
"""
|
||||||
|
from app.models.user import User as UserModel
|
||||||
|
|
||||||
|
red_approved = test.red_validation_status == "approved"
|
||||||
|
blue_approved = test.blue_validation_status == "approved"
|
||||||
|
|
||||||
|
# Identify who approved (they need to be notified)
|
||||||
|
approver_id = None
|
||||||
|
rejector_role = None
|
||||||
|
rejection_notes = None
|
||||||
|
|
||||||
|
if red_approved and test.blue_validation_status == "rejected":
|
||||||
|
approver_id = test.red_validated_by
|
||||||
|
rejector_role = "Blue Lead"
|
||||||
|
rejection_notes = test.blue_validation_notes
|
||||||
|
elif blue_approved and test.red_validation_status == "rejected":
|
||||||
|
approver_id = test.blue_validated_by
|
||||||
|
rejector_role = "Red Lead"
|
||||||
|
rejection_notes = test.red_validation_notes
|
||||||
|
|
||||||
|
if not approver_id:
|
||||||
|
return
|
||||||
|
|
||||||
|
notes_snippet = f': "{rejection_notes[:200]}"' if rejection_notes else ""
|
||||||
|
try:
|
||||||
|
create_notification(
|
||||||
|
db,
|
||||||
|
user_id=approver_id,
|
||||||
|
type="validation_conflict",
|
||||||
|
title="Validation conflict — action required",
|
||||||
|
message=(
|
||||||
|
f"{rejector_role} rejected test '{test.name}' while you approved it{notes_snippet}. "
|
||||||
|
f"Review their reason and either change your decision to 'rejected' "
|
||||||
|
f"or contact {rejector_role} to resolve the disagreement."
|
||||||
|
),
|
||||||
|
entity_type="test",
|
||||||
|
entity_id=str(test.id),
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
logger.warning(
|
||||||
|
"Failed to send conflict notification for test %s: %s",
|
||||||
|
test.id, e, exc_info=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def handle_remediation_completed(db: Session, test: Test, user: User) -> Test | None:
|
def handle_remediation_completed(db: Session, test: Test, user: User) -> Test | None:
|
||||||
"""Create a re-test when remediation is completed.
|
"""Create a re-test when remediation is completed.
|
||||||
@@ -681,13 +736,12 @@ def reopen_test(db: Session, test: Test, user: User) -> Test:
|
|||||||
test.blue_validated_at = None
|
test.blue_validated_at = None
|
||||||
# test.blue_validation_notes → KEEP (rejection reason / clarification needed)
|
# test.blue_validation_notes → KEEP (rejection reason / clarification needed)
|
||||||
|
|
||||||
# Reset phase timing so the new execution starts fresh
|
# Phase timing: kept as historical record of the previous attempt.
|
||||||
test.red_started_at = None
|
# When the team presses "Start Execution" again, red_started_at will be
|
||||||
test.blue_started_at = None
|
# overwritten with the new timestamp — no manual reset needed.
|
||||||
test.blue_work_started_at = None
|
# Only the active-pause marker is cleared (it should never be set on a
|
||||||
|
# rejected test, but clear it defensively to avoid a stuck timer).
|
||||||
test.paused_at = None
|
test.paused_at = None
|
||||||
test.red_paused_seconds = 0
|
|
||||||
test.blue_paused_seconds = 0
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
from app.services.jira_service import push_test_event
|
from app.services.jira_service import push_test_event
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ const stateBadge: Record<TestState, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
const SEVERITY_COLORS: Record<string, string> = {
|
const SEVERITY_COLORS: Record<string, string> = {
|
||||||
|
|||||||
@@ -8,6 +8,8 @@ import {
|
|||||||
Loader2,
|
Loader2,
|
||||||
Shield,
|
Shield,
|
||||||
ShieldCheck,
|
ShieldCheck,
|
||||||
|
AlertTriangle,
|
||||||
|
MessageSquare,
|
||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import type { Test, TestState, User } from "../../types/models";
|
import type { Test, TestState, User } from "../../types/models";
|
||||||
import LiveTimer from "./LiveTimer";
|
import LiveTimer from "./LiveTimer";
|
||||||
@@ -40,6 +42,7 @@ const STATE_BADGE: Record<TestState, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
// ── Props ──────────────────────────────────────────────────────────
|
// ── Props ──────────────────────────────────────────────────────────
|
||||||
@@ -212,6 +215,30 @@ export default function TestDetailHeader({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Disputed: the lead who approved can change their vote to rejected
|
||||||
|
if (test.state === "disputed") {
|
||||||
|
const canChangeVote =
|
||||||
|
(role === "red_lead" && test.red_validation_status === "approved") ||
|
||||||
|
(role === "blue_lead" && test.blue_validation_status === "approved") ||
|
||||||
|
role === "admin";
|
||||||
|
|
||||||
|
if (canChangeVote) {
|
||||||
|
const side = role === "red_lead" ? "red" : role === "blue_lead" ? "blue" : null;
|
||||||
|
if (side || role === "admin") {
|
||||||
|
buttons.push(
|
||||||
|
<button
|
||||||
|
key="change-vote"
|
||||||
|
onClick={() => onOpenValidateModal(side ?? (test.red_validation_status === "approved" ? "red" : "blue"))}
|
||||||
|
className="flex items-center gap-1.5 rounded-lg bg-amber-600 px-4 py-2 text-sm font-medium text-white hover:bg-amber-500 transition-colors"
|
||||||
|
>
|
||||||
|
<XCircle className="h-4 w-4" />
|
||||||
|
Change Vote to Rejected
|
||||||
|
</button>,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Leads/admin on rejected -> Reopen
|
// Leads/admin on rejected -> Reopen
|
||||||
if (
|
if (
|
||||||
test.state === "rejected" &&
|
test.state === "rejected" &&
|
||||||
@@ -238,7 +265,7 @@ export default function TestDetailHeader({
|
|||||||
// ── Dual validation indicators ───────────────────────────────────
|
// ── Dual validation indicators ───────────────────────────────────
|
||||||
|
|
||||||
const renderValidationIndicators = () => {
|
const renderValidationIndicators = () => {
|
||||||
if (test.state !== "in_review" && test.state !== "validated" && test.state !== "rejected") {
|
if (!["in_review", "validated", "rejected", "disputed"].includes(test.state)) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -347,6 +374,31 @@ export default function TestDetailHeader({
|
|||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{/* Disputed conflict banner */}
|
||||||
|
{test.state === "disputed" && (
|
||||||
|
<div className="mt-3 flex items-start gap-3 rounded-xl border border-amber-500/30 bg-amber-500/5 p-4">
|
||||||
|
<AlertTriangle className="mt-0.5 h-5 w-5 shrink-0 text-amber-400" />
|
||||||
|
<div className="flex-1 min-w-0">
|
||||||
|
<p className="text-sm font-semibold text-amber-300">
|
||||||
|
Validation Conflict — Leads Disagree
|
||||||
|
</p>
|
||||||
|
<p className="mt-0.5 text-xs text-amber-400/80">
|
||||||
|
One lead approved and the other rejected this test.
|
||||||
|
{test.red_validation_status === "rejected" && test.red_validation_notes && (
|
||||||
|
<> Red Lead's reason: <span className="italic">"{test.red_validation_notes}"</span></>
|
||||||
|
)}
|
||||||
|
{test.blue_validation_status === "rejected" && test.blue_validation_notes && (
|
||||||
|
<> Blue Lead's reason: <span className="italic">"{test.blue_validation_notes}"</span></>
|
||||||
|
)}
|
||||||
|
</p>
|
||||||
|
<p className="mt-1.5 text-[10px] text-amber-400/60 flex items-center gap-1">
|
||||||
|
<MessageSquare className="h-3 w-3" />
|
||||||
|
The lead who approved should review the rejection reason and either change their vote or discuss with the other lead to resolve the disagreement.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
{/* Progress bar */}
|
{/* Progress bar */}
|
||||||
{test.state !== "rejected" && (
|
{test.state !== "rejected" && (
|
||||||
<div className="pt-2">
|
<div className="pt-2">
|
||||||
|
|||||||
@@ -55,6 +55,7 @@ const testStateColors: Record<string, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
export default function CampaignDetailPage() {
|
export default function CampaignDetailPage() {
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ const testStateBadgeColors: Record<string, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
const testStateLabels: Record<string, string> = {
|
const testStateLabels: Record<string, string> = {
|
||||||
|
|||||||
@@ -45,6 +45,7 @@ const testStateBadgeColors: Record<TestState, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
const testResultBadgeColors: Record<TestResult, string> = {
|
const testResultBadgeColors: Record<TestResult, string> = {
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ const testStateBadgeColors: Record<TestState, string> = {
|
|||||||
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
in_review: "bg-blue-900/50 text-blue-400 border-blue-500/30",
|
||||||
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
validated: "bg-green-900/50 text-green-400 border-green-500/30",
|
||||||
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
rejected: "bg-red-900/50 text-red-400 border-red-500/30",
|
||||||
|
disputed: "bg-amber-900/50 text-amber-400 border-amber-500/30",
|
||||||
};
|
};
|
||||||
|
|
||||||
const testStateLabels: Record<TestState, string> = {
|
const testStateLabels: Record<TestState, string> = {
|
||||||
@@ -41,6 +42,7 @@ const testStateLabels: Record<TestState, string> = {
|
|||||||
in_review: "In Review",
|
in_review: "In Review",
|
||||||
validated: "Validated",
|
validated: "Validated",
|
||||||
rejected: "Rejected",
|
rejected: "Rejected",
|
||||||
|
disputed: "Disputed",
|
||||||
};
|
};
|
||||||
|
|
||||||
const ALL_STATES: TestState[] = [
|
const ALL_STATES: TestState[] = [
|
||||||
@@ -50,6 +52,7 @@ const ALL_STATES: TestState[] = [
|
|||||||
"in_review",
|
"in_review",
|
||||||
"validated",
|
"validated",
|
||||||
"rejected",
|
"rejected",
|
||||||
|
"disputed",
|
||||||
];
|
];
|
||||||
|
|
||||||
/* ── Helper: which team "owns" the current state ────────────────────── */
|
/* ── Helper: which team "owns" the current state ────────────────────── */
|
||||||
|
|||||||
@@ -43,7 +43,8 @@ export type TestState =
|
|||||||
| "blue_evaluating"
|
| "blue_evaluating"
|
||||||
| "in_review"
|
| "in_review"
|
||||||
| "validated"
|
| "validated"
|
||||||
| "rejected";
|
| "rejected"
|
||||||
|
| "disputed"; // one lead approved, the other rejected — needs discussion
|
||||||
|
|
||||||
export type TestResult = "detected" | "not_detected" | "partially_detected";
|
export type TestResult = "detected" | "not_detected" | "partially_detected";
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user