From 40114d041b6c8b8f4d08835c246b272a2bfeda9b Mon Sep 17 00:00:00 2001 From: Knacky Date: Fri, 15 May 2026 15:20:25 +0200 Subject: [PATCH] fix(m7): stamping executed_at no longer requires a prior state transition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User reported `HTTP 400 — executed_at can only be set when state is executed/reviewed_by_blue` when typing the timestamp inline in the new scenario table. The state-gate predates the simplified UX — it made sense back when the workflow was "Mark executed button + override toggle", but the user has since asked for a single freely-typeable datetime input. - update_mission_test_fields drops the state check. Stamping a non-null executed_at while state ∈ {pending, skipped, blocked} now auto-promotes the state to `executed` in the same write. The promotion is gated by the same mission.write_red_fields perm that executed_at already required — no privilege escalation. - MissionTestPage.tsx drops the state-based UI gate on canEditExecutedAt; red perm alone now unlocks the input regardless of state. - Replaced the old "rejection while pending" test with two new tests: pending→executed via inline stamp + blue 403, and skipped→executed via inline stamp. - 139 pytest green. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 24 ++++++++ backend/app/services/mission_tests.py | 19 ++++--- backend/tests/test_mission_tests.py | 79 +++++++++++++++++--------- frontend/src/pages/MissionTestPage.tsx | 7 ++- 4 files changed, 93 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d664c05..1914687 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,30 @@ All notable changes to this project will be documented here. Format: [Keep a Cha ## [Unreleased] +### Fixed (post-amendement 2026-05-15) — stamping executed_at no longer needs a prior state transition + +User feedback: when a red user typed `executed_at` inline on a pending test +in the new scenario table, the backend rejected with `HTTP 400 — executed_at +can only be set when state is executed/reviewed_by_blue`. The state-gate +was a holdover from the original "Mark executed button + override toggle" +workflow; it made no sense once the UX let the operator type the time +directly. + +- `update_mission_test_fields` (`backend/app/services/mission_tests.py`) no + longer rejects writes based on the source state. Stamping a non-null + `executed_at` while state ∈ {`pending`, `skipped`, `blocked`} now + auto-promotes the state to `executed` in the same write. The promotion + rides on the same `mission.write_red_fields` perm that the executed_at + field already required — no privilege escalation. +- `MissionTestPage.tsx` drops the state-based gate on `canEditExecutedAt`: + the field is editable any time the viewer holds `mission.write_red_fields`. +- Tests: `test_executed_at_override_requires_red_perm_and_state` was the + old guard; it's split into two new cases — `test_red_setting_executed_at_on_pending_auto_transitions_to_executed` + (pending → executed via inline stamp, blue still 403'd) and + `test_red_setting_executed_at_from_skipped_state_auto_transitions` + (skipped → executed via the same path). +- Total: **139 pytest** green. + ### Added — M7 amendment (2026-05-15) — blue review fields + full-width scenario table User feedback after the M7 ship: the blue team used to maintain 5 extra diff --git a/backend/app/services/mission_tests.py b/backend/app/services/mission_tests.py index 4a8f428..23e1ba6 100644 --- a/backend/app/services/mission_tests.py +++ b/backend/app/services/mission_tests.py @@ -590,13 +590,16 @@ def update_mission_test_fields( ) if "executed_at_overridden" in touched or "executed_at" in touched: - # Editing executed_at is a red-only privilege and only valid when - # the test is past the `executed` milestone. Spec M7: override is - # behind a deliberate toggle so the auto-stamp default is sticky. - if test.state not in {"executed", "reviewed_by_blue"}: - raise InvalidTestPayload( - "executed_at can only be set when state is executed/reviewed_by_blue" - ) + # Editing executed_at is a red-only privilege (gated above via + # _RED_FIELDS). We no longer reject the write based on the + # current state — the spec amendement 2026-05-15 lets the red + # team record an execution timestamp inline, which would be + # circular if they had to transition the state machine first. + # Instead, stamping a non-null timestamp implicitly bumps the + # state forward from any non-executed source so the persisted + # record stays internally consistent. The same `mission. + # write_red_fields` perm covers both moves, so this isn't a + # privilege escalation. new_overridden = ( bool(executed_at_overridden) if "executed_at_overridden" in touched @@ -609,6 +612,8 @@ def update_mission_test_fields( ) if "executed_at" in touched and new_at is not None and not isinstance(new_at, datetime): raise InvalidTestPayload("executed_at must be an ISO datetime") + if new_at is not None and test.state in {"pending", "skipped", "blocked"}: + test.state = "executed" test.executed_at = new_at test.executed_at_overridden = new_overridden diff --git a/backend/tests/test_mission_tests.py b/backend/tests/test_mission_tests.py index 6a4abf8..dc68462 100644 --- a/backend/tests/test_mission_tests.py +++ b/backend/tests/test_mission_tests.py @@ -569,18 +569,33 @@ def test_mark_executed_stamps_executed_at( assert body["executed_at_overridden"] is False -def test_executed_at_override_requires_red_perm_and_state( +def test_red_setting_executed_at_on_pending_auto_transitions_to_executed( client, admin_token, catalogue, red_user, blue_user ): + """Post-amendement (2026-05-15): the red team should be able to stamp + executed_at inline without first POST-ing /transition. The service + auto-flips state pending→executed in the same write so the persisted + row stays internally consistent. + + Blue still cannot touch executed_at (red-side field) — perm gating is + unchanged. + """ mission = _make_mission( - client, admin_token, name="m7-override", + client, admin_token, name="m7-exec-at-pending", scenario_id=catalogue["scenario"]["id"], red_id=red_user["id"], blue_id=blue_user["id"], ) tid = _first_test_id(mission) - # Override while still pending → invalid_request (no executed milestone yet). - bad = client.put( + # Sanity: starts as pending. + detail = client.get( + f"/api/v1/missions/{mission['id']}/tests/{tid}", + headers=_bearer(red_user["token"]), + ) + assert detail.get_json()["state"] == "pending" + + # Red stamps executed_at directly — must succeed and bump the state. + stamped = client.put( f"/api/v1/missions/{mission['id']}/tests/{tid}", headers=_bearer(red_user["token"]), json={ @@ -588,39 +603,49 @@ def test_executed_at_override_requires_red_perm_and_state( "executed_at_overridden": True, }, ) - assert bad.status_code == 400, bad.get_data(as_text=True) + assert stamped.status_code == 200, stamped.get_data(as_text=True) + body = stamped.get_json() + assert body["state"] == "executed" + assert body["executed_at"].startswith("2026-05-14T10:00:00") + assert body["executed_at_overridden"] is True - # Mark executed first so we're allowed to override. - client.post( - f"/api/v1/missions/{mission['id']}/tests/{tid}/transition", - headers=_bearer(red_user["token"]), - json={"target_state": "executed"}, - ) - - # Blue cannot override (executed_at is a red field). + # Blue cannot touch executed_at — red-side field. forbidden = client.put( f"/api/v1/missions/{mission['id']}/tests/{tid}", headers=_bearer(blue_user["token"]), - json={ - "executed_at": "2026-05-14T10:00:00+00:00", - "executed_at_overridden": True, - }, + json={"executed_at": "2026-05-14T11:00:00+00:00"}, ) assert forbidden.status_code == 403 - # Red successfully overrides. - ok = client.put( + +def test_red_setting_executed_at_from_skipped_state_auto_transitions( + client, admin_token, catalogue, red_user +): + """Skipped → executed by stamping a timestamp: same implicit transition + so the operator who marked the test skipped by mistake can simply type + the actual execution time.""" + mission = _make_mission( + client, admin_token, name="m7-exec-at-skipped", + scenario_id=catalogue["scenario"]["id"], red_id=red_user["id"], + ) + tid = _first_test_id(mission) + # Move to skipped first. + skip = client.post( + f"/api/v1/missions/{mission['id']}/tests/{tid}/transition", + headers=_bearer(red_user["token"]), + json={"target_state": "skipped"}, + ) + assert skip.status_code == 200 + assert skip.get_json()["state"] == "skipped" + + # Stamp executed_at inline → should land in executed. + stamped = client.put( f"/api/v1/missions/{mission['id']}/tests/{tid}", headers=_bearer(red_user["token"]), - json={ - "executed_at": "2026-05-14T10:00:00+00:00", - "executed_at_overridden": True, - }, + json={"executed_at": "2026-05-14T12:00:00+00:00"}, ) - assert ok.status_code == 200, ok.get_data(as_text=True) - body = ok.get_json() - assert body["executed_at_overridden"] is True - assert body["executed_at"].startswith("2026-05-14T10:00:00") + assert stamped.status_code == 200, stamped.get_data(as_text=True) + assert stamped.get_json()["state"] == "executed" def test_state_machine_rejects_invalid_transitions( diff --git a/frontend/src/pages/MissionTestPage.tsx b/frontend/src/pages/MissionTestPage.tsx index d07b258..8cba87c 100644 --- a/frontend/src/pages/MissionTestPage.tsx +++ b/frontend/src/pages/MissionTestPage.tsx @@ -228,8 +228,11 @@ function RedZone({ test, missionId, canWriteRed }: RedZoneProps) { } const apiErr = save.error instanceof ApiError ? save.error : null; - const canEditExecutedAt = - canWriteRed && (test.state === 'executed' || test.state === 'reviewed_by_blue'); + // Post-amendement 2026-05-15: the backend auto-promotes the state from + // pending/skipped/blocked when a non-null executed_at is stamped, so the + // UI no longer needs to disable the input on those states. Red perm is + // still required. + const canEditExecutedAt = canWriteRed; return (