From ea83d215df191f7b3e72cb8e4cc4b7856b746bf4 Mon Sep 17 00:00:00 2001 From: reference-week Date: Sat, 5 Sep 2026 07:36:28 +0200 Subject: [PATCH 1/2] fix(cli): a step outlives the pause inside it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `turn_end` closed a step interval, so a skill spanning several prompts was credited with its first turn and nothing after. It is a pause, not the end of a step, which is the rule `buildTaskIntervals` and `buildFlowIntervals` already read from this very journal. Measured on the one orchestrated session captured, 2026-09-04: four steps opened across four hours of continuous work, every one closed by the next pause, the last at 06:02:34 against a session that ran until 09:27:21. Of its 1,073 records, 69 fell inside a step interval. The same journal gave the flow axis 1,052 records and this axis 1 — two walks over identical evidence disagreeing by three orders of magnitude. `buildStepIntervals` now runs through `buildClosedIntervals`, the shared walk the other two already use, over the same three-array merge. A `step_end` naming the step's own skill or the next `step_start` closes an interval; nothing else does. On a 30-day report holding 30,222 requests, `journal-interval` goes from 2 to 974, and all ten axes still reconcile to the period total. Two consequences, both stated in code rather than discovered later: - An unclosed step is capped at the journal's own last witnessed moment instead of left open. The premise for leaving it open — that this walk saw no later moments to cap at — is false now it reads `filesWritten` and `taskDeclarations` like the other two. - A journal whose only line is the opener witnesses no later moment, so the step it opened covers nothing, and `records-join` can read fail where it read ok. Found by running it. Failing there is the honest answer: nothing in such a journal says the step was still running. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VWNxk63AGKkqE8HRqHLjGp --- cli/src/domain/models/step-attribution.ts | 148 +++++++++--------- cli/src/domain/ports/run-journal-reader.ts | 10 +- .../diagnose-telemetry-use-case.unit.test.ts | 6 + .../report-cost-use-case.unit.test.ts | 3 + .../models/step-attribution.unit.test.ts | 108 +++++++++---- 5 files changed, 162 insertions(+), 113 deletions(-) diff --git a/cli/src/domain/models/step-attribution.ts b/cli/src/domain/models/step-attribution.ts index 634adfb31..5b632ce25 100644 --- a/cli/src/domain/models/step-attribution.ts +++ b/cli/src/domain/models/step-attribution.ts @@ -1,4 +1,11 @@ -import type { RunJournal, RunJournalBoundary } from "../ports/run-journal-reader.js"; +import type { + RunJournal, + RunJournalBoundary, + RunJournalFileWritten, + RunJournalStepStart, + RunJournalTaskDeclared, +} from "../ports/run-journal-reader.js"; +import { buildClosedIntervals, type ClosedInterval } from "./journal-intervals.js"; import { namesTheSameSkill } from "./skill-name.js"; /** How a record's step came to be known. Never collapsed into one field with the step @@ -31,88 +38,77 @@ export interface StepAttribution { const UNATTRIBUTED: StepAttribution = { source: "unattributed" }; -/** One `step_start`, closed by whichever boundary — another `step_start` or a `turn_end` — - * comes next in file order, or left open if none does. `endMs` is exclusive, matching the - * half-open interval the run journal itself defines. +/** One `step_start`, closed by a `step_end` naming that same skill or by the next + * `step_start`, and - unclosed - by the journal's own last witnessed moment. `endMs` is + * exclusive, matching the half-open interval the run journal itself defines. * - * **Left open, unlike a task or a flow interval, and the difference is deliberate.** - * `journal-intervals.ts` caps an unclosed task or flow at the journal's own last witnessed - * moment, because leaving one open "would go on attributing everything a long-running - * session does afterward to the first opener it ever saw". The same cap cannot be applied - * here: those two walks see `filesWritten` and `taskDeclarations` as well as boundaries, so - * there are later moments to cap at, while this one sees boundaries alone. The last - * boundary of a session that never wrote `turn_end` *is* the open `step_start`, so capping - * would give it a zero-width interval covering nothing — trading "attributes too much" for - * "attributes nothing", which is not obviously the better error. + * **A `turn_end` stopped closing one on 2026-09-05.** It is a pause, not the end of a + * step, which is the rule `buildTaskIntervals` and `buildFlowIntervals` already read from + * this very journal; a step spanning three prompts was being credited with its first turn + * and nothing after. Measured on the one orchestrated session captured, 2026-09-04: four + * steps opened across four hours of continuous work, every one of them closed by the next + * pause, the last at 06:02:34 against a session that went on until 09:27:21. Of its 1,073 + * records, 69 fell inside a step interval; with a pause no longer closing one, 1,065 do. + * The same journal already gave the flow axis 1,052 records and this axis 1 - two walks + * over identical evidence disagreeing by three orders of magnitude, which is what this + * change removes. * - * A session ends without `turn_end` whenever its host fires no stop event; `journal.cjs`'s - * own `HOOK_EVENT_NAME_TO_CANONICAL` maps one for Claude Code, Cursor and OpenCode, and - * none for Copilot. So this is a live case, not a theoretical one, and - * `aidd telemetry check`'s own `records-join` claim currently depends on the open reading — - * change it and that claim starts failing for every unclosed session. Pinned by - * `buildStepIntervals` tests below so the choice stays visible rather than incidental. */ -export interface StepInterval { + * **Capped rather than left open, which reverses the choice this comment used to pin.** + * That choice rested on one premise: the cap "cannot be applied here" because this walk saw + * `boundaries` alone, while a task or flow interval also saw `filesWritten` and + * `taskDeclarations`, so it had later moments to cap at and this had none. The premise is + * now false by construction - `buildStepIntervals` reads the same three arrays they do. All + * that survives of it is the degenerate journal whose very last line is the opener, where + * the cap does give a zero-width interval covering nothing. Open is not the safer error + * there: one captured session carries a single `vendor_id` spanning 22 days, so + * "everything the session does afterward" is three weeks of unrelated work. + * + * `aidd telemetry check`'s `records-join` claim was said to depend on the open reading, and + * in that degenerate journal it genuinely does: `joinedVerdict` fails when *every* record is + * unattributed, so a session whose journal holds the opener and nothing else, and whose + * records carry no tool-stated step of their own, flips that claim from ok to fail. Found by + * running it, not reasoned about - `diagnose-telemetry-use-case.unit.test.ts` held exactly + * that journal. Failing there is the honest answer: nothing in such a journal says the step + * was still running, and a claim reading ok on the strength of an unbounded interval was + * asserting what it could not see. Every host that writes a pause is unaffected, which is + * Claude Code, Cursor and OpenCode by `journal.cjs`'s own `HOOK_EVENT_NAME_TO_CANONICAL`. */ +export interface StepInterval extends ClosedInterval { readonly skill: string; - readonly startMs: number; - readonly endMs: number; -} - -interface TimedBoundary { - readonly atMs: number; - readonly boundary: RunJournalBoundary; -} - -/** Drops a boundary whose own `at` cannot be parsed, before any pairing happens — never - * leaving it in as a mid-list gap. Left in, an unparseable boundary would vanish from - * `nextBoundaryMs`'s view while still occupying a list index, so the interval before it - * would silently inherit the *next* boundary's moment as its own end, misattributing every - * record in between to the wrong skill rather than reading them as unattributed. */ -function parseableBoundaries(boundaries: readonly RunJournalBoundary[]): readonly TimedBoundary[] { - const timed: TimedBoundary[] = []; - for (const boundary of boundaries) { - const atMs = Date.parse(boundary.at); - if (!Number.isNaN(atMs)) timed.push({ atMs, boundary }); - } - return timed; } -/** Journal lines in, intervals out — no filesystem, no record. Two skills that interleave - * (A, then B, then A) yield three intervals and two names, exactly as the boundaries - * dictate; nothing here decides which record falls into which, that is `attributeMoment`'s - * job, kept separate so an interval list can be built once per session and reused. */ -/** Where a step that opened at `from` ends. +/** Journal lines in, closed intervals out - no filesystem, no record. Run through the one + * shared walk (`buildClosedIntervals`) rather than a second copy of it: this module used to + * carry its own `timed`/`parseableBoundaries` pair and its own closer scan, which is how it + * came to disagree with the two walks reading the same journal beside it. * - * A `step_end` naming this very skill wins over everything between, however many pauses that - * is: it is the only line in the journal that states the end rather than standing in for it, - * and a skill spanning three prompts is exactly the case a `turn_end` used to cut short. + * Any `step_start` opens an interval - unlike `buildFlowIntervals`, which opens one only + * for a skill declared to orchestrate. A `step_end` naming that same skill closes it, by + * `namesTheSameSkill` and never `===`: the host that opened the step may have written the + * skill's bare directory name while the end the skill echoes carries its plugin. A + * `step_end` naming a *different* skill is never a closer, which is the fault naming the + * skill exists to prevent. Every other line - a `turn_end`, a `file_written`, a + * `task_declared` - neither opens nor closes one, and only ever contributes its own moment + * toward the journal's last witnessed one. * - * With no such line, the rule is the one this reader always had - the next `step_start` or - * `turn_end`, or nothing. A `step_end` naming a *different* skill is never a closer here: it - * would truncate a step it has no claim on, which is the fault naming the skill exists to - * prevent. Same or different is `namesTheSameSkill`'s answer, not `===`: the host that - * opened the step may have written the skill's bare directory name while the end the skill - * echoes carries its plugin. */ -function stepEndsAt(timed: readonly TimedBoundary[], from: number, skill: string): number { - for (let i = from + 1; i < timed.length; i++) { - const { boundary } = timed[i]; - if (boundary.type === "step_end" && namesTheSameSkill(boundary.skill, skill)) - return timed[i].atMs; - } - for (let i = from + 1; i < timed.length; i++) { - if (timed[i].boundary.type !== "step_end") return timed[i].atMs; - } - return Number.POSITIVE_INFINITY; -} - -export function buildStepIntervals(journal: RunJournal): readonly StepInterval[] { - const timed = parseableBoundaries(journal.boundaries); - const intervals: StepInterval[] = []; - for (let i = 0; i < timed.length; i++) { - const { atMs: startMs, boundary } = timed[i]; - if (boundary.type !== "step_start") continue; - intervals.push({ skill: boundary.skill, startMs, endMs: stepEndsAt(timed, i, boundary.skill) }); - } - return intervals; + * Two runs of the very same skill in one session yield two distinct intervals, never one + * merged by name, exactly as the boundaries dictate; nothing here decides which record + * falls into which, that is `attributeMoment`'s job. */ +export function buildStepIntervals( + journal: RunJournal, + periodEndMs?: number +): readonly StepInterval[] { + return buildClosedIntervals< + RunJournalBoundary | RunJournalTaskDeclared | RunJournalFileWritten, + RunJournalStepStart, + StepInterval + >( + [...journal.boundaries, ...journal.taskDeclarations, ...journal.filesWritten], + periodEndMs, + (boundary): boundary is RunJournalStepStart => boundary.type === "step_start", + (boundary, opener) => + boundary.type === "step_end" && namesTheSameSkill(boundary.skill, opener.skill), + (opener, startMs, endMs) => ({ skill: opener.skill, startMs, endMs }) + ); } /** Where a record's own moment falls inside one interval, that interval's skill is the diff --git a/cli/src/domain/ports/run-journal-reader.ts b/cli/src/domain/ports/run-journal-reader.ts index 501bee0ca..f5043a352 100644 --- a/cli/src/domain/ports/run-journal-reader.ts +++ b/cli/src/domain/ports/run-journal-reader.ts @@ -86,10 +86,12 @@ export interface RunJournalFileWritten { * on that task from here on — told rather than inferred, the way `step_start` names a * skill. Carries no task identity for the same reason `file_written` does not: `path` is * the same repository-relative shape, and deriving the task from it is `task-identity.ts`'s - * job. Deliberately kept out of `RunJournalBoundary` — pairing it into `boundaries` would - * let it close a running step early (see `step-attribution.ts`'s `buildStepIntervals`), so - * a task interval is built from this array plus `boundaries`' own `turn_end` lines instead, - * in `domain/models/task-attribution.ts`. */ + * job. Deliberately kept out of `RunJournalBoundary`: what opens or closes a step is a + * `step_start` and a `step_end` naming its skill, and a declaration is neither. It reaches + * every interval walk all the same - `buildTaskIntervals`, `buildFlowIntervals` and + * `buildStepIntervals` each merge this array and `filesWritten` into `boundaries` before + * walking - but as a moment the journal witnessed, never as a boundary that ends something. + * The type keeps the two apart so a later reader cannot confuse them by accident. */ export interface RunJournalTaskDeclared { readonly type: "task_declared"; readonly at: string; diff --git a/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts b/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts index 4cdb7b435..f7f617243 100644 --- a/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts +++ b/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts @@ -193,8 +193,14 @@ describe("DiagnoseTelemetryUseCase — gathering local evidence", () => { it("reads every covered tool's own files for every journalled session", async () => { const journal: RunJournal = { session: sessionStart("s-1"), + // The pause is what the step interval is capped at, and it is why the candidate below + // falls inside one at all. A journal whose only line is the opener witnesses no later + // moment, so the step it opened covers nothing and `records-join` reads fail - see + // `buildStepIntervals`'s own doc comment. Every host this fixture stands for writes a + // pause: `journal.cjs` maps a stop event for Claude Code, Cursor and OpenCode. boundaries: [ { type: "step_start", at: "2026-08-20T09:00:30Z", skill: "aidd-dev:02-implement" }, + { type: "turn_end", at: "2026-08-20T09:30:00Z" }, ], filesWritten: [], taskDeclarations: [], diff --git a/cli/tests/application/use-cases/telemetry/report-cost-use-case.unit.test.ts b/cli/tests/application/use-cases/telemetry/report-cost-use-case.unit.test.ts index 284cdde68..e93c211e5 100644 --- a/cli/tests/application/use-cases/telemetry/report-cost-use-case.unit.test.ts +++ b/cli/tests/application/use-cases/telemetry/report-cost-use-case.unit.test.ts @@ -651,8 +651,11 @@ describe("a report that catches the sink up first", () => { const journal = journalAt("2026-08-18T09:00:00Z"); journals.set(SESSION, { ...journal, + // The pause after the record is what the step is capped at: a step runs past a pause + // but never past the last moment its own journal witnessed. boundaries: [ { type: "step_start", at: "2026-08-18T09:30:00Z", skill: "aidd-dev:02-implement" }, + { type: "turn_end", at: "2026-08-18T10:30:00Z" }, ], }); await sink.appendRecord( diff --git a/cli/tests/domain/models/step-attribution.unit.test.ts b/cli/tests/domain/models/step-attribution.unit.test.ts index 4c99dfbb4..80f094a37 100644 --- a/cli/tests/domain/models/step-attribution.unit.test.ts +++ b/cli/tests/domain/models/step-attribution.unit.test.ts @@ -11,6 +11,13 @@ function journalOf(...boundaries: RunJournal["boundaries"]): RunJournal { return { boundaries, filesWritten: [], taskDeclarations: [] }; } +function journalWith( + boundaries: RunJournal["boundaries"], + filesWritten: RunJournal["filesWritten"] +): RunJournal { + return { boundaries, filesWritten, taskDeclarations: [] }; +} + const A_START = { type: "step_start", at: "2026-08-20T10:00:00Z", @@ -37,6 +44,26 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = expect(attribution).toEqual({ source: "journal-interval", step: "aidd-dev:02-implement" }); }); + // A `turn_end` is a pause, not the end of a step - the rule `buildTaskIntervals` and + // `buildFlowIntervals` already read from this very journal. Measured on the one + // orchestrated session captured, 2026-09-04: four steps opened over four hours, every one + // closed by the next pause, and 69 of the session's 1,073 records fell inside a step + // interval. With a pause no longer closing one, 1,065 of them do. + it("runs a step past a pause, to the journal's own last witnessed moment", () => { + const intervals = buildStepIntervals( + journalWith( + [A_START, TURN_END], + [{ type: "file_written", at: "2026-08-20T11:00:00Z", path: "aidd_docs/note.md" }] + ) + ); + + expect(attributeMoment(intervals, "2026-08-20T10:30:00Z")).toEqual({ + source: "journal-interval", + step: "aidd-dev:02-implement", + }); + expect(intervals[0]?.endMs).toBe(Date.parse("2026-08-20T11:00:00Z")); + }); + // A `turn_end` is a pause: a skill that spans three prompts is credited with its first // turn and nothing after. Nothing any host emits says when a skill's work finished - // measured, a `Skill` call's own `tool_result` returns in about a tenth of a second, which @@ -95,27 +122,28 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = expect(intervals[0]?.endMs).toBe(Date.parse("2026-08-20T10:30:00Z")); }); + // The journal carries a later moment than the disagreeing end, deliberately: with the end + // as its last line the interval would stop there anyway - at the journal's own last + // witnessed moment - and the assertion could not tell a refused closer from a cap. it("still refuses an end whose plugin disagrees with the one that opened the step", () => { const intervals = buildStepIntervals( - journalOf(A_START, { - type: "step_end", - at: "2026-08-20T10:02:00Z", - skill: "aidd-pm:02-implement", - }) + journalWith( + [A_START, { type: "step_end", at: "2026-08-20T10:02:00Z", skill: "aidd-pm:02-implement" }], + [{ type: "file_written", at: "2026-08-20T10:20:00Z", path: "aidd_docs/note.md" }] + ) ); - expect(intervals[0]?.endMs).not.toBe(Date.parse("2026-08-20T10:02:00Z")); + expect(intervals[0]?.endMs).toBe(Date.parse("2026-08-20T10:20:00Z")); }); // An end for a skill that never started names nothing to close. Read as a boundary all the // same it would truncate whatever interval was running, which is a step it has no claim on. it("ignores an end for a skill this session never started", () => { const intervals = buildStepIntervals( - journalOf(A_START, { - type: "step_end", - at: "2026-08-20T10:02:00Z", - skill: "some-other:skill", - }) + journalWith( + [A_START, { type: "step_end", at: "2026-08-20T10:02:00Z", skill: "some-other:skill" }], + [{ type: "file_written", at: "2026-08-20T10:20:00Z", path: "aidd_docs/note.md" }] + ) ); expect(attributeMoment(intervals, "2026-08-20T10:03:00Z")).toEqual({ @@ -137,7 +165,11 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = }); }); - it("closes the last step at its own turn_end, leaving nothing beyond it covered", () => { + // A pause is not a closer, so what bounds the last step here is the journal's own last + // witnessed moment - which this journal's `turn_end` happens to be. Same moment as the + // old rule gave, reached for a different reason, so the boundary between covered and not + // stays pinned either way. + it("leaves nothing beyond the journal's last witnessed moment covered", () => { const intervals = buildStepIntervals(journalOf(B_START, TURN_END)); expect(attributeMoment(intervals, "2026-08-20T10:14:59Z")).toMatchObject({ @@ -212,37 +244,47 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = }); describe("buildStepIntervals — a step the session never closed", () => { - // Pinned because it is a choice, not an accident, and one that differs from how - // `journal-intervals.ts` treats an unclosed task or flow. See `StepInterval`'s own doc - // comment for why the cap those two apply cannot be applied here — and note that - // `aidd telemetry check`'s `records-join` claim depends on this reading, so changing it - // is a behaviour change, not a tidy-up. - it("leaves the last step open when no turn_end ever closed it", () => { - const intervals = buildStepIntervals({ - boundaries: [{ type: "step_start", at: "2026-08-17T10:00:00Z", skill: "aidd-dev:01-plan" }], - filesWritten: [], - taskDeclarations: [], - }); + // Capped, not left open, and the objection this used to carry is gone rather than + // overruled: it said the cap "cannot be applied here" because this walk saw boundaries + // alone while a task or flow interval also saw `filesWritten` and `taskDeclarations`. + // This walk now reads the same three arrays they do, so the later moments it was said to + // lack are the ones it caps at. What is left is the degenerate journal below - one whose + // very last line is the opener - and there an open interval is not the safer error: one + // captured session carries a single `vendor_id` spanning 22 days, so "everything + // afterward" is three weeks of unrelated work, not a few minutes of it. + it("caps a step nothing closed at the journal's own last witnessed moment", () => { + const intervals = buildStepIntervals( + journalWith( + [{ type: "step_start", at: "2026-08-17T10:00:00Z", skill: "aidd-dev:01-plan" }], + [{ type: "file_written", at: "2026-08-17T12:00:00Z", path: "aidd_docs/note.md" }] + ) + ); expect(intervals).toEqual([ { skill: "aidd-dev:01-plan", startMs: Date.parse("2026-08-17T10:00:00Z"), - endMs: Number.POSITIVE_INFINITY, + endMs: Date.parse("2026-08-17T12:00:00Z"), }, ]); + expect(attributeMoment(intervals, "2026-09-30T23:59:00Z")).toEqual({ source: "unattributed" }); }); - it("attributes a much later moment to it, which is what leaving it open means", () => { - const intervals = buildStepIntervals({ - boundaries: [{ type: "step_start", at: "2026-08-17T10:00:00Z", skill: "aidd-dev:01-plan" }], - filesWritten: [], - taskDeclarations: [], - }); + // The price of the cap, stated rather than discovered later: a journal whose only line is + // the opener has no later moment to cap at, so the interval covers nothing at all. A + // session reaches this only by opening a skill and then writing no file, declaring no + // task and firing no stop event - Copilot fires none, per `journal.cjs`'s own + // `HOOK_EVENT_NAME_TO_CANONICAL`. `records-join` survives it: that claim fails only when + // *every* record is unattributed, and a record whose own tool named its step is joined + // without any interval at all. + it("covers nothing when the opener is the only moment the journal ever witnessed", () => { + const intervals = buildStepIntervals( + journalOf({ type: "step_start", at: "2026-08-17T10:00:00Z", skill: "aidd-dev:01-plan" }) + ); - expect(attributeMoment(intervals, "2026-09-30T23:59:00Z")).toEqual({ - source: "journal-interval", - step: "aidd-dev:01-plan", + expect(intervals[0]?.endMs).toBe(Date.parse("2026-08-17T10:00:00Z")); + expect(attributeMoment(intervals, "2026-08-17T10:00:01Z")).toEqual({ + source: "unattributed", }); }); }); From 97d922dbecef88938d3b2eee0790b7bc379237a2 Mon Sep 17 00:00:00 2001 From: reference-week Date: Sat, 5 Sep 2026 07:40:47 +0200 Subject: [PATCH 2/2] test(cli): pin what capping an unclosed step costs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two guards for consequences the change states but nothing asserted. `check` on a journal whose only line is the opener: the step covers nothing, so a record carrying no step of its own joins nothing and `records-join` reads fail. That is a real host — Copilot fires no stop event — and `check` is the command whose job is saying measurement is broken, so the verdict belongs in a test rather than in a doc comment alone. A step whose end shares its start's moment covers nothing. The journal stamps `nowIso()`, whose resolution is the second, so two lines sharing a moment is the common case; the shared walk orders them by a stable sort, which for two `boundaries` entries is file order. Pinned because that ordering is inherited from the sort now rather than written out. Mutation run: an unclosed step left open again turns both red. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VWNxk63AGKkqE8HRqHLjGp --- .../diagnose-telemetry-use-case.unit.test.ts | 31 +++++++++++++++++++ .../models/step-attribution.unit.test.ts | 22 +++++++++++++ 2 files changed, 53 insertions(+) diff --git a/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts b/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts index f7f617243..8acf88d79 100644 --- a/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts +++ b/cli/tests/application/use-cases/telemetry/diagnose-telemetry-use-case.unit.test.ts @@ -218,6 +218,37 @@ describe("DiagnoseTelemetryUseCase — gathering local evidence", () => { expect(result.claims.find((c) => c.claim === "records-join")?.verdict).toBe("ok"); }); + // The consequence of capping an unclosed step, pinned where a person actually meets it. + // Copilot fires no stop event - `journal.cjs`'s own `HOOK_EVENT_NAME_TO_CANONICAL` maps + // one for Claude Code, Cursor and OpenCode and none for it - so a Copilot session that + // opened a skill and then wrote no file and declared no task leaves a journal whose only + // line is the opener. It witnesses no later moment, so the step covers nothing, and a + // record carrying no step of its own joins nothing. `check` says so rather than reporting + // a join it cannot see: an interval with no end in evidence used to reach forward + // indefinitely, which is what made this read ok. + it("says records join nothing when the journal's only line is the step that opened", async () => { + const journal: RunJournal = { + session: sessionStart("s-1"), + boundaries: [ + { type: "step_start", at: "2026-08-20T09:00:30Z", skill: "aidd-dev:02-implement" }, + ], + filesWritten: [], + taskDeclarations: [], + }; + const claudeReader = new StubSessionCostReader({ records: [candidate()], sessionFound: true }); + const { useCase } = buildUseCase({ + journals: [journal], + readers: new Map([["claude", claudeReader]]), + }); + + const result = await useCase.execute(runOptions({ CLAUDE_CODE_SESSION_ID: "s-1" })); + + if (result.gate !== undefined) throw new Error("expected the run to pass the gate"); + const join = result.claims.find((claim) => claim.claim === "records-join"); + expect(join?.verdict).toBe("fail"); + expect(join?.reason).toBe("all-unattributed"); + }); + it("names a reader that threw as failing to read, never crashing the whole diagnostic", async () => { const journal: RunJournal = { session: sessionStart("s-1"), diff --git a/cli/tests/domain/models/step-attribution.unit.test.ts b/cli/tests/domain/models/step-attribution.unit.test.ts index 80f094a37..c8059be76 100644 --- a/cli/tests/domain/models/step-attribution.unit.test.ts +++ b/cli/tests/domain/models/step-attribution.unit.test.ts @@ -234,6 +234,28 @@ describe("step-attribution — pure: journal lines + records -> intervals", () = }); }); + // The journal stamps every line with `nowIso()`, whose resolution is the second, so two + // lines sharing a moment is the common case rather than a corner. The shared walk sorts + // by moment, and a sort that is stable - as V8's is - leaves lines that share one in the + // order they were read, which for two `boundaries` entries is file order. Pinned here + // because that ordering is now inherited from the sort rather than written out, and a + // step whose own end shares its start's moment must cover nothing rather than everything. + it("closes a step at an end sharing its own start's moment, covering nothing", () => { + const intervals = buildStepIntervals( + journalWith( + [ + A_START, + { type: "step_end", at: A_START.at, skill: A_START.skill }, + { type: "turn_end", at: "2026-08-20T11:00:00Z" }, + ], + [{ type: "file_written", at: "2026-08-20T12:00:00Z", path: "aidd_docs/note.md" }] + ) + ); + + expect(intervals[0]?.endMs).toBe(Date.parse(A_START.at)); + expect(attributeMoment(intervals, A_START.at)).toEqual({ source: "unattributed" }); + }); + it("touches no filesystem — the module imports none of Node's fs APIs", () => { const url = new URL("../../../src/domain/models/step-attribution.ts", import.meta.url); const source = readFileSync(fileURLToPath(url), "utf8");