Repository navigation
Conversation
The Chainloop-Trace-Sessions trailer was chosen by intersecting the staged files with every ai-lines ledger on disk, with no scoping in time, branch or session liveness. Those ledgers never expire — WipeTraceDir preserves them and GCOrphans only drops one when no surviving commit references it — so a session pinned itself into the repository after its first commit and kept being stamped onto every later commit that touched any file it had ever edited. The consequences compounded downstream: a session's evidence file set comes from the diffs of the commits credited to it, so a foreign commit contributed its whole diff, and every file the session had no ranges for was reported as human-written. A single session accumulated commits from unrelated branches and surfaced on pull requests it never touched. Attribution is now consumed when it lands. Each ai-lines entry carries a recorded_at, the post-commit hook appends a consumed_by marker for the files the commit contained, and matching runs against the pending set instead of the full history. Ranges are retained for push-time enrichment, and per-file consumption keeps credit intact for a session committing its work in chunks. Entries written before recorded_at existed are never pending, so ledgers already on disk stop matching immediately. Two supporting changes: session-end now persists active: false and session-start revives it on resume, which lets a push with no commits to go on skip sessions that have ended; and GCOrphans retires sessions older than a week once everything they produced has been attested, since reachability alone never expires them. Active remains unsuitable as an attribution signal — session-end is not installed for `trace run` and never fires on SIGKILL or a closed terminal. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Consuming attribution on commit closed the case where a finished session had already committed its work, but not the one where it had not. Two sessions in a row over the same file, neither having committed, both stayed pending, so the commit carried both IDs — attaching a session that had moved on to that commit and to the pull request holding it, with the whole commit diff attributed to it. Each staged file is now owned by the session that edited it last: the earlier edit no longer describes the file, since the later session rewrote those lines. Ownership is per file, so a session still keeps credit for the other files it has pending. Ordering this needs better than the second resolution NowTimestamp provides — two sessions editing one file within the same second were indistinguishable, and the tie-break decided it. Attribution entries now record RFC3339Nano through a new NowTimestampPrecise, and Pending carries the parsed edit time per file. Timestamps are compared as times, never as strings: a fractional-second form does not sort lexicographically against a whole-second one. The tie-break stays for genuinely equal timestamps, falling to the lower session ID so the trailer does not follow map iteration order. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Security Checks — ✅ 8 passing✅
|
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | secrets-detection |
- |
✅ sast-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | owasp-top10-2025 |
- |
| ✅ Passed | sast |
- |
| ✅ Passed | cwe-top25 |
- |
| ✅ Passed | cwe-top26-40-cusp |
- |
PR info
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | pr-min-approvals |
- |
| ✅ Passed | pr-description-required |
- |
| ✅ Passed | pr-user-story-linked |
- |
⏭️ 3 scans not applied
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
iac-scan |
no IaC files changed |
Powered by Chainloop and Chainloop Trace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…test clocks Only session-start revives an ended session. The revival was in ensureSessionTracked, which the pre- and post-tool-use hooks also call, so one of those arriving after session-end resurrected a session that really had finished and the next no-commit push would attest it. A consumption marker now names the edit it retires rather than the time it was written. MarkConsumed reads the pending set and then appends, so a concurrent RecordLineRanges could land in between and leave the marker behind an edit the commit never contained; replay then cleared that edit's pending state and the session lost credit for work still in the working tree. Replay compares the pending time against the marker's, so a later edit survives. The snapshot is now an explicit parameter, since it is the boundary this has to be correct across. Ordering tests no longer race the wall clock. They asserted strict ordering between two consecutive RecordLineRanges calls, which ties on a host whose clock advances in coarse ticks; the new RecordLineRangesAt lets them set the times instead, and the same seam makes the two-session lifecycle test deterministic. Left as is, with the reasoning recorded in staleSessionIDs: the GC does not spare a week-old session's uncommitted attribution. Keeping it means a session nothing has heard from in a week can still claim a file and pull the whole commit's diff into its evidence, and by then its transcript is usually gone, so the attestation it wins yields nothing. Attributing that edit to nobody is the safe direction. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
|
Thanks — four of the five were real. Addressed in 79a3764. Fixed
Not changed — deliberate, reasoning now recorded in The GC not sparing a week-old session's uncommitted attribution is the intended trade-off, not an oversight. Keeping it means a session nothing has heard from in a week can still win ownership of a file and pull the whole commit's diff into its evidence — the exact bug this PR exists to fix. By that point its transcript is usually gone ( |
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Bounding a consumption marker with the retired edit's timestamp left the race it was meant to close open at the edges: a racing edit that landed in the same clock tick compared equal, so the marker swept it up and the session lost credit for work still in the working tree. Timestamps cannot settle this — that is the same coarse-clock tie the ordering tests had to stop relying on. Pending entries now carry their position in the ledger alongside the edit time, and a marker records the position it retires. Positions come from an append-only file, so they are unique and strictly increasing no matter what the clock does, and an edit appended after the snapshot sits past the marker's bound and stays pending. A marker without a position retires the file outright, matching the behaviour of the markers written before the field existed. Ownership between sessions still goes by edit time: positions from different ledgers are not comparable. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
|
Good catch, and valid — fixed in 5f2d38d. You were right that a timestamp cannot bound the marker: a racing edit landing in the same clock tick compared equal, so the marker swept it up and the session lost credit for work still in the working tree. That is the same coarse-clock tie that forced the ordering tests off the wall clock, so keying retirement to a time was never going to hold. Rather than add a separate ordering token, the bound is now the entry's position in the ledger. Ownership between sessions still goes by Two tests added, both verified to fail against the timestamp-bounded version: a racing edit that shares the snapshot's exact timestamp survives, and a marker with no position retires the file. |
2 participants
The
Chainloop-Trace-Sessionstrailer was chosen by intersecting the staged files with everyai-linesledger on disk, with no scoping in time, branch or session liveness. Those ledgers never expire —WipeTraceDirpreserves them andGCOrphansonly drops one when no surviving commit references it — so a session pinned itself into the repository after its first commit and kept being stamped onto every later commit that touched any file it had ever edited.The consequences compounded downstream: a session's evidence file set comes from the diffs of the commits credited to it, so a foreign commit contributed its whole diff, and every file the session had no ranges for was reported as human-written. A single session accumulated commits from unrelated branches and surfaced on pull requests it never touched.
What changed
Attribution is now consumed when it lands:
ai-linesentry carries arecorded_at, and the post-commit hook appends aconsumed_bymarker for the files the commit contained.recorded_atexisted are never pending, so ledgers already on disk stop matching immediately.Two supporting changes:
session-endpersistsactive: falseandsession-startrevives it on resume, which lets a push with no commits to go on skip sessions that have ended.activewas previously written once and never read.GCOrphansretires sessions older than a week once everything they produced has been attested, since reachability alone never expires them.activeremains unsuitable as an attribution signal —session-endis not installed fortrace runand never fires on SIGKILL or a closed terminal.