From da6e3d5d49ffbd8c1acf2346578f3a680bcf26a4 Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 01:05:51 +0200 Subject: [PATCH 1/2] fix(feedback): seal the event time in the ledger, not the write time record_feedback accepts an `at`, writes it to the store row, and then appended the ledger entry without it -- so the tamper-evident record an auditor reads disagreed with the store about when the review happened. A feedback backdated to January was sealed under September's wall clock. Even the default path took two separate clock readings, one for the row and one for the chain. Pass `at=entry.at`, the contract runner.record already keeps with at=run.finished_at, and that ledger.py's docstring blesses explicitly: timestamps may be supplied, because append order is proven by the entry's seq rather than by its stamp. Nothing about the chain changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/feedback.py | 8 ++++++++ tests/test_feedback.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/flightdeck/feedback.py b/src/flightdeck/feedback.py index c3ab19d..4a9ca2d 100644 --- a/src/flightdeck/feedback.py +++ b/src/flightdeck/feedback.py @@ -48,6 +48,8 @@ def record_feedback( ``feedback_recorded`` event the reports read: ``{run_id, outcome, human_minutes, by}``. ``human_minutes`` left ``None`` means "not timed" and the metrics fall back to the org's conservative ``default_review_minutes``. + ``at`` stamps both the row and the ledger entry, so an imported or backdated + review reads the same on either side of the evidence trail. """ if outcome not in VALID_OUTCOMES: raise FeedbackError(f"outcome must be one of: {', '.join(VALID_OUTCOMES)}") @@ -70,5 +72,11 @@ def record_feedback( ledger.append( "feedback_recorded", {"run_id": run_id, "outcome": outcome, "human_minutes": human_minutes, "by": entry.by}, + # The event time, not the write time -- the same contract runner.record + # keeps with at=run.finished_at. Without it a backfilled or imported + # review is sealed under the wall clock, and the ledger an auditor reads + # contradicts the store row it is supposed to be evidence for. Append + # order is carried by the entry's seq, so this costs the chain nothing. + at=entry.at, ) return entry diff --git a/tests/test_feedback.py b/tests/test_feedback.py index 47ddf39..196929d 100644 --- a/tests/test_feedback.py +++ b/tests/test_feedback.py @@ -5,6 +5,8 @@ API and ledger events" is enforced at the source. """ +from datetime import timedelta + import pytest from flightdeck.feedback import FeedbackError, record_feedback @@ -61,3 +63,31 @@ def test_record_feedback_rejects_negative_or_non_finite_minutes(org, store, ledg run = _seed_run(org, store, ledger) with pytest.raises(FeedbackError, match="non-negative number"): record_feedback(store, ledger, run.id, "accepted", human_minutes=bad) + + +def test_backdated_feedback_seals_the_event_time_in_the_ledger(org, store, ledger): + # A review imported or backfilled from another system carries its own time. + # The store row and the ledger entry describe the SAME event, so they must not + # disagree about when it happened -- the ledger is the artifact an auditor + # reads, and runner.record already keeps this contract (at=run.finished_at). + run = _seed_run(org, store, ledger) + when = NOW - timedelta(days=200) + + entry = record_feedback(store, ledger, run.id, "accepted", human_minutes=3, by="ana", at=when) + sealed = [e for e in ledger.entries() if e["event"] == "feedback_recorded"][-1] + + assert entry.at == when + assert sealed["at"] == when.isoformat() # was the wall clock, months adrift + assert store.feedback_map()[run.id].at.isoformat() == sealed["at"] + + +def test_feedback_without_an_explicit_time_still_seals_what_the_row_says(org, store, ledger): + # The default path must stay consistent too: whatever "now" the row got is the + # one the chain seals, not a second clock reading taken a moment later. + run = _seed_run(org, store, ledger) + + entry = record_feedback(store, ledger, run.id, "edited", human_minutes=2, by="ana") + sealed = [e for e in ledger.entries() if e["event"] == "feedback_recorded"][-1] + + assert sealed["at"] == entry.at.isoformat() + assert ledger.verify().ok # and the chain still walks clean From e7aecf5aaef2165e13ee78cb8da2e0f0b7971303 Mon Sep 17 00:00:00 2001 From: Marc Sturlese Date: Fri, 4 Sep 2026 01:12:12 +0200 Subject: [PATCH 2/2] fix(feedback): normalize the sealed stamp to UTC Gate review: forwarding entry.at was right, but it imported the row's local offset into a ledger every other writer keeps in UTC -- runner.record via datetime.now(UTC), the demo seeder via explicit tzinfo=UTC. Because `audit tail` renders entry["at"][:16], slicing the offset off before the reader sees it, a review recorded from a UTC-7 machine displayed seven hours BEFORE the run it reviews, with nothing on screen to explain it. That regressed the two live entry points, the CLI and the Slack button, to fix a path nothing in the product calls yet. Normalize at the append site, so the invariant holds for an explicit caller too: the alternative of defaulting the row to UTC only covers the path where no `at` is passed, and lets an explicitly local one back into the file. The tests were blind to this -- one asserted the sealed stamp equalled entry.at.isoformat(), a tautology once the value is forwarded verbatim, whatever offset it carries. They now compare instants and pin the convention: every ledger stamp ends in +00:00 even when the caller hands in a Tokyo offset, and feedback never predates its run. Suite green under TZ=UTC, America/Los_Angeles and Europe/Madrid. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XjLWb6igee2tVTG7wg93Fx --- src/flightdeck/feedback.py | 16 ++++++++++------ tests/test_feedback.py | 23 +++++++++++++++++++++-- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/flightdeck/feedback.py b/src/flightdeck/feedback.py index 4a9ca2d..da57dbf 100644 --- a/src/flightdeck/feedback.py +++ b/src/flightdeck/feedback.py @@ -13,7 +13,7 @@ """ import math -from datetime import datetime +from datetime import UTC, datetime from flightdeck.ledger import Ledger from flightdeck.schemas import Feedback, Outcome @@ -73,10 +73,14 @@ def record_feedback( "feedback_recorded", {"run_id": run_id, "outcome": outcome, "human_minutes": human_minutes, "by": entry.by}, # The event time, not the write time -- the same contract runner.record - # keeps with at=run.finished_at. Without it a backfilled or imported - # review is sealed under the wall clock, and the ledger an auditor reads - # contradicts the store row it is supposed to be evidence for. Append - # order is carried by the entry's seq, so this costs the chain nothing. - at=entry.at, + # keeps with at=run.finished_at. Without it a backfilled review is sealed + # under the wall clock, and the ledger an auditor reads contradicts the + # store row it is evidence for. Normalized to UTC because the row's own + # default is local-offset while every other ledger writer is UTC, and + # `audit tail` prints the stamp without its offset: mixing conventions in + # one file would show a review as happening before the run it reviews. + # Append order is carried by the entry's seq, so this costs the chain + # nothing. + at=entry.at.astimezone(UTC), ) return entry diff --git a/tests/test_feedback.py b/tests/test_feedback.py index 196929d..73ff8fd 100644 --- a/tests/test_feedback.py +++ b/tests/test_feedback.py @@ -5,7 +5,7 @@ API and ledger events" is enforced at the source. """ -from datetime import timedelta +from datetime import datetime, timedelta, timezone import pytest @@ -89,5 +89,24 @@ def test_feedback_without_an_explicit_time_still_seals_what_the_row_says(org, st entry = record_feedback(store, ledger, run.id, "edited", human_minutes=2, by="ana") sealed = [e for e in ledger.entries() if e["event"] == "feedback_recorded"][-1] - assert sealed["at"] == entry.at.isoformat() + # Compare instants, not strings: the row's default carries the local offset, + # the ledger is normalized to UTC, and both name the same moment. + assert datetime.fromisoformat(sealed["at"]) == entry.at assert ledger.verify().ok # and the chain still walks clean + + +def test_ledger_stays_utc_whatever_offset_the_caller_hands_in(org, store, ledger): + # `audit tail` renders entry["at"][:16] -- the offset is sliced off before the + # reader sees it. So every entry in the file has to share one convention, or a + # review shows up hours before the run it reviews. The runner and the demo + # seeder both write UTC; feedback must not be the one exception. + run = _seed_run(org, store, ledger) + tokyo = timezone(timedelta(hours=9)) + + record_feedback(store, ledger, run.id, "accepted", by="ana", at=NOW.astimezone(tokyo)) + stamps = [e["at"] for e in ledger.entries()] + + assert all(s.endswith("+00:00") for s in stamps), stamps + # ...and the feedback still cannot predate the run it attests to. + events = {e["event"]: e["at"] for e in ledger.entries()} + assert events["feedback_recorded"] >= events["run_completed"]