Skip to content

fix(blame): order a session's own edits by seq, not by timestamp - #172

Open
thegoodengineer wants to merge 1 commit into
agitHQ:mainfrom
thegoodengineer:fix/blame-seq-order
Open

thegoodengineer wants to merge 1 commit into
agitHQ:mainfrom
thegoodengineer:fix/blame-seq-order

Conversation

@thegoodengineer

Copy link
Copy Markdown
Contributor

blameFile sorts every step by timestamp first, including two edits from the same session. When the recording clock steps back between two edits to one file, blame replays them in the wrong order and still marks the result verified. SPEC §2 rules that out:

seq is the ordering. ts SHOULD be non-decreasing but readers MUST NOT rely on it: it is source data, and clocks are what they are.

fork walks seq, so the two verbs disagree about the same log. Here is a synthetic session in the same record shape as the demo fixture: a Write of a b c, then an Edit of b to B, with the clock a minute behind by the time of the Edit (the same records the second test builds):

$ agit replay clock --timeline
...
    5  10:00:03  file.diff      create C:\app\cfg.txt (+3 -0)
    6  09:59:00  tool.call      Edit file_path=C:\app\cfg.txt
    ...
    9  09:59:01  file.diff      modify C:\app\cfg.txt (+1 -1)
$ agit blame cfg.txt
C:\app\cfg.txt  (1 session(s) touched it)
clock-sk @    5     1  a
clock-sk @    5     2  b
clock-sk @    5     3  c
...
$ agit blame cfg.txt --json
...
  "verified": true
}
$ agit why cfg.txt:2
C:\app\cfg.txt:2  b

written by  clock-skew-0001 @5  (2026-09-06T10:00:03.000Z)
...
$ agit fork clock --at 10 --out ./fork && cat fork/tree/cfg.txt
...
a
B
c

Blame replays seq 9 first. It seeds from the Edit's recorded originalFile, which hash-checks, and the create at seq 5 then lands on top and wins. So blame reports, and vouches for, the file as it was before the edit. Without originalFile (Codex, OpenClaw) the same reordering goes wrong the other way: the later edit has nothing to apply to, and blame reports a divergence, which the CLI prints as "proof the file changed outside structured edits".

After:

$ agit blame cfg.txt
C:\app\cfg.txt  (1 session(s) touched it)
clock-sk @    5     1  a
clock-sk @    9     2  B
clock-sk @    5     3  c
...
$ agit blame cfg.txt --json
...
  "verified": true
}
$ agit why cfg.txt:2
C:\app\cfg.txt:2  B

written by  clock-skew-0001 @9  (2026-09-06T09:59:01.000Z)
...

Cause

steps.sort((a, b) => a.e.ts.localeCompare(b.e.ts) || a.e.seq - b.e.seq);

seq only breaks ties here. Each step now also carries a running clock: the latest ts its session's steps on this path have reached. The sort uses that clock in place of ts and keeps seq as the tie-break. Inside a session the clock never goes back and seq always goes up, so a session's own steps follow seq. Across sessions, steps still interleave by time. The docstring said "in timestamp order"; it now says which order applies where.

When a session's timestamps never go back, the clock is just ts, so the order is exactly what it was. To check that, I blamed every path in every text fixture and golden log with both sorts, with each session alone and with all of them together (13 sessions, 16 paths, 224 calls). Every result was byte-identical.

Tests

test/blame-clock-skew.test.ts has five tests. The first three cover the symptoms and the last two pin the ordering between sessions:

  • A create followed by two edits, both stamped before the create, so the clock stays behind for more than one step. It must replay 1, 2, 3 and verify. On main it fails with verified: false (divergence at seq 2).
  • The session above, run through the real adapter from record-shaped input. Line 2 must be B, from the modify event. On main it fails: line 2 is b and verified is true.
  • A file.delete stamped before the create it follows. The file must end up deleted. On main it fails: the delete replays first, and blame shows the created lines.
  • Two sessions with sane clocks, passed in both orders: A, then B on top, then A again on top of B. The interleaving must match what blame gave before. This one passes on main; it guards against a fix that folds each session in whole.
  • The same shape with a stepped-back edit in A. That edit must sort next to A's previous edit, which puts it before B's later edit rather than before everything. On main it fails (divergence at seq 2).

The existing blame, no-trailing-newline, atif-adapter and cline-sdk-adapter suites pass unchanged.

Not changed

  • The clock follows only this path's own steps, not every event in the session. fixtures/openclaw/edits.jsonl and fixtures/codex/edits.jsonl already stamp file events a second earlier than the event just before them. That is ordinary adapter output, and a clock taken over every event would push those edits later relative to other sessions. SPEC settles the order inside a session. Across sessions a stepped-back timestamp can only be guessed at, however it is handled, so I left that part alone.
  • Ties between sessions still break on seq, as before.

Full suite 876 green; typecheck, lint and prettier clean on Windows.

SPEC §2 says seq is the ordering and readers must not rely on ts, but blameFile sorted every step by ts first, even two edits from the same session.
When the clock stepped back between two edits, blame replayed them out of order: with originalFile it reported the pre-edit content as verified, without it a false divergence.
Each step now sorts by the latest ts its session has reached on that path, so a session's edits follow seq while sessions still interleave by time. Logs whose timestamps never go back sort exactly as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant