diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 2acc782c6..314ab4379 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -55,6 +55,13 @@ "description": "ALPHA, not ready for use. UI and UX concern: design, review, and improve frontend interfaces.", "strict": true, "recommended": false + }, + { + "name": "aidd-telemetry", + "source": "./plugins/aidd-telemetry", + "description": "Measurement: journals every session so a unit of work can be tied to what it cost. Ships hooks only, and carries no measurement itself.", + "strict": true, + "recommended": false } ] } diff --git a/.claude/settings.json b/.claude/settings.json index 04bbef92f..f5ebe8d8d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,18 +1,5 @@ { "$schema": "https://json.schemastore.org/claude-code-settings.json", - "extraKnownMarketplaces": { - "aidd-framework": { - "source": { "source": "directory", "path": "." } - } - }, - "enabledPlugins": { - "aidd-context@aidd-framework": true, - "aidd-dev@aidd-framework": true, - "aidd-vcs@aidd-framework": true, - "aidd-pm@aidd-framework": true, - "aidd-orchestrator@aidd-framework": true, - "aidd-refine@aidd-framework": true - }, "hooks": { "WorktreeCreate": [ { @@ -24,5 +11,21 @@ ] } ] + }, + "enabledPlugins": { + "aidd-context@aidd-framework": true, + "aidd-dev@aidd-framework": true, + "aidd-vcs@aidd-framework": true, + "aidd-pm@aidd-framework": true, + "aidd-orchestrator@aidd-framework": true, + "aidd-refine@aidd-framework": true + }, + "extraKnownMarketplaces": { + "aidd-framework": { + "source": { + "source": "directory", + "path": "." + } + } } } diff --git a/.github/workflows/cli-ci.yml b/.github/workflows/cli-ci.yml index 86258ddc9..13d102b5d 100644 --- a/.github/workflows/cli-ci.yml +++ b/.github/workflows/cli-ci.yml @@ -13,10 +13,18 @@ on: paths: - "cli/**" - "kanban/**" + # The plugin ships the hooks and skills these jobs exercise, and the Windows job + # exists for them in particular - without this, changing any of it runs nothing. + - "plugins/aidd-telemetry/**" + - "scripts/__tests__/**" pull_request: paths: - "cli/**" - "kanban/**" + # The plugin ships the hooks and skills these jobs exercise, and the Windows job + # exists for them in particular - without this, changing any of it runs nothing. + - "plugins/aidd-telemetry/**" + - "scripts/__tests__/**" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -108,6 +116,32 @@ jobs: - run: cd cli && pnpm install --frozen-lockfile - run: cd cli && pnpm knip:production + identifier-join: + name: cli / Identifier join (Claude Code) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install pnpm + run: | + corepack enable + corepack prepare pnpm@latest --activate + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "22" + - run: cd cli && pnpm install --frozen-lockfile + - run: cd cli && pnpm build + # The probe drives the real tool, so the real tool has to be here. No credentials are + # configured and none are needed: it points Claude Code at a dead address with a fake + # key, and the session id is minted before any of that matters. + - name: Install Claude Code + run: npm install -g @anthropic-ai/claude-code + # Re-checks what #632 measured once, by running a session: that the identifier a hook + # receives is the one the export carries. Everything in this layer joins on that, and a + # tool update can break it with nothing else turning red. Exit 2 means the probe could + # not form an opinion and says so rather than blaming the tool. + - name: Probe the identifier join + run: node scripts/probe-identifier-join.cjs + cli-jscpd: name: cli / JSCPD (duplication) runs-on: ubuntu-latest @@ -140,3 +174,90 @@ jobs: - run: cd kanban && pnpm typecheck - run: cd kanban && pnpm lint - run: cd kanban && pnpm test + + windows: + # Runs under bash (Git Bash, bundled on windows-latest) so every command is the one the + # Linux jobs run, not a PowerShell rewrite of it. + name: cli / Windows + runs-on: windows-latest + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install pnpm + run: | + corepack enable + corepack prepare pnpm@latest --activate + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "22" + + # The WRITE path, on the platform where a home directory, a line ending and an absolute + # path all resolve differently than they do on the Linux jobs above. It runs before the + # CLI is built, on purpose: recording is supposed to need nothing installed, and this is + # where that claim is exercised. Reading is the CLI's now, and the e2e project below + # covers it on this same runner. + - name: Chain - allow measurement, with nothing installed + run: | + # Written directly rather than through a command: the switch is a file, the hooks + # read it fresh at every write, and no binary has to exist for that to work. + mkdir -p .aidd + node -e "require('fs').writeFileSync('.aidd/config.json', JSON.stringify({ telemetry: { enabled: true } }, null, 2) + '\n');" + - name: Chain - journal a captured payload + run: | + # The fixture's own cwd is a path from whatever machine captured it - rewritten to + # this checkout's real path so getRepoRoot resolves a repository that exists here. + node -e "const p=require('./scripts/__tests__/fixtures/claude-code-session-start.json'); p.cwd=process.cwd(); require('fs').writeFileSync('payload.json', JSON.stringify(p));" + node plugins/aidd-telemetry/hooks/journal.cjs session-start < payload.json + node plugins/aidd-telemetry/hooks/journal.cjs turn-end < payload.json + - name: Chain - the journal stays private and git add -A still works + run: | + git add -A + git status --porcelain >/dev/null + + - name: Plugin suite + run: node --test "scripts/__tests__/*.test.js" + + - run: cd kanban && pnpm install --frozen-lockfile + - run: cd cli && pnpm install --frozen-lockfile + # A real global install, not `node dist/cli.js` by path: build, pack, and + # `npm install -g` the tarball โ€” the same shim generation a person's own + # `npm install -g @ai-driven-dev/cli` produces. Every suite below invokes the built + # file directly and proves nothing about whether `aidd` actually resolves on this + # platform's PATH โ€” Windows is where that kind of assumption has broken silently + # before (`os.homedir()` never reading `$HOME` here). Built here, ahead of the + # unit/integration/e2e suites below, so this is the only build this job needs. + # + # Inlined rather than `pnpm run install:local` (the equivalent script this package + # already ships): a package.json script runs through pnpm's own configured shell, + # `cmd.exe` on Windows unless `script-shell` says otherwise, which this repository + # never sets โ€” its `$(node -p ...)` command substitution is bash syntax and would + # not survive that. This step's own `run:` block is guaranteed bash by the job's + # `defaults.run.shell` above, so the same two commands run here directly instead, + # with a glob standing in for the version substitution. + - name: Install the built CLI globally, the way a person actually would + run: | + cd cli + pnpm build + pnpm pack --pack-destination ./dist + npm install -g ./dist/ai-driven-dev-cli-*.tgz --force + # `02-check` diagnoses the chain above through this exact command, lifted from its + # own markdown โ€” not a script beside it, the same move `00-init`'s own chain step + # made for the switch. `aidd --version` is the same command every skill's own locate + # step runs first; failing here means the CLI could not be resolved on the PATH. + - name: Chain - diagnose, through the command every skill's own markdown names + run: | + aidd --version + aidd telemetry check + - run: cd cli && pnpm test:unit + - run: cd cli && pnpm test:integration + - name: cli e2e + run: | + cd cli + pnpm exec vitest run --project=e2e \ + --exclude "tests/e2e/persona.e2e.test.ts" \ + --exclude "tests/e2e/telemetry-multi-tool.e2e.test.ts" + # persona.e2e.test.ts hardcodes /usr/bin/expect for TTY emulation, which windows-latest + # does not carry. telemetry-multi-tool.e2e.test.ts puts a `#!/bin/sh` stand-in binary + # with no extension on PATH; Windows resolves an executable by PATHEXT, never a shebang. diff --git a/.gitignore b/.gitignore index f5455cc1a..8ce3232e3 100644 --- a/.gitignore +++ b/.gitignore @@ -32,6 +32,22 @@ coverage/ .claude/settings.local.json .claude/worktrees/ +# AIDD CLI's own local state (install manifest, auth): machine-local, never +# part of the project's own tracked content. config.json is the exception: +# it is the committed telemetry switch (see .aidd/config.json and +# aidd_docs/tasks/2026_08/2026_08_20_telemetry-export-enable/phase-1.md), +# tracked so a fresh clone inherits the project's decision. +.aidd/* +!.aidd/config.json + +# AIDD run-journal records: where they land once .aidd/config.json turns +# telemetry on (see plugins/aidd-telemetry/hooks/journal.js) - this +# directory being committed is a location, not a permission. The records it +# holds never are. +aidd_docs/runs/* +!aidd_docs/runs/.gitkeep +!aidd_docs/runs/README.md + # SpecStory captures (may contain transcripts / secrets) .specstory/ diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 92ba4bd61..9288b7a76 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -7,5 +7,6 @@ "plugins/aidd-orchestrator": "2.2.1", "plugins/aidd-refine": "3.0.0", "plugins/aidd-ui": "0.2.1-alpha.0", + "plugins/aidd-telemetry": "0.1.0", "cli": "5.2.1" } diff --git a/README.md b/README.md index b7ed19099..8c3b23ad0 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Unify **engineering teams** around **standardized workflows** and **shared best ๐Ÿงฑ **IDE agnostic** ยท ๐Ÿ—๏ธ **Legacy systems** ยท ๐ŸŒฑ **Token-optimized** ยท ๐Ÿ‡ซ๐Ÿ‡ท **Made in France**

- 7 plugins ยท 47 skills ยท 2 agents + 8 plugins ยท 50 skills ยท 2 agents

[![Open Source](https://img.shields.io/badge/Open_Source-Yes-yellow?logo=open-source-initiative&logoColor=white)](https://opensource.org/) @@ -57,7 +57,7 @@ Why not just write your own commands? โ†’ [FAQ](docs/FAQ.md#-why-aidd-instead-of ### Claude Code -Installs the 6 stable plugins (`aidd-ui` is ๐Ÿšง alpha, install separately โ€” see [Plugins](#-plugins)). +Installs the 6 stable plugins (`aidd-ui` and `aidd-telemetry` are ๐Ÿšง alpha, install separately โ€” see [Plugins](#-plugins)). **In the session** (slash commands) @@ -213,7 +213,7 @@ flowchart TD ## ๐Ÿงฉ Plugins -Seven plugins covering the whole SDLC โ€” **install all of them**; they work together. (`aidd-ui` is ๐Ÿšง **alpha**, off the curated path.) +Eight plugins covering the whole SDLC โ€” **install all of them**; they work together. (`aidd-ui` and `aidd-telemetry` are ๐Ÿšง **alpha**, off the curated path.) @@ -284,7 +284,15 @@ Synchronous feature flow, async issue-to-PR automation, and product backlog. UI / UX design โ€” smoke-test only, not ready for use. - +
+ +### ๐Ÿ“ˆ [aidd-telemetry](plugins/aidd-telemetry/README.md) ๐Ÿšง + +`3 skills` ยท **alpha** + +Answers what a piece of work cost โ€” tokens, models, and which skill spent them. Off unless you turn it on, and nothing leaves your machine. + +
diff --git a/aidd_docs/memory/testing.md b/aidd_docs/memory/testing.md index 648c2da2d..ea8860891 100644 --- a/aidd_docs/memory/testing.md +++ b/aidd_docs/memory/testing.md @@ -6,12 +6,16 @@ ## Testing Strategy -- No unit test runner configured at framework level +- The CLI runs vitest in three projects: `unit`, `integration`, `e2e` (`cli/`, ~2,600 tests) +- The plugins' own scripts run under `node --test`, in `scripts/__tests__/`, reaching their subject by path rather than by import - Skills are validated by running each action's `## Test` end-to-end against a real environment - Framework correctness validated by running actual skills against a real project (integration) ## Test Execution Process +- **While working, run `pnpm test:changed`** โ€” it runs only the specs a change can break: vitest resolves the CLI's import graph, and the plugin specs are selected by the paths their own text names. Minutes become seconds, and nothing that could break is skipped +- Before declaring work done, run the full suites: `cd cli && pnpm test:unit && pnpm test:integration && pnpm test:e2e`, plus `node --test "scripts/__tests__/*.test.js"` +- Run biome through `rtk proxy` (`rtk proxy npx biome check src/ tests/`): the plain call's output is filtered and reports "no issues" while errors are pending - Each action declares a `## Test` (a command to run, an artifact check, or an observable side-effect) that must pass before the next action runs - `scripts/build-dist-verification.md` documents how to verify the build output diff --git a/aidd_docs/product/cost-report-contract.md b/aidd_docs/product/cost-report-contract.md new file mode 100644 index 000000000..781facfb7 --- /dev/null +++ b/aidd_docs/product/cost-report-contract.md @@ -0,0 +1,306 @@ +# Cost report contract + +**Read this if you are writing a skill, or anything else that reports on AIDD work.** +It describes what `aidd telemetry report --json` prints: one object, the same shape +whatever tool did the work, carrying both the figures and a statement of what each tool +could and could not supply. + +> If instead you are building a **pricing service or an aggregator** that consumes stored +> records directly, read [`metrics-contract.md`](./metrics-contract.md) โ€” the contract for +> one stored line. The two are deliberately different audiences, and picking the wrong one +> is expensive: the record contract makes you responsible for the three double-count rules +> (the two record kinds, a local re-read โ€” including correcting, never summing, a still-open +> turn a later read completes โ€” and one billed call seen by both an export and a local read +> at once โ€” Claude Code today), and re-read deduplication. This one has already applied all +> four. + +**Never reconstruct these figures from stored records.** One computation in one place is +the whole point: two ways of computing a number is how they start disagreeing. + +## Getting the object + +```bash +aidd telemetry report --json +aidd telemetry report --from 2026-08-01 --to 2026-08-31 --json +aidd telemetry report --task 2026_08/2026_08_21_cost-reporter --json +aidd telemetry report --project acme/widgets --step aidd-dev:02-implement --json +``` + +Prints one JSON object on stdout and exits `0`, including when the period holds nothing. +A period that is not a period โ€” `--from notaday`, `--days 0` โ€” exits `1` naming the flag. +A filter naming a value nothing ever recorded still exits `0` โ€” see **Filters** below; +only a malformed period is a usage error. + +## Filters + +Six dimensions exist: day, project, task, step, model, tool โ€” an axis says how to +*group*, a filter says what to *keep*, and every one of them works as either. The period +(`--from`/`--to`/`--days`) is the day filter; `--task` already existed; `--project`, +`--step`, `--model` and `--tool` are the other four, each optional: + +```bash +aidd telemetry report --project acme/widgets --json # this project, whole period +aidd telemetry report --project acme/widgets --step aidd-dev:02-implement --json +``` + +**Filters compose by `and`, never by a query language.** Two given narrow to their +intersection; there is no `or` and no parentheses โ€” the moment a report needs one it has +stopped being a report. Filtering and grouping on the same dimension (`--project X` next +to a `by_project` breakdown that then holds one row) is a legal, boring answer, not an +error. + +**"Axis" above means a breakdown, not a flag.** Every `by_*` array is always present in +the `--json` object, whatever filters were given โ€” grouping by any of the six dimensions +needs no separate flag; reading the matching array is the axis. `aidd telemetry report` +also takes `--axis ` (`total`, `day`, `step`, `model`, `tool` or `project`), which +picks one of those arrays and renders it alone as a small pasteable artefact instead of +the whole object โ€” a convenience for copying one figure out, not a second way to group. +Every figure `--axis` can show is already in the plain `--json` object; only the +one-artefact-at-a-time rendering is what it adds. A name outside the six is a usage error +naming the valid list (`Error: Unknown axis 'bogus'. Expected one of: total, day, step, +model, tool, project.`, exit `1`), not a silently empty artefact. Given both flags at +once, `--json` wins and `--axis` is ignored, never the reverse. + +**A filter matching nothing names itself**, in `empty_selection`, rather than the object +quietly reporting the same shape a genuinely idle period would: + +```jsonc +"empty_selection": { "filter": "project", "value": "acme/ghost", "known": false } +``` + +`known: false` means no record this call could see ever carried that value โ€” a project +nobody ever worked in. `known: true` means the value is real, just idle in this selection; +an optional `"combination": true` alongside it means the value matches something on its +own, and it is the intersection with an already-applied filter that emptied the +selection, not the value itself. `empty_selection` is **never** present for a period that +is genuinely idle โ€” that case is a row of zeros, because the zero is true; this field +exists only for the different case, where a filter is what emptied it. + +The known/unknown distinction is only as good as what a call could still see: a value +whose every record has since rotated out of the sink reads as `known: false`, the same as +one that never existed. It answers "did anything I can still read ever carry this", not +"did this ever happen". + +**A model filter always drops a whole-session figure; a step filter usually does.** +`active_time_s` and a tool's `session_totals` come from `kind: "session"` records, and +those never carry a `model` โ€” no reader stamps one on a session-kind record, on any tool +measured so far. Filtering by model is correct to exclude them: a model selection cannot +speak to a whole-session figure no model was ever attached to. A `step` is different: a +session record still gets one wherever its own moment happens to fall inside a journal's +`step_start` interval, the same attribution every other record gets โ€” so a step filter +keeps a session record when that interval matches, and drops it otherwise. Either way the +number does not appear as `0` when dropped; it is simply absent, the same convention every +other "never observed" quantity in this object uses. + +Adding `filters` and `empty_selection` was not a `cost_report_version` bump: a consumer +built against version 2 that never passes a filter never sees either field, and neither +changes what any field it already understood means. + +## Determinism + +**The same files and the same absolute period produce byte-identical output.** That holds +across repeated calls and across the order records happen to sit in on disk, which differs +between machines because a re-read appends. + +It does **not** hold for `--days`, which resolves against today. `--days` is the human +shorthand; anything that stores or compares a figure should ask for `--from` and `--to`. +The object always reports the period **as it resolved**, absolutely, never as it was asked +for โ€” so a figure taken from a `--days` call can still be cited by the days it covered. + +## Versioning + +Every object carries `cost_report_version`, currently `3` โ€” bumped from `2` when `by_model` +gained a row with no `model`, for a record neither reader that permits one could name (a +consumer that read `row.model` as always a string on every prior version would misread this +one). Bumped from `1` to `2` when `by_day` and `by_project` joined `by_step`, `by_model` and +`by_tool` as top-level breakdowns. + +**Set aside an object whose version you do not recognise rather than guessing its shape.** +The number is bumped when a consumer that understood the previous shape would misread this +one. Adding a field you may ignore is not a bump; changing what an existing field means is. + +## The shape + +```jsonc +{ + "cost_report_version": 3, + "period": { "from_day": "2026-07-01", "to_day": "2026-07-31" }, + "task": "2026_08/2026_08_21_cost-reporter", // absent unless --task was given + "filters": { "project": "acme/widgets" }, // absent unless a generic filter was given + "empty_selection": { "filter": "project", "value": "acme/ghost", "known": false }, // absent unless a filter, not the period, emptied this selection + "sessions": 1, + "totals": { "requests": 2, "input_tokens": 13930, "output_tokens": 4377, "cache_read_tokens": 165632, "cache_creation_tokens": 0 }, + "active_time_s": 2820, // absent when no record carried it + "by_step": [{ "step": "aidd-dev:02-implement", "attribution": "journal-interval", "totals": {} }], + "by_model": [{ "model": "gpt-5.6-sol", "totals": {} }], // a row with no "model" names none known + "by_tool": [{ "tool": "codex", "coverage": "covered", "reason": "โ€ฆ", "capability": {}, "totals": {}, "session_totals": {} }], // session_totals absent unless the tool has one (Copilot, today) + "by_project": [{ "project": "acme/widgets", "totals": {} }], // a row with no `project` names none known + "by_day": [{ "day": "2026-07-01", "totals": {} }], // every day in the period, in order, gaps included + "attribution": [{ "attribution": "tool-stated", "totals": {} }], + "task_attribution": [{ "attribution": "declared", "totals": {} }], // present only alongside "task" + "read": { "undated_records": 0, "unreadable_lines": 0 } +} +``` + +### Totals + +The same object appears as `totals` everywhere โ€” at the top level and on every row. + +| Field | Meaning | +| --- | --- | +| `requests` | Billed requests. Always present. | +| `cost_micro_usd` | Whole micro-dollars. Divide by 1,000,000 for dollars, at the moment of display and not before. | +| `input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_creation_tokens` | The four counters, disjoint โ€” adding all four gives total tokens without counting anything twice. | + +**An absent counter means never observed, which is not zero.** A tool whose files carry no +amount has an *unknown* cost, not a free one. Print "unknown", never `$0.00`. + +**No amount reaches this object from a local read, on any tool.** Claude Code's `cost_usd` +exists only on its OTLP export. If you are reporting on locally-read sessions, you are +reporting tokens; the rates that turn them into money live outside this repository. + +### Breakdowns + +`by_step`, `by_model`, `by_tool` and `by_project` are ordered largest first, with a stable +tie-break, so the biggest thing is the first thing you read. Ordered by `cost_micro_usd` +where a row has one, and by all four token counters summed where it does not โ€” never by +`input_tokens` and `output_tokens` alone, which every tool here dwarfs with cache. `by_day` +is the one exception: it is chronological, one row per day the period spans โ€” a series read +out of order is not a series, and a day nothing ran on is a row of zeros rather than an +omitted day. + +**Every breakdown sums exactly back to `totals`.** That is asserted, on integers, not +hoped for. + +`by_step` is keyed by the step **and** the strength of its attribution: one skill reached +once from the tool's own statement and once from a journal interval is two rows, because +they are two different claims. A row with no `step` carries `attribution: "unattributed"`. + +`by_project` carries a row with no `project` for a record stored before this field existed, +or whose session journal named none โ€” never folded into a project the reader happens to be +standing in. A record's project comes from the run journal that covered its session, not +from wherever the report itself happens to run. An empty string is treated the same as no +project at all - never its own row. + +`by_model` carries a row with no `model` the same way: both the Codex and OpenCode readers +permit a request with no model, and that record gets its own row rather than vanishing from +the breakdown while staying in `totals`. + +### `session_totals` โ€” a session total, never a sum of requests + +`by_tool` rows carry `totals`, summed from `kind: "request"` records, exactly like every +other breakdown in this object. A `by_tool` row can also carry `session_totals` โ€” present +only for a tool whose own file yields one already-complete, per-session figure rather than +per-request records. Today that is Copilot alone: its `session.shutdown` event reports the +whole session's four token counters once, at the end, never per call. + +**The two are never the same number and are never added together.** `totals.requests` +counts billed requests; a tool that has none of those (Copilot) reports `requests: 0` there +regardless of what `session_totals` carries. Read `session_totals` as its own answer to "what +did this session report", not as a fallback for a zero in `totals`. It carries no +`cost_usd` โ€” the tool's own file states no billed amount for it, only a session's tokens. + +`session_totals` is absent, never present-and-empty, for every tool that has none โ€” reading +it as `{ "requests": 0 }` by default would claim a session total was measured and found +empty, which is a different fact from the tool never producing this figure at all. + +### Attribution + +`attribution` always has exactly three rows, in this order: + +| `attribution` | Means | +| --- | --- | +| `tool-stated` | The tool named the running skill itself, on the line with the counters. Exact. | +| `journal-interval` | Derived from the interval between two boundaries the framework recorded. An inference. | +| `unattributed` | Neither source could say. | + +A strength that accounts for nothing is present with `requests: 0`. That zero is a +measurement โ€” the total is known and none of it came from that source. + +**`unattributed` does not mean no step ran.** On at least one measured tool the two are +indistinguishable, so the stronger reading would be a fact nobody measured. Do not collapse +it into anything else, and do not call it a residual. + +### Task attribution + +`task_attribution` exists only alongside `task` โ€” an unfiltered period carries no +per-record task identity to break down, so there is nothing here to say for it. Where +present it always has exactly two rows, in this order: + +| `attribution` | Means | +| --- | --- | +| `declared` | The record's own moment fell inside an interval a flow explicitly opened, by naming a file under this task's folder in a tool call โ€” a run journal `task_declared` line. Works on every tool the journal hook reaches, not only the one whose payload names a written path. | +| `inferred` | The record's session wrote into the task folder at some point, with no declared interval covering this specific record. The pre-existing, whole-session route. | + +A source that accounts for nothing is present with `requests: 0`, the same convention +`attribution` uses. There is no `unattributed` row here: every record inside a `--task` +report already matched one of the two routes, or it would not be in the report at all. + +**A declaration is bounded, never boundless.** It closes at whichever of a later +declaration or a turn boundary comes next; left open by a session that never closed it +(a crash, most often), it is capped at the last moment that session's journal actually +recorded โ€” never at "still open," which would let one long-running session's later, +unrelated work read as this task's cost. + +### Capability, per tool + +This is the field that makes the contract the same across tools. **Branch on it. Never +infer a tool's limits from whether a number happened to be present** โ€” a tool that cannot +supply an amount and a session that cost nothing look identical in the numbers. + +```jsonc +"capability": { + "local_read": { "token_counters": true, "amount": false, "tool_stated_step": false }, + "export": { "token_counters": false, "amount": false, "tool_stated_step": false }, + "journal_attributable": true, + "task_attributable": false +} +``` + +| Field | Meaning | +| --- | --- | +| `local_read`, `export` | What that route was **measured** to supply. `null` means the tool declares no such route at all, which is not the same as a declared route supplying nothing. | +| `token_counters` | That route yields the four counters. | +| `amount` | That route yields a figure denominated in currency. Never a credit or a premium request. | +| `tool_stated_step` | The tool names the running step itself. A journal interval is not this. | +| `journal_attributable` | The run journal names this tool's sessions. **False means two things:** no step can come from an interval, *and* a read that sweeps the journal never reaches one of its sessions โ€” so the tool can be perfectly readable and still report nothing until someone names a session by hand. | +| `task_attributable` | A session on this tool can be traced to the task it worked on โ€” declared, inferred, or both. False only where the journal hook never reaches a tool call for this host at all (OpenCode's plugin observes session lifecycle events alone, never one), since a declaration needs a tool call's own arguments to read. | + +`coverage` is `"covered"` or `"not-covered"`, and `reason` says why when it is the second, +or what a covered tool's figures cannot be used for. + +**Five silences, and only one is a zero.** A tool with `requests: 0` may be: not covered at +all (`coverage: "not-covered"`, read `reason`), covered but unreachable by the sweep +(`journal_attributable: false`), covered and reached and idle (a real zero), covered and +its reader failed (the human output says so; `aidd telemetry read` reports it per tool), or +covered and reporting only a `session_totals` figure โ€” `requests: 0` there is correct and +permanent for that tool, not a silence to explain away. + +### What the read could not do + +```jsonc +"read": { "undated_records": 3, "unreadable_lines": 2 } +``` + +`undated_records` are records carrying no moment at all. They belong to **no** period โ€” +the only other moment available is the day the line was stored, which is when AIDD heard +about the work rather than when it happened. `unreadable_lines` are lines no parser could +read. + +**Both non-zero means your total is partial.** Say so rather than presenting it as whole. + +## Filling it + +Records reach storage when someone runs: + +```bash +aidd telemetry read # every session the run journal knows +aidd telemetry read --session +``` + +A period that reports nothing usually means its sessions have not been read yet. + +## Known limits + +[The plugin README](../../plugins/aidd-telemetry/README.md) states what each tool can and +cannot be measured for, and why. Read it before explaining a missing figure. diff --git a/aidd_docs/product/metrics-contract.md b/aidd_docs/product/metrics-contract.md new file mode 100644 index 000000000..aea998a63 --- /dev/null +++ b/aidd_docs/product/metrics-contract.md @@ -0,0 +1,733 @@ +# Metrics contract + +This is the contract for `TelemetrySinkRecord`, the one shape every AI-tool telemetry +line takes once it reaches storage. It is written for a consumer outside this +repository โ€” a pricing service, an aggregator โ€” that needs to price and attribute a +session's usage without reading this repository's source. + +> **Writing a skill, or anything that reports on AIDD work?** Read +> [`cost-report-contract.md`](./cost-report-contract.md) instead. It describes the object +> `aidd telemetry report --json` prints, with the rules below already applied. Reading raw +> records makes you responsible for the three double-count rules, the split between the two +> record kinds, and re-read deduplication โ€” which is worth doing once, in one place, and +> that place already exists. Everything a correct +consumer needs is below: the file layout, every field's meaning and presence +condition, the three ways a naive reader double counts, and what each tool can and +cannot supply. + +## Where records live + +Records are appended as JSON Lines, one JSON object per line, to a UTC-day file: + +``` +~/.config/aidd/telemetry/YYYY-MM-DD.jsonl +``` + +or under `$AIDD_USER_CONFIG_DIR/telemetry/YYYY-MM-DD.jsonl` when that environment +variable is set. A day file is append-only for its whole life โ€” lines are never +rewritten in place, only added. A session's records can span more than one day +file if the session crosses midnight. + +Every record carries `sink_schema_version` (currently `2`). A consumer that does +not recognize the version on a line should set that line aside rather than guess +its shape โ€” a version exists precisely so a future, incompatible shape does not +get read as this one. + +## The two record kinds, and why they are never summed + +Every record's `kind` is either `"request"` or `"session"`, and the two measure +overlapping quantities in incompatible ways. + +**`kind: "request"`** is one line per billed request โ€” one line per call to the +model that produced a charge. `cost_usd` and the four token counters +(`input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_creation_tokens`) on +a `"request"` line are complete for that request: summing every `"request"` line +for a session gives that session's true total. + +**`kind: "session"`** is a periodic delta of the same quantities, taken from a +metrics export that flushes on a fixed interval (10 seconds, for Claude Code โ€” +`OTEL_METRIC_EXPORT_INTERVAL`) with delta aggregation temporality +(`aggregationTemporality: 1` in the OTLP payload): each flush reports only what +changed *since the previous flush*, not a running total. A `"session"` line is +**not** a per-session cumulative figure, and it is not guaranteed complete โ€” +whichever flush windows happened to be exported before the process exited are +what got captured, and no more. Summing `"session"` lines therefore does not +reliably reproduce a session's true total, even before double-counting against +`"request"` lines is considered. + +Copilot's is the exception that shows why the kind is drawn where it is: read +locally rather than exported, it is a **one-shot cumulative total** written once +at shutdown rather than a delta of a flush window. Both meanings share the rule +that matters โ€” a `"session"` line is never added to a `"request"` line, because +one already contains what the other counts โ€” so they share the kind. What +separates them is `provenance`: `"export"` for a flush delta, `"local-read"` for +a total a tool wrote for itself. A consumer that needs to tell them apart reads +that field, and no other. + +**Measured on one captured session** (Claude Code, `session.id` = +`22177147-d8cb-4ee1-976f-0ef82bd62491`, captured 2026-08-20): + +| Source | Kind | Lines | `cost_usd` total | +| ----------------------------------------------- | ----------- | ----- | ----------------- | +| `otlp-logs-claude-code-subagent.json` fixture | `"request"` | 2 | **$0.1605** | +| `otlp-metrics-claude-code.json` fixture | `"session"` | 1 (of 6) | **$0.0151** | + +This is not a contradiction: the request lines are every billed request the +session made; the metric line is one 10-second flush window's own delta. Summing +the two ($0.1605 + $0.0151 = $0.1756) overstates the session's true cost, and +using only the metric total ($0.0151) understates it by an order of magnitude, +because only one flush window was ever captured for this session. + +**Rule: take `cost_usd` and the four token counters from `kind: "request"` lines +only.** Take `active_time_s` from `kind: "session"` lines only โ€” no `"request"` +line, on any tool measured so far, carries active time; it exists solely as a +`"session"`-kind metric. + +### One line per datapoint, never merged + +A `kind: "session"` line is one metric datapoint, never merged with any other +datapoint from the same flush. The captured session above produced **six** +`"session"` lines for one flush window, one per datapoint: `cost_usd`, +`input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_creation_tokens`, and +`active_time_s` โ€” each its own line, each carrying only that one field among the +six (the other five are absent on that line). A consumer expecting one +`"session"` line per session, or one line per flush bundling every quantity +together, reads a fifth (or a sixth) of the truth per line it looks at. + +## The other way to double count: a re-read appends unless matched + +Local reading (`provenance: "local-read"`) works by re-opening a tool's own +transcript file, which keeps growing for as long as the session runs. Each read +sees every turn the file holds so far, not just what is new since the last read. +To keep a re-read from storing the same turn twice, the writer matches each +candidate record against what is already stored for that session, on `turn_id` +alone (never on line content, never on arrival order โ€” a hash of the line changes +the moment the tool appends anything else to that same record). + +**This match requires a `turn_id`.** A candidate with no `turn_id` cannot be +matched against anything, and is appended again on every read that sees it โ€” by +design, not by omission: inventing an unstable key would be worse than leaving it +unmatched. + +**A matched candidate is not always dropped โ€” a still-open turn can correct +itself.** Some tools (Codex's rollout is the one measured so far) emit a +`kind: "request"` record for a turn before the tool itself has said the turn is +finished: a Codex turn is closed by the *next* `turn_context` line, and the last +turn in a file whose session is still running has none, so a rollout read while +its session runs is stored with whatever counters had arrived by then โ€” partial, +and, without this rule, permanently so, since the next read's completed figures +would match the same `turn_id` and be silently dropped. Instead: a matched +candidate lands as a second line โ€” the sink is append-only, so this is never an +edit of the stored one โ€” whenever it is a `kind: "request"` local-read record +that **strictly improves** on the largest already stored for that `turn_id` +(every counter at least as large, at least one larger). Nothing else gates it: +a run journal's `turn_end` line, where one exists, says only that no further +growth is coming โ€” it is never asked before accepting a correction, because a +candidate that strictly improves is itself the only proof needed that an +earlier reading was not final, whatever a journal's own clock says about it. +Idempotence โ€” a re-read that brings nothing larger stores nothing new โ€” falls +out of "strictly improves" alone, the same property an unmatched-then-matched +`turn_id` already had; nothing here depends on a session ever being confirmed +finished. A `kind: "session"` record sharing a `turn_id` (Copilot's shutdown +total, keyed on the shutdown event's own id) is never treated as a correction +opportunity: it is a one-shot cumulative figure with no provisional reading to +correct, and is dropped on a re-read exactly as before. + +Because more than one record can now legitimately share a `turn_id` on the +sink, a consumer reading raw records must also collapse them before summing โ€” +see "Consuming a session correctly" below. The largest wins, never a sum of two: +a later record that reads *smaller* than one already stored is never written at +all, so a stored `turn_id` group never needs to guard against a shrink, only +against counting more than one of its members. + +**Worked example**, mirroring the tested behavior of the local-read use case: one +turn's transcript line carries 10 input tokens and 20 output tokens, under +`turn_id: "req_1"`. A first read appends it โ€” 30 tokens stored. The session +continues and the same transcript file is read again (a re-read, same turn still +present in the file, no larger reading of it yet). Because `req_1` is already +stored and the new reading offers nothing more, the second read matches it and +stores nothing new. **Stored total after the second read: 30 tokens, not 60.** +Had the same candidate carried no `turn_id`, the second read would have appended +it again, and the stored total would have become 60 tokens for one real turn. +Had the second read's own transcript line instead carried 15 input tokens and 25 +output tokens for the same still-open turn, it would land as a second, +correcting line, and the group's true total would be 40 tokens โ€” the larger +reading, not 30, not 70. + +A consumer aggregating raw appends from the sink file without replicating this +`turn_id` match โ€” for example, re-implementing a local reader against a tool's +own files rather than consuming this sink โ€” will double, triple, or *N*-times +count any record whose route has no stable per-record identifier, once per read +of an active session; and, for a route whose turns can be re-read while still +open, will sum a turn's own successive readings of itself rather than keeping +only the largest. + +## A third way to double count: one billed call, seen by both routes + +A tool whose export and local read are both declared and measured โ€” today, +only Claude Code โ€” can have both routes live for the same session at once: the +OTLP export streams `api_request` as each call completes, and a local read of +the same session's transcript, run at any point, sees the same calls in the +tool's own file. Each route names the call in a namespace the other never +reads โ€” export's `turn_id` is `prompt.id`, one user turn, which a main-agent +request and the subagent it spawned can share (see `turn_id` below); local's +`turn_id` is `requestId`, one billed call โ€” so matching on `turn_id` alone, as +the re-read rule above does, never catches this: the two routes' records for +the same real call never share that key, and a consumer that only guards +against the re-read case sums both. **Measured on this repository's own +captured export fixture** (`otlp-logs-claude-code-subagent.json`, a +main-agent request and the subagent request it spawned): naively unioning +those two export records with what a local read of the same two calls would +produce doubles every figure โ€” four `"request"` lines instead of two, every +token counter twice its true value. + +**The fix is not a write-time match.** The sink is append-only (see "Where +records live"): a record already stored can never be corrected in place, only +reconciled by whatever reads it back. `billed_request_id` is the field that +makes the reconciliation possible โ€” the one identifier measured so far that +both routes compute the same value for, for the same real call (see +`billed_request_id` below). A consumer building a session's true totals groups +`"request"` records sharing `tool`, `vendor_id` and `billed_request_id` and +treats them as one billed call: keep the group's `cost_usd` and four token +counters from whichever record carries `cost_usd` at all (on every tool +measured so far, that record's counters are also the complete ones for the +call), and take `step_attribution`/`step`/`step_plugin` from whichever record +resolved one, so a call seen by both routes still shows the export's money and +the local read's tool-stated step, never one thrown away for the other. A +record with no `billed_request_id` joins no group and is counted exactly as it +arrived โ€” the same rule an unmatched `turn_id` follows for the re-read case. +`cost-report-contract.md` calls this the third of the double-count rules +`aidd telemetry report --json` has already applied. + +**The same collapse also absorbs a retried OTLP delivery.** `/v1/logs` and +`/v1/metrics` are received unconditionally โ€” nothing at write time recognizes a +redelivered payload, because OTLP delivery is itself at-least-once and a +receiver refusing a delivery it cannot prove is a duplicate would risk refusing +a real one. A redelivered `api_request` names the same `billed_request_id` as +the first delivery, so the group it joins on read has two (or more) identical +records instead of one, and the same "keep one, never sum" rule that reconciles +two routes reconciles two deliveries of one route just as well. + +**This protection requires `billed_request_id`, and most declared exports do +not carry one.** Measured so far, only Claude Code's export names it (its +`request_id` log attribute). Codex's and Copilot's exports are declared โ€” +`identityAttribute` is real, measured from a captured session โ€” but neither has +ever been measured carrying a `request_id`, or any counter at all through this +route; Cursor's and OpenCode's exports are not even declared (`kind: +"unmeasured"`), so this receiver never resolves an identity for either and +stores nothing from them regardless. A route with no `billed_request_id` has no +group to collapse into: a retried delivery for it is indistinguishable from two +real calls, and doubles every counter it carries. Today this is a latent gap, +not an observed one โ€” the one route currently seen carrying real counters +(Claude Code's) is also the one route that is protected โ€” but a consumer must +not assume a future export automatically inherits this collapse; it inherits it +only once that route is measured naming `billed_request_id` too. + +## Identity and joins + +- **`tool`** names which AI tool produced a record, as a fact stated on the + record itself. A consumer never infers the tool from the name of another + field (`vendor_field`, `vendor_id`) โ€” that attribute name differs by tool + *and by route*: the same Claude Code session identifier is named `sessionId` + when read locally and `session.id` when exported. Reversing the attribute name + back into a tool identity works only until a tool reuses another's attribute + name. +- **`vendor_id`** is that tool's own session identifier, as a string, in + whatever form the tool itself uses it โ€” a UUID for Claude Code and Codex, an + OpenCode `ses_โ€ฆ` id, and so on. **`vendor_field`** names which attribute + carried it (`sessionId`, `session.id`, `session_meta.id`, `sessionID`, + `conversation.id`, `gen_ai.conversation.id`, depending on tool and route). Two + records with the same `tool` and the same `vendor_id` describe the same real + session, regardless of which route produced either one, since the identifier + value itself is the tool's own and does not change between its local file and + its export. +- **`project_id`** is the repository a session ran in, when it is known. + On the export route it is set directly from the `aidd.project_id` resource + attribute, with no join and no `project_field`. On the local-read route it + is joined from the run journal's own `session_start` line, which already + resolves `project_id` and `project_remote` for the repository the hook + fired in โ€” the same value never re-derived from wherever the reader + happens to be standing. **`project_field`** names which of the journal's + two fields the value came from, present only on a record joined this way: + `"project_remote"` when the journal named a git remote (the same value for + every checkout of one repository), `"project_id"` otherwise (a directory + name, which can collide across machines). A record with neither field + belongs to no known project โ€” never guessed at from the current + repository, and never dropped. +- **`turn_id`** is the tool's own identifier for one turn or request, when the + tool's file or export can name one. It is the key local-read re-reads are + matched on (above), but **it is not guaranteed unique to one billed request**: + measured on the captured session above, a main-agent request and the subagent + request it spawned share one `prompt.id` โ€” two `"request"` lines, $0.1086 and + $0.0519, both under the same `turn_id`. Do not use `turn_id` as a primary key + for billed requests; use it only for the re-read match it exists for. + **`turn_field`** names which attribute carried it. +- **`person_id`** is not a tool's own, uncontrolled user attribute. This sink + once stored that as `user_id`, mapped straight from whatever `user.id` an + export happened to carry, regardless of whether the person using the tool + agreed to be named; that field is gone, and an export-provenance record now + carries no identity of any kind. `person_id` is the opposite in every way + that mattered: an identifier a person generated for themselves, on their own + machine, opted into per person rather than defaulted on for a whole export. + It is present only on a `provenance: "local-read"` record โ€” the one route + guaranteed to run as that person, on that machine โ€” and never on an + `"export"` record, since an OTLP receiver is not guaranteed to run on the + identified person's own machine. `person_id` is never derived from `user_id`, + a git author, an email, or a hostname, in either direction. See `person_id` + and `person_display_name` below for what each carries and when. + +## Step attribution + +Every record states **how**, not just whether, its step is known, via +`step_attribution`: `"tool-stated"` (the tool itself reported the running +step, exact for that record), `"journal-interval"` (derived: the record's own +moment fell inside a step's start/end interval recorded by AIDD's run journal โ€” +an inference, not a measurement), or `"unattributed"` (no step could be +determined by either route). `step_attribution` is always present; it is never +omitted, because an absent field here would read as "no step ran," which is +exactly the assertion nothing on a transcript or a journal can support. + +`step` (the skill or step name) is present exactly when `step_attribution` names +a source that found one โ€” absent, never a placeholder, when `step_attribution` +is `"unattributed"`. `step_plugin` (the plugin the step came bundled with) is +present only when `step_attribution` is `"tool-stated"` and the tool reported a +plugin alongside the step name; a journal interval never carries a plugin at +all, so `step_plugin` is absent whenever `step_attribution` is +`"journal-interval"`, even though `step` itself is present there. + +**`step_attribution: "unattributed"` does not mean "this request ran outside any +step."** Claude Code's own attribution field is omitted from its transcript both +when no skill was running and when the running Claude Code version predates the +field (it arrived around version 2.1.220). Measured across 40 real transcripts +and twelve versions, there is not one `null` value that distinguishes the two +cases โ€” the field is omitted identically either way. A consumer that reads +`"unattributed"` as "confirmed to be outside any step" is asserting a fact the +data cannot support. Read it only as: no step could be determined for this +record, for whatever reason. + +## Field reference + +Every field below states its type, when it is present, what it means, and โ€” +because an absent counter and a zero counter are different facts โ€” what its +absence means. + +### Always present + +#### `sink_schema_version` +- **Type**: number. +- **Present**: always. +- **Meaning**: the wire format version this line was written under. Currently `2`. +- **If absent**: never absent on a well-formed line; a line missing it, or + carrying a version a consumer does not recognize, should be set aside rather + than parsed as if its shape were known. + +Adding `person_id` and `person_display_name` was not a version bump: a +consumer built against version 2 that never wrote or read either field never +sees them, and neither changes what any field it already understood means โ€” +the same rule `cost_report_version` follows in `cost-report-contract.md`. + +Removing a field is neither of the two cases above โ€” it neither adds +something ignorable nor changes what a kept field means. Whether it needs a +bump turns on whether a version has shipped: once a consumer could be reading +a real line, taking the field away without warning breaks it silently. Before +that, the risk does not exist โ€” nobody can depend on a field no released +build ever produced. Removing `user_id` from the allowlist is not a version +bump for exactly that reason: `sink_schema_version` 2 has never been +released, so no consumer anywhere has read a real line carrying it. A day +file a pre-release build already wrote may still carry `user_id` on an old +line โ€” the sink is append-only (see "Where records live") โ€” and a reader +ignores it exactly as it would any other field it does not recognize. + +#### `kind` +- **Type**: `"request"` or `"session"`. +- **Present**: always. +- **Meaning**: which of the two measurement kinds this line is โ€” see "The two + record kinds" above. +- **If absent**: never absent. + +#### `provenance` +- **Type**: `"export"` or `"local-read"`. +- **Present**: always. +- **Meaning**: which route produced this line โ€” a tool's OTLP export received + over `/v1/logs` or `/v1/metrics`, or a tool's own file read directly from disk. + Never defaulted, so a third route arriving later cannot be mistaken for one of + these two. +- **If absent**: never absent. + +#### `tool` +- **Type**: one of `"claude"`, `"cursor"`, `"copilot"`, `"opencode"`, `"codex"`. +- **Present**: always. +- **Meaning**: the AI tool that produced this record, stated directly โ€” see + "Identity and joins" for why this is never inferred from another field. +- **If absent**: never absent. + +#### `vendor_id` +- **Type**: string. +- **Present**: always. +- **Meaning**: the tool's own session identifier โ€” see "Identity and joins." +- **If absent**: never absent. + +#### `vendor_field` +- **Type**: string. +- **Present**: always. +- **Meaning**: which attribute on the source payload carried `vendor_id` โ€” the + route as much as the tool (the same tool's own identifier can be named + differently on its local file versus its export). +- **If absent**: never absent. + +#### `step_attribution` +- **Type**: `"tool-stated"`, `"journal-interval"`, or `"unattributed"`. +- **Present**: always. +- **Meaning**: how the step (if any) was determined โ€” see "Step attribution." +- **If absent**: never absent, deliberately โ€” see "Step attribution" for why an + absent field here would be misread. + +### Identity and joins (conditional) + +#### `turn_id` +- **Type**: string. +- **Present**: conditional โ€” when the producing route can name a stable + identifier for this specific turn or request. Present on most `"request"` + lines measured so far (Claude Code, Codex, OpenCode). Never present on any + export-route metric datapoint (no `"session"`-kind OTLP datapoint measured so + far carries a turn identifier), but present on Copilot's local-read + `"session"` line โ€” its one-shot shutdown total is keyed on the shutdown + event's own id, so a re-read can match it the same way a `"request"` line's + is matched. +- **Meaning**: the tool's own turn/request identifier, and the key a local + re-read is matched on. **Not guaranteed unique per billed request** โ€” a + main-agent request and the subagent request it spawns can share one `turn_id` + on the export route. A `kind: "request"`, `provenance: "local-read"` turn is + the one shape that can also be *corrected*, not just matched โ€” see "A re-read + appends unless matched" above; a `"session"`-kind turn (Copilot's) and an + export-route turn are never corrected this way, only matched-and-dropped or + left unmatched. +- **If absent**: this record's route has no stable per-record identifier to + offer. It cannot be matched by a re-read, and will be appended again, once per + read, for as long as the session's underlying file keeps being re-read โ€” see + "A re-read appends unless matched." + +#### `turn_field` +- **Type**: string. +- **Present**: conditional โ€” present exactly when `turn_id` is present. +- **Meaning**: which attribute on the source payload carried `turn_id` + (`requestId`, `prompt.id`, `turn_id`, `id`, depending on tool and route). +- **If absent**: `turn_id` is also absent on this record. + +#### `billed_request_id` +- **Type**: string. +- **Present**: conditional โ€” measured so far only for Claude Code, on both routes: + its export names it via the `request_id` attribute on the `api_request` log + record, and its local transcript names it via `requestId` โ€” the same + attribute the local route already uses for `turn_id`. No other tool or route + has ever been measured naming a billed call this way. +- **Meaning**: the tool's own identifier for one billed call, and, unlike + `turn_id`, **guaranteed unique per billed request where it is present at + all** โ€” a main-agent request and the subagent request it spawns each carry + their own. It exists so a consumer can collapse two records describing one + real call โ€” made when both an export and a local read are live for the same + tool at once โ€” into one, instead of summing both. See "One billed call, + both routes" below. Never used for the local-read re-read match `turn_id` + exists for, and never a primary key for anything beyond that collapse. +- **If absent**: this record's route has no stable per-call identifier to + offer beyond `turn_id`, or the record's tool has never been measured + carrying one. A record with no `billed_request_id` is never collapsed with + any other โ€” it is kept exactly as it arrived, the same rule an unmatched + `turn_id` already follows for a local re-read. + +#### `step` +- **Type**: string. +- **Present**: conditional โ€” present exactly when `step_attribution` is + `"tool-stated"` or `"journal-interval"`. +- **Meaning**: the skill or step name that was running. +- **If absent**: `step_attribution` is `"unattributed"` โ€” no step name is known, + which is a different fact from "no step was running." Never a placeholder + string. + +#### `step_plugin` +- **Type**: string. +- **Present**: conditional โ€” present only when `step_attribution` is + `"tool-stated"` *and* the tool reported a plugin name alongside the step. +- **Meaning**: the plugin the stated step came bundled with. +- **If absent**: either no step is known, the step came from a journal interval + (which never carries a plugin), or the tool named a step with no plugin. + +#### `project_id` +- **Type**: string. +- **Present**: conditional. On the export route: present when the emitting + environment set a project identity (the `aidd.project_id` resource + attribute). On the local-read route: present when the run journal's + `session_start` line named a project for the session โ€” see "Identity and + joins." +- **Meaning**: the repository this session ran in. +- **If absent**: no project identity is known for this record โ€” read as + belonging to no known project, never attributed to a guess. + +#### `project_field` +- **Type**: `"project_id"` or `"project_remote"`. +- **Present**: conditional โ€” present only on a local-read record whose + `project_id` was joined from the run journal; absent on an export-route + record, whose `project_id` is set directly with no journal join to name a + source for. +- **Meaning**: which of the journal's own two fields `project_id` came + from โ€” see "Identity and joins." +- **If absent**: either the record carries no `project_id` at all, or it + does and came from the export route. + +#### `person_id` +- **Type**: string. +- **Present**: conditional โ€” present only on a `provenance: "local-read"` + record, and only when the machine that read it holds a file recording that a + person opted in. Never present on a `provenance: "export"` record. +- **Meaning**: a stable identifier a person generated for themselves and chose + to attach, on their own machine โ€” never derived from the export route's + now-removed `user_id`, a git author, an email, or a hostname. Withdrawing + removes the file this comes from; records already written keep whatever + they were stamped with, since a day file is never rewritten in place (see + "Where records live"). +- **If absent**: nobody opted in on the machine that produced this record โ€” + the default for a fresh installation โ€” or the record came from the export + route, which never carries this field regardless of any opt-in. + +#### `person_display_name` +- **Type**: string. +- **Present**: conditional โ€” present only alongside `person_id`, and only once + the person separately asked for a display name to be shown. Setting one + never happens as part of opting in. +- **Meaning**: a name the person chose to show beside their figures. Never + derived from `person_id`, and never used to derive it. +- **If absent**: either nobody opted in at all, or they opted in without ever + setting a display name โ€” the ordinary state, not an incomplete one. + +### Cost and token counters (conditional) + +#### `cost_usd` +- **Type**: number, US dollars. +- **Present**: conditional. On `"request"` lines: present on every + export-route record (a log record without `cost_usd` is not a billed request + and is never turned into a record at all) and **never** present on a + local-read record for any tool measured so far โ€” no local reader has + captured a billed amount from a tool's own file. On `"session"` lines: + present on exactly the one (of six) datapoint lines per flush that carries + the cost measure. +- **Meaning**: the billed amount for this request, or this flush window's delta. +- **If absent**: on a local-read `"request"` line, this route cannot see a + billed amount for this tool at all โ€” see the coverage table. On a + `"session"` line, this is one of the other five datapoints in the flush, not + the cost one. + +#### `input_tokens`, `output_tokens`, `cache_read_tokens`, `cache_creation_tokens` +- **Type**: number. +- **Present**: conditional, and independently per field. On `"request"` lines: + Claude Code (both routes) reads all four together or none โ€” a partial + `usage` object yields no record at all, rather than a record with a missing + counter silently read as zero. Codex reads each independently: a counter a + turn never reported (Codex sometimes omits `cache_write_input_tokens` + entirely, rather than sending zero) stays unset on that record rather than + being summed in as a fabricated zero. On `"session"` lines: exactly one of + these four fields is present per line โ€” see "One line per datapoint, never + merged" โ€” the other three, plus `cost_usd` and `active_time_s`, are absent on + that same line. +- **Meaning**: token counts for the request or the flush delta, normalized to + mean the same thing across tools (OpenAI's Responses API convention makes + Codex's raw `input_tokens` *inclusive* of its cached figure; this field + subtracts the cache figure out, matching Claude Code's already-exclusive + convention). +- **If absent**: this specific counter has no known value for this record โ€” a + fact distinct from a stored `0`, which means the tool reported the counter + as exactly zero. + +#### `model` +- **Type**: string. +- **Present**: conditional โ€” present when the producing route names a model + for this record (Claude Code, on both routes and both kinds; Codex, on + `"request"` lines via its own `turn_context`). +- **Meaning**: the model identifier the tool itself used, unmodified. +- **If absent**: this route did not carry a model name for this record. + +#### `effort` +- **Type**: string. +- **Present**: conditional โ€” present when the route carries it (Claude Code, + both routes; Codex, local read). +- **Meaning**: the tool's own effort/reasoning-level setting for the request. +- **If absent**: not carried by this tool's route. + +#### `speed` +- **Type**: string. +- **Present**: conditional โ€” measured so far only on Claude Code's export + route. +- **Meaning**: the tool's own speed tier for the request. +- **If absent**: not carried by this tool's route. + +#### `query_source` +- **Type**: string. +- **Present**: conditional โ€” measured so far only on Claude Code's export + route (values seen: `"main"`, `"sdk"`, `"agent:builtin:general-purpose"`). +- **Meaning**: what originated the request within the tool (its own + main loop, its SDK, a named built-in agent). +- **If absent**: not carried by this tool's route. + +#### `agent_name` +- **Type**: string. +- **Present**: conditional โ€” present when the record is a subagent's own + request. On Claude Code: set from the export's `agent.name` attribute, and + from the local transcript's `attributionAgent` field when the transcript + line is itself marked as a subagent line (`isSidechain: true`). +- **Meaning**: which named subagent made this request. +- **If absent**: for Claude Code, this was the main agent's own request, not a + subagent's. For every other tool measured so far, this field is never set at + all โ€” its route does not name subagents as a concept, so its absence there + says nothing about whether one ran. + +#### `duration_ms` +- **Type**: number. +- **Present**: conditional โ€” measured so far only on Claude Code's export + route. +- **Meaning**: the request's own wall-clock duration, in milliseconds. +- **If absent**: not carried by this tool's route. + +#### `active_time_s` +- **Type**: number. +- **Present**: conditional โ€” the field target of exactly one `"session"`-kind + metric measure, measured so far only for Claude Code + (`claude_code.active_time.total`). Never present on any `"request"` line, on + any tool. +- **Meaning**: seconds of active engagement Claude Code measured during this + flush window โ€” not wall-clock time, and not a per-request figure. +- **If absent**: no `"request"` line carries this at all โ€” it exists solely as + a `"session"`-kind measure; on a `"session"` line, this is one of the other + five datapoints in the flush, not the active-time one. + +#### `event_timestamp` +- **Type**: string, ISO 8601. +- **Present**: on every route measured so far, from its own source: + - **Export**, both kinds: the OTLP record's own `timeUnixNano` (nanoseconds + since the epoch, converted here to milliseconds). The `event.timestamp` + attribute is read in preference when a payload carries one, but no captured + payload ever has. + - **Claude Code, local**: the transcript line's `timestamp` field. + - **Codex, local**: the turn's own *start*, from the `turn_context` event โ€” + not a moment inside the turn. A record spans a whole turn, so a moment + inside it would claim a precision the record does not have. + - **OpenCode, local**: the message's `time.created`, in epoch milliseconds. + Not `time.completed`, which is absent on some counted messages โ€” a field + that sometimes means "started" and sometimes "finished" is worse than one + that always means the same thing. +- **Meaning**: when the work this record measures happened. Two consumers rely + on it and they are separate: attributing a record against a run-journal step + interval when `step` is not already tool-stated, and placing the record in a + reporting period. +- **If absent**: two things become impossible, and neither may be substituted + for. The record can no longer be attributed via a journal interval (only via + a tool-stated `step`, if one exists), so it falls back to + `step_attribution: "unattributed"`. And it belongs to **no period**: the only + other moment available is the day file it was appended to, and that is when + the record was received, not when the work ran โ€” a session read locally days + after it happened lands in the day file for the day it was *read*. A consumer + reports such records as undated; it never places them by their day file. + +#### `event_sequence` +- **Type**: number. +- **Present**: conditional โ€” measured so far only on Claude Code's export + route. +- **Meaning**: a monotonic counter the tool emits alongside its events. +- **If absent**: not carried by this tool's route. + +## Per-tool coverage + +Coverage is not uniform across tools, and it is not uniform across routes for the +same tool. A tool absent from one route is not a zero for that route โ€” it is +"not covered," and a consumer should print it that way rather than infer a zero +from silence. + +| Tool | Export route | Local-read route | +| ---- | ------------- | ------------------ | +| **Claude Code** | Declared and measured: full request-level counters via `/v1/logs`, plus the six `"session"`-kind delta metrics via `/v1/metrics` every 10 seconds. `cost_usd` is only ever available through this route โ€” no local file carries it. Also the only route measured naming `billed_request_id` (its `request_id` attribute). | Declared and measured: complete token counters per assistant message, keyed on `requestId`. Step is stated by the tool itself (`attributionSkill`), exact per message โ€” the strongest attribution any tool or route offers. No `cost_usd`. Also names `billed_request_id` (the same `requestId`). **The one tool measured by both routes at once**: when both are live for a session, both routes append a record for the same billed call under two different `turn_id`s (`prompt.id` here, `requestId` there) โ€” collapse on `billed_request_id` before summing, or every figure doubles. See "A third way to double count." | +| **Codex** | Declared (`conversation.id` measured, zero-token, to verify the identifier only). Turn identifier and any metrics export are unmeasured โ€” no counters, no cost, flow through this route today. | Declared and measured: complete counters per turn, keyed on `turn_id`, from the rollout's `token_count` events paired with the preceding `turn_context`. A turn read while its session is still running is stored with whatever counters had arrived so far, and corrected โ€” never edited, a second line โ€” once a later read brings a larger reading of the same `turn_id`; see "A re-read appends unless matched." No tool-stated step โ€” attribution is only ever a run-journal interval, or unattributed. No `cost_usd`. | +| **OpenCode** | Unmeasured โ€” no export payload has ever been captured for this tool. | Declared and measured, via `opencode export --sanitize`: counters per request (message), keyed on the message's own `id`. No established join to a run-journal entry โ€” no captured hook or plugin payload has ever carried OpenCode's own session identity, so nothing exists to join on; these figures answer only what a session consumed, alone. `info.cost` is deliberately never read: it is `0` in every message captured, and its denomination (which currency, computed vs. billed) has never been established โ€” a figure whose meaning is unknown is worse than an absent one. Records carry `event_timestamp` from the message's `time.created`, so they can be placed in a period; step attribution stays out of reach regardless, since there is no join to a run journal to attribute against. | +| **Copilot** | Declared (`gen_ai.conversation.id` measured, zero-credit, to verify the identifier only) โ€” but that attribute lives on the `invoke_agent` *span*, not on a log record or a metric, and this receiver only listens on `/v1/logs` and `/v1/metrics`. A receiver limited to those two paths never sees the one attribute that identifies a Copilot session, so this route yields nothing in practice today. | Supported at **session** granularity only: `session.shutdown` in `~/.copilot/session-state//events.jsonl` carries input, output, cache read and cache write together, once, for the whole session. Nothing in its files counts a single request, so it yields a `session` record and never a `request` one, and no amount can be placed inside a step. Read `tokenDetails`, never `usage` โ€” the latter is inclusive of cache writes where every other reader's input is exclusive. No model is stamped: `currentModel` names the session's last model, not the one that spent. Separately, its file's own `cost` field is denominated in premium requests, not currency, so it could not be treated as `cost_usd` even where it is present. | +| **Cursor** | Unmeasured โ€” no payload has ever been captured. Cursor's own documentation names `cursor.conversation.id`, but a name read from documentation is a guess, and enabling the export to verify it is a team setting on an Enterprise plan, in beta, that nobody outside a Cursor admin can turn on โ€” so it is declared unmeasured rather than declared from an unverified guess. | Unsupported (probed): Cursor writes no token count in any file it produces โ€” there is nothing on disk for a local reader to find. | + +Cursor is the one tool uncovered by both routes today: its export cannot be +enabled here to measure, and its local files carry nothing to read. + +### Attributing records to a task + +A record carries no task identity, on any route. A task is derived by whatever +reads the records, from two kinds of line the run journal records beside them. +That derivation is deliberately not stored: a conclusion frozen at write time +cannot be revised, while a derivation re-runs over every past session the day +it changes. + +**A written file.** The journal hook reads a written path from the tool's own +hook payload, and only Claude Code's carries one in a readable form: Copilot's +and Cursor's were never captured doing so, and Codex writes through an +`apply_patch` command string that would have to be parsed rather than read. A +session whose journal names a written path this way belongs, as a whole, to +whatever task that path resolves to. + +**A declared ticket.** `task_declared` records that a tool call's own +arguments named a file under a task folder โ€” the same move `step_start` +already makes for which skill is running, and it asks nothing of a payload's +shape. It reaches every host the journal hook dispatches a tool-call event +for, which today is every declared host except OpenCode: its plugin observes +only session lifecycle events, never an individual tool call, so there is no +payload for a declaration to read arguments out of. A declaration is an +interval, not a whole-session fact โ€” it opens where the tool call happened and +closes at whichever of a later declaration or a turn boundary comes next, or, +left open, at the last moment that session's journal actually recorded. Only a +record whose own moment falls inside that interval belongs to the task by +this route; the rest of the session falls back to whether it wrote into the +folder, exactly as before. + +A session on a tool that produces neither kind of line is attributable to a +**period** and, where a journal covers it, to a **step** โ€” but never to a +task. A consumer prints that as a limit of the tool, exactly as it prints "not +covered": a session with no task is not a session that touched nothing. + +The Copilot denomination is measured, though not from anything in this +repository โ€” it comes from reading that tool's own session files, and is +recorded here so the claim is auditable rather than taken on trust. Across +fourteen local sessions, `modelMetrics..requests.cost` sits at `0.33` +for every single-request `claude-haiku-4.5` session while `totalNanoAiu` +ranges from 2.04 to 2.95 billion and output ranges from 46 to 154 tokens; +a five-request `gpt-5-mini` session reads `0`. The figure tracks request +count times a per-model multiplier and is invariant to consumption, which is +what makes it premium requests rather than currency. + +## Consuming a session correctly + +To compute one session's true totals from a set of stored records: + +1. Group records by matching `tool` and `vendor_id` โ€” that pair names one real + session, regardless of which `provenance` produced any individual record. +2. Within that group, collapse every `kind: "request"`, `provenance: "local-read"` + record sharing a `turn_id` into the one carrying the largest `input_tokens` + + `output_tokens` + `cache_read_tokens` + `cache_creation_tokens` โ€” never a sum + of the group, which would state a combination of counters the tool's own file + never actually reported together. This is what a still-open turn re-read more + than once (Codex, so far) leaves behind โ€” see "A re-read appends unless + matched." Restricted to `kind: "request"` and `provenance: "local-read"`: a + `"session"`-kind turn (Copilot's) is a one-shot total with nothing to + collapse, and an export-route `turn_id` is a prompt id several distinct + billed calls can share, so applying this step there would merge them. +3. Then collapse every `kind: "request"` record sharing a `billed_request_id` + into one before summing anything โ€” see "A third way to double count: one + billed call, seen by both routes." A record with no `billed_request_id` is + never collapsed with another. Order between steps 2 and 3 does not matter โ€” + the two key on disjoint fields โ€” but doing step 2 first means a still-open + local-read turn is already down to one record before it is ever compared + against the export route's record for the same call. +4. Sum `cost_usd`, `input_tokens`, `output_tokens`, `cache_read_tokens`, and + `cache_creation_tokens` from the collapsed `kind: "request"` records in that + group only. Never include `kind: "session"` records in this sum. +5. Sum `active_time_s` from `kind: "session"` records in that group only โ€” no + `"request"` record carries it. +6. Do not key anything else on `turn_id` beyond what it is documented for here: + it is a write-time match-and-correct key for local-read re-reads, not a + unique identifier for a billed request. `billed_request_id`, not `turn_id`, + is what step 3 above collapses on, precisely because `turn_id` lacks that + guarantee. +7. Where a tool's row above says a route is not covered, or covered without an + amount, report that plainly rather than defaulting the missing figure to + zero. diff --git a/aidd_docs/runs/.gitkeep b/aidd_docs/runs/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/aidd_docs/runs/README.md b/aidd_docs/runs/README.md new file mode 100644 index 000000000..666890c0f --- /dev/null +++ b/aidd_docs/runs/README.md @@ -0,0 +1,24 @@ +# aidd_docs/runs + +Where the run journal's records land once AIDD telemetry is turned on. This directory being present or committed is **not the permission**. The single authoritative switch is `.aidd/config.json`'s `telemetry.enabled`, read by `plugins/aidd-telemetry/hooks/journal.js` at the point of every write, never cached across a session. With that switch on, `aidd_docs/runs/` is created on demand if it does not already exist; with it off, no record lands here regardless of whether this directory exists. Records are ignored by git (see `.gitignore`), so cloning the repository never carries anyone's session history. + +## Shape + +One file per session: `__.jsonl`. One JSON object per line, appended and never rewritten โ€” a JSON object is a closed block that can only be rewritten whole, so this is what lets a session leave two hundred observations at the cost of two hundred appends instead of two hundred rewrites, and what lets a process that dies mid-write lose at most the one line it was writing. + +`schema_version: 2` on the `session_start` line. Version 1 was a single mutable ten-key object per session, rewritten on every turn (`ended_at`, `tasks[]`, `parent_run_id` among its fields) โ€” replaced because a value frozen at write time cannot be revised, and the hook was writing conclusions (an interval a task attached to, a session's end time) instead of observations. + +Every line carries `at` (ISO 8601, UTC, second precision) and `type`: + +| `type` | Carries | Fired by | +| --- | --- | --- | +| `session_start` | `schema_version`, `run_id`, `project_id`, `project_remote`, `tool`, `vendor_id`, `vendor_field`, plus `worktree_id` and `worktree_repo_id` when the session ran in a linked git worktree | SessionStart | +| `turn_end` | `prompt_id` when the host provides one, omitted otherwise | Stop | +| `file_written` | `path`, repository-relative and `/`-separated | PostToolUse, for a write that lands inside a task folder | +| `task_declared` | `path`, repository-relative and `/`-separated | PostToolUse, for a call whose own arguments name a file under a task folder | + +`worktree_id` is git's own name for the linked worktree the session ran in, and `worktree_repo_id` names the repository those worktrees share โ€” read from one `git rev-parse`, never from an agent runner's environment variable, which names that runner's concept rather than the repository's. A plain checkout, the common case, carries **neither key at all**: absent, never `null` and never `""`, because an empty string would gather every plain checkout into one group as though they were the same worktree. + +Neither `file_written` nor `task_declared` ever carries a `task_id`: task identity is a derivation from the path, and derivations belong to whatever reads the log, not to the hook that writes it. `task_declared` differs from `file_written` in what it takes as evidence, not in what it stores: a mention in a tool call's arguments (a read, an edit, a shell command line) rather than a payload naming a write outright - the move that reaches a task on a tool whose payload never hands over a written path at all. A reader turns a run of `task_declared` lines into a bounded interval, closed by whichever of a later declaration or a `turn_end` comes next; see `aidd_docs/product/metrics-contract.md`'s "Attributing records to a task". + +Whether any of these records is ever shared beyond the machine that wrote it is undecided. Nothing here sends anything anywhere. diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/phase-1.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-1.md similarity index 100% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/phase-1.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-1.md diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/phase-2.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-2.md similarity index 100% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/phase-2.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-2.md diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/phase-3.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-3.md similarity index 100% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/phase-3.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-3.md diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/phase-4.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-4.md similarity index 100% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/phase-4.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-4.md diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/phase-5.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-5.md similarity index 100% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/phase-5.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/phase-5.md diff --git a/aidd_docs/plans/2026_06_23-unify-taxonomy/plan.md b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/plan.md similarity index 96% rename from aidd_docs/plans/2026_06_23-unify-taxonomy/plan.md rename to aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/plan.md index 2e8f91cad..2237f8455 100644 --- a/aidd_docs/plans/2026_06_23-unify-taxonomy/plan.md +++ b/aidd_docs/tasks/2026_06/2026_06_23_unify-taxonomy/plan.md @@ -10,7 +10,7 @@ status: implemented | Field | Value | | ---------- | --------------------------------------------------------------------- | | **Goal** | One canonical routing table; all surfaces link/derive; board playbook. | -| **Source** | [`2026_06_23-unify-taxonomy.md`](../../specs/2026_06/2026_06_23-unify-taxonomy.md) (spec, VALID 100/100) | +| **Source** | [`2026_06_23-unify-taxonomy.md`](../../../specs/2026_06/2026_06_23-unify-taxonomy.md) (spec, VALID 100/100) | ## Phases diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-1.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-1.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-1.md diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-2.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-2.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-2.md diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-3.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-3.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-3.md diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-4.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-4.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-4.md diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-5.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/phase-5.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/phase-5.md diff --git a/aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md b/aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/plan.md similarity index 100% rename from aidd_docs/plans/2026_07_01-sdlc-anti-slop/plan.md rename to aidd_docs/tasks/2026_07/2026_07_01_sdlc-anti-slop/plan.md diff --git a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-1.md b/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-1.md deleted file mode 100644 index 2e0618f97..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-1.md +++ /dev/null @@ -1,51 +0,0 @@ ---- -status: done ---- - -# Instruction: Apply and prove skill transformation - -## Architecture projection - -> Tree of the final files. โœ… create ยท โœ๏ธ modify ยท โŒ delete - -```txt -. -โœ๏ธ cli/src/application/use-cases/framework/strategies/marketplace-build-strategy.ts -โœ๏ธ cli/src/application/use-cases/framework/strategies/marketplace-strategy-helpers.ts -โœ๏ธ cli/src/application/use-cases/framework/strategies/tool-contracts.ts -โœ๏ธ cli/src/domain/tools/ai/codex.ts -โœ๏ธ cli/tests/application/use-cases/framework/marketplace-build-strategy.codex.integration.test.ts -``` - -## User Journey - -```mermaid -flowchart TD - A[Framework source skill] --> B[Codex marketplace build] - B --> C[Codex-transformed SKILL.md] - C --> D[Native marketplace artifact] -``` - -## Tasks to do - -### `1)` Reuse the Codex skill converter during marketplace builds - -> Route markdown skill files through the target artifact transform after link rewriting. - -1. Extend the marketplace skill-tree writer to receive and apply the skill artifact transform. -2. Expose or extract the existing Codex frontmatter allowlist as the build transform's source of truth. -3. Configure the Codex marketplace contract to transform skill markdown while preserving non-markdown assets and existing link rewriting. - -### `2)` Lock the regression with an integration test - -> Assert the native Codex marketplace artifact omits `model` and unsupported frontmatter. - -1. Add a fixture-backed test covering a skill with Claude model metadata. -2. Assert supported Codex frontmatter remains and `model` is absent. - -## Test acceptance criteria - -| Task | Acceptance criteria | -| --- | --- | -| 1 | A Codex marketplace build emits every markdown skill using the Codex frontmatter allowlist, while source skills remain unchanged. | -| 2 | The build integration suite fails if a native Codex marketplace `SKILL.md` contains `model`. | diff --git a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-2.md b/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-2.md deleted file mode 100644 index 4633db1bc..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/phase-2.md +++ /dev/null @@ -1,47 +0,0 @@ ---- -status: done ---- - -# Instruction: Verify installed origin marketplace - -## Architecture projection - -> Tree of the final files. โœ… create ยท โœ๏ธ modify ยท โŒ delete - -```txt -. -``` - -## User Journey - -```mermaid -flowchart TD - A[Build current origin] --> B[Install its Codex marketplace] - B --> C[Inspect installed skills] - C --> D[No model frontmatter] -``` - -## Tasks to do - -### `1)` Build and test the current origin - -> Run the targeted regression suite and create a fresh Codex marketplace output from this worktree. - -1. Run the Codex marketplace integration tests. -2. Build a fresh Codex marketplace from the worktree. -3. Scan the generated `SKILL.md` files for a `model` frontmatter key. - -### `2)` Install and identify the local origin artifact - -> Use Codex to install the marketplace generated from this worktree in an isolated Codex home, then inspect installed skills and marketplace metadata. - -1. Install from a local artifact or an origin-addressable ref that names this repository, never the upstream repository. -2. Assert installed skill files contain no `model` frontmatter key. -3. Assert installation metadata identifies this origin artifact, not `ai-driven-dev/framework`. - -## Test acceptance criteria - -| Task | Acceptance criteria | -| --- | --- | -| 1 | The current-worktree Codex marketplace build and targeted integration tests succeed, and generated skills have no `model` key. | -| 2 | An isolated Codex installation sourced from this origin artifact has no `model` key in installed `SKILL.md` files and carries origin-specific marketplace identity. | diff --git a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/plan.md b/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/plan.md deleted file mode 100644 index cab96086e..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/plan.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -objective: "Codex marketplace installs from this origin distribute transformed SKILL.md files with no model frontmatter." -status: reviewed ---- - -# Plan: Transform Codex marketplace skills - -## Overview - -| Field | Value | -| --- | --- | -| **Goal** | Make native Codex marketplace output use the same skill-frontmatter conversion as the AIDD CLI install path. | -| **Source** | [upstream issue #570](https://github.com/ai-driven-dev/framework/issues/570) | - -## Phases - -| # | Phase | File | -| --- | --- | --- | -| 1 | Apply and prove skill transformation | [phase-1.md](./phase-1.md) | -| 2 | Verify installed origin marketplace | [phase-2.md](./phase-2.md) | - -## Resources - -| Source | Verified | -| --- | --- | -| https://github.com/ai-driven-dev/framework/issues/570 | Native Codex marketplace currently bypasses `stripCodexSkillFrontmatter`; installed skills must omit `model`. | - -## Decisions - -| Decision | Why | -| --- | --- | -| Make marketplace skill writes honor each target's existing skill artifact transform. | It reuses the target contract rather than adding a Codex-only branch, keeps Claude source untouched, and makes native marketplace output follow the same conversion policy as the CLI install path. | diff --git a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/review.md b/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/review.md deleted file mode 100644 index 968b35065..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/review.md +++ /dev/null @@ -1,34 +0,0 @@ -# Review: Transform Codex marketplace skills - -- **Verdict**: approve -- **Diff**: `main...20e46f31967a308c795015c975fd89967ba2cdf1 + worktree` -- **Axes run**: code, functional, relevancy -- **Date**: 2026-08-03 -- **Findings**: 0 critical, 0 warning, 0 minor - -## Phases - -### Phase 1 โ€” Apply and prove skill transformation - -- [x] A Codex marketplace build emits every `SKILL.md` using the Codex frontmatter allowlist while source skills remain unchanged; auxiliary Markdown assets remain byte-preserved. โ€” `cli/src/application/use-cases/framework/strategies/marketplace-build-strategy.ts:80-84`, `cli/src/application/use-cases/framework/strategies/marketplace-strategy-helpers.ts:93-102`, `cli/src/application/use-cases/framework/strategies/tool-contracts.ts:337-360`, `cli/tests/application/use-cases/framework/marketplace-build-strategy.codex.integration.test.ts:227-263` -- [x] The build integration suite fails if a native Codex marketplace `SKILL.md` contains `model`. โ€” `cli/tests/application/use-cases/framework/marketplace-build-strategy.codex.integration.test.ts:227-243`, `cli/tests/fixtures/framework-codex/plugins/aidd-codex-fixture/skills/sample/SKILL.md:1-8` - -### Phase 2 โ€” Verify installed origin marketplace - -- [x] The current-worktree Codex marketplace build and targeted integration tests succeed, and generated `SKILL.md` files have no `model` key. โ€” `aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md:3-10`, `aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md:31-32`; `pnpm vitest run tests/application/use-cases/framework/marketplace-build-strategy.codex.integration.test.ts` => `30 passed` -- [x] An isolated Codex installation sourced from the local artifact has no `model` in installed `SKILL.md` and carries origin-specific marketplace identity. โ€” `aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md:17-32` (`sourceType: local`, local artifact path, 13 installed `SKILL.md`, scan clean) - -## Findings - -| Sev | Kind | Phase | Location | Issue | Fix | -| --- | ---- | ----- | -------- | ----- | --- | -| โ€” | โ€” | โ€” | โ€” | None. | โ€” | - -## Verification - -| Metric | Value | -| --- | --- | -| Verified | 100% (4/4) | -| Files checked | `cli/src/application/use-cases/framework/strategies/marketplace-build-strategy.ts`, `cli/src/application/use-cases/framework/strategies/marketplace-strategy-helpers.ts`, `cli/src/application/use-cases/framework/strategies/tool-contracts.ts`, `cli/src/domain/tools/ai/codex.ts`, `cli/tests/application/use-cases/framework/marketplace-build-strategy.codex.integration.test.ts`, `cli/tests/fixtures/framework-codex/plugins/aidd-codex-fixture/skills/sample/SKILL.md`, `cli/tests/fixtures/framework-codex/plugins/aidd-codex-fixture/skills/sample/assets/template.md`, `aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md` | -| Unchecked | none | -| Unplanned | `aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/review.md` is the required review deliverable; none otherwise | diff --git a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md b/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md deleted file mode 100644 index 72e257091..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_03_codex-marketplace-skill-transform/validation.md +++ /dev/null @@ -1,32 +0,0 @@ -# Native Codex installation validation - -Observed on 2026-08-03 from a locally built marketplace artifact: - -```json -{ - "marketplaceName": "aidd-framework", - "installedRoot": "/private/tmp/aidd-codex-marketplace.EOuI9t/marketplace", - "alreadyAdded": false -} -``` - -Installed plugin identity: - -```json -{ - "pluginId": "aidd-context@aidd-framework", - "installedPath": "/tmp/aidd-codex-marketplace.EOuI9t/home/.codex/plugins/cache/aidd-framework/aidd-context/2.5.0", - "source": { - "source": "local", - "path": "/private/tmp/aidd-codex-marketplace.EOuI9t/marketplace/plugins/aidd-context" - }, - "marketplaceSource": { - "sourceType": "local", - "source": "/private/tmp/aidd-codex-marketplace.EOuI9t/marketplace" - } -} -``` - -`sourceType: "local"` and the temporary artifact path prove this is not the upstream -`ai-driven-dev/framework` marketplace. The installed cache contained 13 `SKILL.md` files; -the completed `^model:` scan returned no matches. diff --git a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-1.md b/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-1.md deleted file mode 100644 index 35233ec6b..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-1.md +++ /dev/null @@ -1,65 +0,0 @@ ---- -status: done ---- - -# Instruction: Migrate `01-ticket-info` to the router contract - -## Architecture projection - -> โœ… create ยท โœ๏ธ modify ยท โŒ delete - -```txt -. -โ””โ”€โ”€ plugins/aidd-pm/skills/01-ticket-info/ - โ”œโ”€โ”€ SKILL.md โœ๏ธ mermaid flow, `| Action | Does |` table, canonical lead-in; drop the `| # | Action | Role | Input |` shape and duplicated process rules - โ”œโ”€โ”€ actions/01-ticket-info.md โœ๏ธ cite references/ and assets/ instead of restating rules and output fields - โ”œโ”€โ”€ references/ โœ… create โ€” issue scope mandates a references/ folder on every one of the 3 skills - โ”‚ โ””โ”€โ”€ tool-detection.md โœ… create โ€” where the configured ticketing tool and identifier convention are declared - โ””โ”€โ”€ assets/ - โ””โ”€โ”€ ticket-template.md โœ… create โ€” standardizes the display fields (title, status, assignee, priority, URL, description); user-requested during phase review, not in the original issue scope -``` - -## Tasks to do - -### `1)` Rebuild the router - -> Router keeps only the flow, the action table, and whatever rule truly belongs to no single action or reference (R6, R9, R10). - -1. Add a mermaid `flowchart LR`: one entry node (ticket id or branch-derived id) โ†’ `ticket-info` โ†’ terminal node (displayed ticket). Single action, no loop. -2. Replace the `| # | Action | Role | Input |` table with `| Action | Does |`: bare slug `ticket-info`, lowercase imperative half-line, no trailing period (R8). -3. Replace "Before running an action, read its file in `actions/`, not only the table or assets." with the canonical lead-in: "Run the flow above. Read only the next action file." (matches `07-epic`, `skill-template.md`). -4. Compare each line under `## Transversal rules` against `actions/01-ticket-info.md`'s `## Process` steps 1-3: every rule already stated there is deleted from the router, not restated (R17). -5. Drop the line repeating the frontmatter `Not for` list / intent โ€” `description` already carries it (R3). - -### `2)` Create `references/tool-detection.md`, decide the rest - -> Issue Scope is explicit: "Give each of the three a `references/` folder." Not optional โ€” this is the one reference file this skill gets. - -1. Create `references/tool-detection.md`: a table or list stating where the configured ticketing tool is declared (project memory first, otherwise repo configuration or environment) and the identifier-format convention (branch-derived id, project prefix/separator/casing) โ€” the two facts that are lookup-order-shaped, not process-shaped (R15). -2. Cite it from the action's `## Process` steps 1 and 3 with a relative link (R14, R18) โ€” delete the equivalent prose from the router's `## Transversal rules` once cited, don't keep both (R17). -3. Check whether "read-only: never create, comment, transition, or reassign" states something the frontmatter `description`'s `Not for` clause doesn't already cover. If it's pure duplication, delete it; otherwise it's the one line that stays in `## Transversal rules` (it governs the whole skill, not one process step). - -### `3)` Create `assets/ticket-template.md` - -> User-requested during phase review: standardize the display output. Checked first whether `aidd-orchestrator:01-sdlc` (the known caller, `references/01-frame.md:15-16`) needs a specific shape โ€” it consumes `$resolved_source` as free text, no schema, so this is a display-consistency choice, not an external contract requirement. - -1. Create `assets/ticket-template.md` with the fields the action already outputs: title, status, assignee, priority, URL, description. Follow the existing template idiom (leading HTML comment instructing fill-and-strip, bracketed placeholders โ€” see `spec-template.md`, `epic-template.md`). -2. Cite it from the action's `## Output` and the `## Process` "Display" step (R18) instead of enumerating the fields inline. -3. Do not add a router `## Assets` section โ€” R6 forbids it; the citation lives in the action, same as every other asset in the plugin. - -### `4)` Verify the action still stands alone - -> The action file must state everything needed to run, citing references and assets rather than depending on the router. - -1. Re-read `actions/01-ticket-info.md`: confirm it states, on its own plus its citations, everything needed to run โ€” no missing step because the router used to cover it. -2. Confirm each citation sits in the sentence that uses it (R18), not as a standalone line. - -## Test acceptance criteria - -| Task | Acceptance criteria | -| ---- | ------------------------------------------------------------------------------------------------------ | -| 1 | `SKILL.md` has a mermaid flow, a `\| Action \| Does \|` table, and the exact canonical lead-in sentence. | -| 1 | No line in `## Transversal rules` duplicates a `## Process` step in `actions/01-ticket-info.md`, nor the frontmatter `description`. | -| 2 | `references/tool-detection.md` exists, is cited from `actions/01-ticket-info.md`, and states a fact no action process step restates. | -| 3 | `assets/ticket-template.md` exists, is cited from `actions/01-ticket-info.md`, and `SKILL.md` has no `## Assets` section. | -| 4 | `actions/01-ticket-info.md` read together with its cited reference and asset fully describes how to run the action, with nothing left only in `SKILL.md`. | diff --git a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-2.md b/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-2.md deleted file mode 100644 index 8f4b09faf..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-2.md +++ /dev/null @@ -1,39 +0,0 @@ ---- -status: done ---- - -# Instruction: Migrate `03-prd` to the router contract - -## Architecture projection - -> โœ… create ยท โœ๏ธ modify ยท โŒ delete - -```txt -. -โ””โ”€โ”€ plugins/aidd-pm/skills/03-prd/ - โ”œโ”€โ”€ SKILL.md โœ๏ธ mermaid (3-node chain), `| Action | Does |`, canonical lead-in, 5+1 Transversal rules, no `## Assets` - โ”œโ”€โ”€ actions/01-prd.md โŒ delete โ€” split below - โ”œโ”€โ”€ actions/01-draft.md โœ… create โ€” parse+fill+iterate to approval - โ”œโ”€โ”€ actions/02-finalize.md โœ… create โ€” save+verify - โ”œโ”€โ”€ assets/prd-template.md โ€” unchanged, sole source of truth for the 8 sections - โ””โ”€โ”€ assets/task-template.md โŒ delete โ€” unfilled, collides by name with 10-task's real Task template -``` - -No `references/` folder. Issue Scope says "give each of the three a `references/` folder", but the only candidate content (the save path, one sentence) isn't reference-shaped โ€” no table, no branching, nothing 04-spec's `tbd-marker.md`-style multi-site drift applies to. The literal AC ("no router states a rule an action or reference could hold") is satisfied either way; inlined into `finalize`'s Process step 1 instead. - -## Decisions made during review (deviate from the original phase draft) - -- **Split `01-prd` into `draft` + `finalize`.** The 1-action design (parse+draft+validate+save in one file) was measurably more verbose per-action than every comparable migrated skill. Matches `08-three-amigos`'s 2-action, single-responsibility shape. Net behavior unchanged (same approval gate, same output) โ€” action-count restructuring past issue #564's literal "no behaviour change" scope, so flagged here rather than silently absorbed. -- **Added the 5-line shared boilerplate + 1 skill-specific line to `## Transversal rules`.** 6 of 7 already-migrated skills (`02,05,06,07,09,10`) share these 5 lines verbatim; a missed pattern in the first draft. -- **Dropped `affected relations` and `before -> after`** from `finalize`'s report contract โ€” both are copy-pasted from Task/Defect/Epic, neither applies (PRD has no `relations.md`, and always creates a fresh dated file โ€” no update-in-place exists to diff). -- **No `references/persistence.md`.** See above. - -## Test acceptance criteria - -| # | Acceptance criteria | -| - | -------------------- | -| 1 | `assets/task-template.md` no longer exists; nothing references it. | -| 2 | `SKILL.md` has a mermaid flow, `\| Action \| Does \|` table, canonical lead-in, no `## Assets`, and the 5+1 `## Transversal rules`. | -| 3 | The 8 PRD section names appear in exactly one file: `assets/prd-template.md`. | -| 4 | `draft` never writes to disk; `finalize` only ever receives an already-approved draft. | -| 5 | Live headless run: both actions chain correctly, saved file matches `prd-template.md` exactly (verified). | diff --git a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-3.md b/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-3.md deleted file mode 100644 index 7a2b51004..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/phase-3.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -status: done ---- - -# Instruction: Migrate `04-spec` to the router contract - -## Architecture projection - -> โœ… create ยท โœ๏ธ modify ยท โŒ delete - -```txt -. -โ””โ”€โ”€ plugins/aidd-pm/skills/04-spec/ - โ”œโ”€โ”€ SKILL.md โœ๏ธ mermaid (2 entry nodes: build vs refine), `| Action | Does |`, canonical lead-in, no `## Assets`, own 4-line Transversal rules (not the epic/task/prd boilerplate โ€” spec has no approval gate, doesn't fit) - โ”œโ”€โ”€ actions/01-build.md โœ๏ธ cite tbd-marker.md, cite spec-template.md (pre-existing gap, was never actually linked), drop before->after/affected relations (always creates fresh, no diff) - โ”œโ”€โ”€ actions/02-refine.md โœ๏ธ cite tbd-marker.md, fix 2 drifted TBD spellings, keep before->after (legitimate โ€” rewrites in place), drop affected relations - โ””โ”€โ”€ references/ - โ””โ”€โ”€ tbd-marker.md โœ… create โ€” the one canonical `TBD: ` spelling -``` - -## Decisions - -- **No epic/task/prd Transversal-rules boilerplate.** Checked each of the 5 shared lines against what `build`/`refine` actually do: no approval gate before write, no lifecycle, uses TBD-marking instead of interactive questioning. None of it fits โ€” inventing it would add behavior that doesn't exist. Kept spec's own rules instead (matches `08-three-amigos`'s precedent of not sharing the boilerplate either). -- **Dispatch-by-input moved into the mermaid** as two entry nodes (request/PRD โ†’ `build`, spec+findings โ†’ `refine`), replacing the router prose that stated the same branch (R7, R17). -- **`before -> after` kept in `refine`, dropped from `build`.** `refine` genuinely rewrites an existing file in place (real diff). `build` always creates a fresh dated file โ€” no prior state exists to diff. -- **Router never cites a reference.** No other router does (checked all 7) โ€” R18 only names Process/Output/Test as valid citation sites. Router states policy in plain words ("Never invent; mark every gap instead of guessing"); the actions cite `tbd-marker.md` where they apply it. -- **`tbd-marker.md` trimmed to the literal string only** (`TBD: `, no policy prose) โ€” the policy already lives once in the router; anything more would duplicate it. -- **`build.md`'s Source step split into 2 sub-bullets** (PRD path vs free-form request) instead of one dense sentence; "never explore the codebase" promoted out of it into the router's Transversal rules (applies to both actions, not just Source). -- **`refine.md`'s Output cut to one line**, TBD citation removed from Output (stays in Process step 4 only โ€” was duplicated), added an explicit `Verify` step so `before -> after` reporting has a Process home instead of living only in prose. -- **Two pre-existing gaps fixed while auditing citations, not part of the original scope:** `build.md` never linked `spec-template.md` (called it "the template" in prose only); the router's "reuse the folder when it exists" line was deleted without moving its actual path pattern into `build.md` (first draft), caught by a live headless run hitting the gap itself. Second catch: the fixed version still lost the word "resolve" (search-then-reuse-or-create), reducing it to a same-day-only check โ€” restored the two-outcome framing. -- **Two follow-up issues filed, not fixed here** (behavior changes, out of #564's "no behaviour change" scope): [#625](https://github.com/ai-driven-dev/framework/issues/625) โ€” SDLC's Frame stage never checks `spec-validator.yml` before handing a spec to Deliver. [#626](https://github.com/ai-driven-dev/framework/issues/626) โ€” `spec-template.md` has no `## Open Questions` section, so TBD placement is non-deterministic (confirmed: same feature, two runs, two different placements). - -## Plugin-wide verification (AC#1, AC#5 โ€” span all 10 skills, checked here as the last phase) - -- All 10 `SKILL.md` files: `# Title` โ†’ `## Actions` (mermaid + `| Action | Does |` + canonical lead-in) โ†’ optional `## Transversal rules`. Section presence varies only where content is legitimately absent (`01-ticket-info` has none โ€” real precedent elsewhere in the framework, e.g. `aidd-dev:01-plan`), never order. -- Every asset in `plugins/aidd-pm/skills/*/assets/*` is cited from a named action. `spec-validator.yml` is read against, not filled โ€” the "consumed by a named action" reading (see `plan.md` Decisions) covers it. -- **Issue count correction:** issue #564 says "eight follow the contract, three don't" (= 11) and AC#1 says "the eleven routers." The plugin holds 10 skills, not 11 โ€” 7 already matched the contract, 3 migrated here. Reporting this rather than silently treating "eleven" as satisfied. diff --git a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/plan.md b/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/plan.md deleted file mode 100644 index d48df3842..000000000 --- a/aidd_docs/tasks/2026_08/2026_08_10_migrate-pm-router-contract/plan.md +++ /dev/null @@ -1,53 +0,0 @@ ---- -objective: "01-ticket-info, 03-prd, and 04-spec routers match the aidd-pm contract (07-epic shape): references/ hold every rule an action doesn't, TBD: has one spelling in one file, and the PRD sections live only in prd-template.md." -status: implemented ---- - -# Plan: Migrate ticket-info, PRD, spec to the router contract - -## Overview - -| Field | Value | -| ---------- | ---------------------------------------------------------------------- | -| **Goal** | Bring the 3 remaining `aidd-pm` skills in line with the other 7: mermaid flow, `\| Action \| Does \|` table, canonical lead-in, no `## Assets`, one rule one home. | -| **Source** | GitHub issue #564 (ai-driven-dev/framework) | - -Each phase migrates exactly one skill. Stop after each phase for interactive review before starting the next โ€” explicit user instruction, not the default. - -## Phases - -| # | Phase | File | -| --- | ------------------------------- | ------------------------------ | -| 1 | Migrate `01-ticket-info` | [`phase-1.md`](./phase-1.md) | -| 2 | Migrate `03-prd` | [`phase-2.md`](./phase-2.md) | -| 3 | Migrate `04-spec` | [`phase-3.md`](./phase-3.md) | - -## Resources - -| Source | Verified | -| --------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | -| GitHub issue #564 | Scope, acceptance criteria, and the two corrected claims (argument-hint, `## Test` shape). | -| `plugins/aidd-pm/skills/07-epic/*` | Reference migration: router mermaid + `\| Action \| Does \|` shape, references/ split, canonical lead-in wording. | -| `plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md` | The contract, R1-R19, per artifact (skill, router, action, reference, asset). | -| `plugins/aidd-context/skills/04-skill-generate/assets/{skill,action}-template.md` | Exact canonical lead-in text and section order/frontmatter shape. | -| Direct read of all 3 target `SKILL.md` + `actions/*.md` | Confirmed every duplication the issue names, and the exact 3 spellings of `TBD:` in `04-spec`. | - -## Decisions - -| Decision | Why | -| -------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | -| `TBD:` marker's one home is `04-spec/references/tbd-marker.md`, not a shared cross-skill reference | All 3 `TBD:` sites (`SKILL.md`, `01-build.md`, `02-refine.md`) live inside `04-spec`; this codebase doesn't share reference files across skills. | -| `references/` is per-skill judgment, not mandatory on all 3 | Issue Scope says "give each of the three a `references/` folder", but the literal AC only requires a rule to live in *an action or reference* โ€” not that the folder exist. `01-ticket-info` gets one (`tool-detection.md`, genuinely lookup-table-shaped). `03-prd` doesn't โ€” its only candidate (a one-sentence save path) isn't reference-shaped, inlined into `finalize` instead. | -| `01-ticket-info` ends up with no `## Transversal rules`; `03-prd` gets the 6-line pattern | `01-ticket-info`: every prior line moved to a reference or was a pure duplicate of `description` โ€” real precedent, 8 framework skills ship `## Actions` with no `## Transversal rules`. `03-prd`: 6 of 7 already-migrated skills share 5 verbatim boilerplate lines + 1 skill-specific line โ€” missed in the first draft, restored during review. | -| `03-prd` splits into `draft` + `finalize` (2 actions), matching `08-three-amigos`'s single-responsibility shape | User-requested during phase 2 review: the 1-action design (parse+draft+validate+save) was measurably more verbose per-action than every comparable migrated skill. Net behavior unchanged (same approval gate, same output) but this is action-count restructuring โ€” past issue #564's literal "no behaviour change" scope. Extension flagged, not silently absorbed. | -| `03-prd`'s report contract drops `affected relations` and `before -> after` | Both copy-pasted from Task/Defect/Epic; neither applies โ€” PRD has no `relations.md`, and always creates a fresh dated file (no update-in-place to diff). | -| `04-spec`'s build-vs-refine dispatch moves into the mermaid as two entry nodes, not prose | R7: a branch stated in prose is a branch missing from the flow; R17: one fact, one home. | -| One phase per skill, review gate between phases | User-requested; keeps each migration independently verifiable against the shape criterion. | -| Verify AC#1 (section order, all 10 skills) and AC#5 (every plugin asset filled) in phase 3, not a separate phase | Both criteria span skills beyond any single phase's scope; checking them after the last migration is cheaper than a 4th review-gated phase. | -| AC#5's "filled by an action" reads as "consumed by a named action" โ€” a template is filled, a validator/checklist is read against | `spec-validator.yml` is read, never written; the issue's actual named defect is `task-template.md` being cited by nothing at all. Resolving the reading now avoids relitigating it mid-phase-3. | -| `04-spec` keeps its own 4-line `## Transversal rules`, not the epic/task/prd 5+1 boilerplate | Checked each shared line against what `build`/`refine` actually do: no approval gate before write, no lifecycle, TBD-marking instead of interactive questioning. None fit โ€” matches `08-three-amigos`'s precedent of not sharing the boilerplate either. | -| `04-spec`'s report contract keeps `before -> after` in `refine`, drops it from `build`; drops `affected relations` from both | `refine` rewrites an existing file in place (real diff); `build` always creates fresh (nothing to diff). No `relations.md` exists for spec, same as PRD. | - -## Correction - -The issue's own arithmetic doesn't match the repo: "Eight `aidd-pm` skills follow the router contract. Three never migrated" (= 11) and AC#1 "the eleven `aidd-pm` routers" both assume 11 skills. `plugins/aidd-pm/skills/` holds 10 (`01`-`10`). 7 already match the contract (`02,05,06,07,08,09,10` โ€” confirmed identical heading sequence, mermaid, and canonical lead-in against `07-epic`), plus the 3 this plan migrates = 10, not 11. Same category of error as the two claims the issue itself already corrected under "What already landed". Flagged for the user in phase 3, not silently resolved. diff --git a/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/measurements.md b/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/measurements.md new file mode 100644 index 000000000..8fefc727d --- /dev/null +++ b/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/measurements.md @@ -0,0 +1,754 @@ +# Measurements + +## Phase 1 โ€” the confrontation with data the code has never seen + +The committed fixture is synthetic, so it agrees with the code that reads it. This is the +step that does not: the deleted script and the CLI, run over the machine's own sink, whose +records nobody authored for this test. + +Run 2026-08-26, on a sink holding 34 records across five tools, three days, four steps and +both record kinds. The script was restored from git into a temporary directory for the +comparison, and removed after it. + +``` +report --json identical +report --axis total identical +report --axis day identical +report --axis model identical +report --axis project identical +report --axis step identical +report --axis tool identical +``` + +Every rendering the two share is byte-identical on real data. Nothing was recorded to fix. + +## What the fixture cannot show, and why + +Two fields are absent from the committed envelope by construction, and are declared +conditional rather than asserted: + +- `cost_micro_usd` โ€” present only once a record states an amount. No tool read locally + writes one, which is why every figure reads `amount unknown`. +- `task`, `task_attribution`, `filters`, `empty_selection`, `active_time_s` โ€” present only + under a selection. + +Their presence and shape are pinned by the CLI's own envelope tests. What +`scripts/__tests__/aidd-telemetry-cost-skill.test.js` pins is narrower and still worth +having: the skill names no envelope field that exists nowhere at all. + +## A coverage gap this phase opened, and where it is closed + +`--axis` did not exist on the CLI. The skill's markdown named it, the CLI refused it, and it +was caught before anything was deleted โ€” by the check that every command the skill names is +one the CLI accepts. It is now `cost-report-artefact.ts`, byte-identical to the script's own +rendering on all six axes, on both the synthetic fixture and the real sink above. + +Two suites moved out of `scripts/__tests__/aidd-telemetry-identity.test.js` and into phase 2: +*"what a default install actually stores, proven from the stored bytes"* and *"a choice made +today does not reach backwards"*. Both drove `read` through the deleted reporter. Until phase +2 restores them against `aidd telemetry read`, the behaviour is pinned one level down, in +`read-local-cost-use-case.unit.test.ts`. That is real coverage, and it is not the same as +reading the bytes. + +## Phase 2 โ€” `aidd telemetry identity` confronted with the script it replaces + +Both suites named above are now restored as e2e tests against `aidd telemetry read` itself +(`cli/tests/e2e/telemetry-identity.e2e.test.ts`), reading the stored JSONL lines from disk +rather than a stubbed sink โ€” the same claim the deleted reporter's tests made, made again +against the CLI. + +`telemetry-identity.cjs` and `aidd telemetry identity` were then run side by side over the +same starting profile, once per verb, and the resulting `identity.json` compared byte for +byte (`identityFileIn` gives both sides the same path convention: `.config/aidd/identity.json` +on POSIX, `AppData/Roaming/aidd/identity.json` on Windows). Run 2026-08-26. + +``` +name (same pre-written identity on both sides) byte-identical +on (against an existing identity, both untouched) byte-identical +on (from empty โ€” each side mints its own uuid) identical shape once each side's own v4 id is normalized to a placeholder; + both ids independently verified against the v4 pattern +off (both remove the file) file absent on both sides +status (no write, whatever the starting state) file unchanged on both sides; both print the same person_id +``` + +`on` from an empty profile cannot be raw-byte-identical by construction โ€” both sides call +their own random UUID generator, so the two files necessarily hold two different +identifiers. The comparison that carries the real risk is `name`: it is the one write whose +shape (key order `person_id` before `display_name`, two-space indent, one trailing newline) +could silently drift into camelCase or a different field order without any test noticing +before this one. It came back byte-identical on the first run โ€” nothing was recorded to fix. + +Mode bits were checked once, on the freshly-minted file from the empty-profile `on` run, +skipped on `win32` where `mode` is a documented no-op: `identity.json` is `0600`, its +directory `0700`. + +## What this phase deliberately does not match + +The script's `readIdentity()` folds a read failure (a directory sitting where the file +should be, a permission error, damaged JSON) into the same `null` as a plain missing file โ€” +"nobody chose" reads the same whichever of the three caused it. `aidd telemetry identity` +does not: `readStrict()` throws on anything past `ENOENT`, so `status`, `on` and `name` all +surface a read failure as an error rather than as "no identity is set" โ€” the Test Scope's own +"the identity file is unreadable" edge case, and the fourth line of task 4's acceptance +criteria. `read()`, the method `aidd telemetry read`'s local-cost sweep depends on, keeps the +script's original swallow-everything behaviour unchanged: one damaged identity file must +never cost every tool's figures for an entire sweep. + +`off` is the one exception to "surface a read failure as an error": it catches its own +`readStrict()`'s throw and discards the damaged file anyway, stating that it did โ€” a real +probe (`{ not json` written over `identity.json`, then `status`, then `off`) confirmed +`status`/`on` correctly error and leave the file in place, and that `off` now removes it +and says why, where an earlier version of this phase left a damaged file with no way out +of the CLI. `status`, `on` and `name` are unchanged: erroring there is still correct, and +still the contract's own edge case. + +The script's `name` also accepts a whitespace-only value: `!value` is `false` for `" "`, so +it writes `display_name: " "`, keeps it, and echoes it straight back โ€” a probe against the +real script (`telemetry-identity.cjs name " "` then `status`, read with `cat`, not a filter +that can eat the thing under test) prints `display name " "` verbatim. There is no drop +anywhere in the script's own path; an earlier draft of this note claimed `readIdentity()` +silently discards it on the next read, which is false โ€” that check only fires on a literal +empty string (`display_name !== ""`), never on whitespace. + +The CLI refuses a whitespace-only value anyway (`EmptyDisplayNameError`), on an independent +ground the script does not share: this layer's own rule that an identity is never a default, +and a name nobody chose to type is not a name. This is a deliberate parity deviation, not a +behaviour the script already has and the CLI merely preserves โ€” recorded here plainly so +phase 3 does not read it as behaviour-preserving. Any non-blank value, including one with +leading or trailing whitespace, is stored exactly as given, untrimmed โ€” that is what keeps +the `name` parity test above byte-identical to the script for every value that is not this +one refused case. + +## A deviation from the architecture projection, and why + +`phase-2.md`'s tree lists no new port file, and this phase added one: +`domain/ports/person-identity-store.ts`. `0-layer-responsibilities.md` requires an adapter to +implement exactly one port, and the four new verbs need write methods the existing +`PersonIdentityReader.read()` never had โ€” `read()` also has a documented "never throws" +promise that `read-local-cost-use-case.ts` depends on, which the identity verbs' `readStrict()` +deliberately breaks (see above), so the two could not live on one interface without either +weakening that promise or overloading one method name with two contracts. `PersonIdentityStore +extends PersonIdentityReader`, the same shape already used by `CliAuthProvider extends +TokenResolver` (`domain/ports/oauth-provider.ts`), so `PersonIdentityAdapter` still implements +exactly one port โ€” `PersonIdentityStore` โ€” while `ReadLocalCostUseCase` keeps depending on the +narrower `PersonIdentityReader` it always has. + +## Phase 3 โ€” `00-init` calls the CLI, confronted with a live absent-CLI run + +Tasks 0 (separating the switch from the endpoint) landed in the working tree ahead of this +phase and is not redone here; this phase is tasks 1โ€“4 only โ€” rewriting `00-init`'s own +markdown, confirming what the switch script did beyond flipping a flag still happens, +deleting the scripts and their suite, and proving every command the skill now names is one +the CLI accepts. + +### The confrontation: `aidd` present, then removed from `PATH`, for real + +`01-check.md` now runs `aidd --version` and reuses `01-cost/actions/01-locate.md`'s own +absent-CLI paragraph verbatim (pinned by a containment test in +`telemetry-init-skill-commands.e2e.test.ts`, not just eyeballed). Run for real rather than +only through a test double: + +``` +$ aidd --version +aidd/5.1.3 node/25.8.0 darwin-arm64 # exit 0 โ€” the check continues to enable/verify + +$ env -i PATH="/usr/bin:/bin" aidd --version +env: aidd: No such file or directory # exit 127 โ€” "a command that is not found" +``` + +The documented rule ("No output, or a command that is not found, means this machine cannot +answer") matches both outcomes on a real shell, not a mocked one โ€” the same two-line proof +phase 1 already ran for `01-cost`, now re-run for `00-init` since the wording, not just the +behaviour, has to agree with it. + +### Task 1 โ€” every `.cjs` path is gone from `00-init`'s own markdown + +`SKILL.md`, `01-check.md`, `02-enable.md`, `04-identify.md` and `05-forget.md` no longer name +a script. `01-check.md`'s absent-CLI rule is byte-identical to `01-cost/actions/01-locate.md`'s +from "No output, or a command that is not found" through "cost nothing." โ€” verified once by +`diff` while writing it, and pinned going forward by +`telemetry-init-skill-commands.e2e.test.ts`'s own containment test, so a future edit to +either file that drifts the wording fails the suite rather than waiting for a reviewer to +notice. + +### Task 2 โ€” what the switch script did beyond flipping a flag, confirmed present and completed + +`aidd telemetry on` already git-ignored `aidd_docs/runs/` and warned on an already-tracked +journal before this phase started โ€” task 0's `protectRunsDir` โ€” so nothing needed adding +there; `cli/tests/e2e/telemetry-on-runs-privacy.e2e.test.ts` (also landed with task 0) already +pinned five of the six claims `aidd-telemetry-switch-gitignore.test.js` pinned against the +script. The one claim neither owned โ€” `git add -A` actually succeeds against what `on` +writes, and the journal stays out of the index โ€” is now the sixth test in that same file. + +What each half is proven by, since no single file owns all of criterion 2: + +| Claim | Where | +| --- | --- | +| `.gitignore` gets exactly `aidd_docs/runs/`, deduped, and a tracked journal is named once | `telemetry-on-runs-privacy.e2e.test.ts` | +| `git add -A` succeeds and stages nothing under `aidd_docs/runs/` | `telemetry-on-runs-privacy.e2e.test.ts` (new) | +| The run directory and journal file are `0700`/`0600` | `scripts/__tests__/aidd-telemetry-journal.test.js:959-962` โ€” the hook's own write path, which `aidd telemetry on` never touches (it writes `.gitignore` and the switch file only) | + +### Task 3 โ€” the scripts, their suite, and what deleting them broke elsewhere + +`plugins/aidd-telemetry/skills/00-init/scripts/` (2 files, `telemetry-switch.cjs` and +`telemetry-identity.cjs`, plus `lib/journal-privacy.cjs` and `lib/identity.cjs` โ€” the phase's +own "4 files, 278 lines") is deleted; its `package.json` marker was already gone, removed in +`ec15a80f` when the plugin moved to a per-file module-system declaration, well before this +phase. + +`scripts/__tests__/aidd-telemetry-identity.test.js` (14 tests) is deleted, as named. One +suite the phase's own architecture projection did not name is deleted alongside it: +`scripts/__tests__/aidd-telemetry-switch-gitignore.test.js` (6 tests). Its subject, +`telemetry-switch.cjs`, is exactly the file task 3.1 removes, and every one of its six +claims is already re-proven against the CLI in `telemetry-on-runs-privacy.e2e.test.ts` (see +the table above) โ€” keeping it would mean spawning a file that no longer exists. Declared here +rather than left to surface as a failure: `node --test scripts/__tests__/*.test.js` drops by +20 from this pair alone (14 + 6), not the 14 the phase's own acceptance table anticipated. + +A further 10 tests drop from `plugin-install-shape.test.js` without any file being deleted: +it discovers every `*.cjs` script under `skills/*/scripts/` and generates one +"starts and prints its own output" test per script per install shape (5 shapes). Two scripts +disappearing means 2 ร— 5 = 10 fewer generated tests; `KNOWN_INVOCATIONS` no longer names +either. Total drop across the plugin suite: 20 + 10 = 30 (365 โ†’ 335), all accounted for. + +Two more suites named a `00-init` script and needed updating, not deleting, once it was +scripted no longer: + +- `aidd-telemetry-cost-skill.test.js` asserted `01-check.md` searches for + `telemetry-switch.cjs` on a tool with no plugin-root variable, and โ€” in the same file โ€” + that `00-init` must **not** depend on `aidd telemetry` (line-for-line the opposite of what + this phase does). Both assertions are inverted to match: the `searched` table drops the + 00-init row (it ships no script to find any more, same as 01-cost already read), and the + "owns turning measurement on" test now requires `aidd telemetry on` and forbids any + `.cjs` mention instead. +- `plugin-install-shape.test.js`'s `KNOWN_INVOCATIONS` drops `telemetry-switch.cjs` and + `telemetry-identity.cjs` (see above). + +Deleting the script also broke three files this phase's own architecture projection never +named, because they drove the plugin's write path standalone through the same script the +CLI now owns the switch through: + +- `cli/tests/domain/models/plugin-asset-translation.unit.test.ts` asserted + `skills/00-init/scripts/telemetry-switch.cjs` survives a tool's rewrite byte for byte โ€” + one entry removed from its `ARTEFACTS` list; the same claim is still proven for every + hook script, which is the one this test exists to guard. +- `cli/tests/e2e/telemetry-plugin-standalone.e2e.test.ts` spawned the script directly to + prove the plugin works with `aidd` off `PATH`. Its three switch-behaviour tests + ("turns measurement on", "keeps whatever else the config held", "turns it back off") are + removed โ€” `cli/tests/e2e/telemetry.e2e.test.ts` and `telemetry-on-runs-privacy.e2e.test.ts` + already prove the same claims against the CLI, which now owns the switch and is what a + reader would look for first. What survives, unweakened: the journaling test, whose entire + point is that recording needs no `aidd` anywhere โ€” it now seeds `.aidd/config.json` + directly instead of calling the script, so the one thing left running with no CLI on + `PATH` is exactly the write path this file exists to prove standalone. The trailing + "keeps the switch short enough to read" check on the script's own line count is removed; + there is no script left to be short. +- `cli/tests/e2e/telemetry-lifecycle.e2e.test.ts` โ€” the full nothing-to-off-and-back + sequence โ€” called the script for every `switchTo("on"|"off")` inside an environment that + deliberately strips `aidd` from `PATH`. `switchTo` now calls the built CLI by its own + path instead (`run(CLI_PATH, ["telemetry", state])`), the same mechanism `measure` already + used for `report`/`read` โ€” so `PATH` being stripped of `aidd` no longer changes what this + file proves about the switch, only about the hooks, which was always the point of + stripping it. All three of this file's tests still pass unchanged otherwise. + +### Task 4 โ€” every named command, proven against the CLI, in a safe order + +`00-init`'s own commands are stateful โ€” `identity name` and `identity off` only make sense +once `identity on` has run โ€” unlike `01-cost`'s, which are independent reads. Running the +extracted `Set` in whatever order the markdown walk happens to visit files risks `identity +name` executing before `identity on` ever did. `telemetry-init-skill-commands.e2e.test.ts` +sorts every `on` command first and every `off` command last before running any of them, so +the six named commands (`on`, `off`, `identity status`, `identity on`, `identity name +""`, `identity off`) always run in an order where every precondition is already met, +regardless of file walk order. + +### The parity suite `telemetry-identity.cjs`'s deletion leaves behind + +Phase 2 pinned the CLI against `telemetry-identity.cjs` with six tests +("confronted with the script phase 3 deletes"). Deleting the script this phase kills that +comparison; each of the six is accounted for rather than dropped silently: + +| Former parity test | What happens to it | +| --- | --- | +| `name`: byte-identical from the same starting identity | **Kept**, as a fixture pin โ€” the exact bytes the script wrote, captured 2026-08-26 before deletion, asserted directly rather than compared live | +| `on`, from empty: same on-disk shape and modes | **Kept**, as a fixture pin โ€” the normalized shape (`{ "person_id": "" }`) plus the `0600`/`0700` modes, both captured the same way | +| `on`, against an existing identity: file untouched | **Left with the script.** The CLI's own claim โ€” a second `on` reports the same identifier and does not rewrite the file โ€” is already proven by the journey block's "a second `on` reports the same identifier, never a new one" | +| `off`: both remove the file | **Left with the script.** The journey block's own walk (`status -> on -> name -> status -> off`) already asserts the file is gone after `off` | +| `status`: no write, against an existing identity | **Left with the script.** The journey walk reads `status` against a minted identity and asserts the identifier appears; nothing rewrites the file on a read, and no test anywhere claims otherwise | +| `status`: no write, against an empty profile | **Left with the script.** The journey walk's first step is exactly this: `status` against nothing, asserting `off` | + +The two kept as fixture pins are the one claim nothing else in the suite owns: the literal +on-disk byte format a person's `identity.json` would have taken under the deleted script. +The four left behind were never distinct claims about the *CLI* โ€” they were the same claim +the journey block already made, run a second time against a script that is now gone. Keeping +all six as live fixtures would have been the duplication `plan.md`'s own Decision names +("one equivalence pin, not a suite watching two implementations agree with themselves"). + +### Net test counts, tasks 1โ€“4 + +| Suite | Before | After | Why | +| --- | --- | --- | --- | +| `scripts/__tests__/*.test.js` | 365 | 335 | โˆ’14 (identity suite deleted) โˆ’6 (switch-gitignore suite deleted) โˆ’10 (install-shape's dynamic script discovery, 2 scripts ร— 5 shapes) | +| `cli` unit | 2069 | 2068 | โˆ’1 (`plugin-asset-translation.unit.test.ts`'s `ARTEFACTS` entry for the deleted script) | +| `cli` integration | 608 | 608 | unchanged | +| `cli` e2e | 200 | 196 | +3 (`telemetry-init-skill-commands.e2e.test.ts`, new) +1 (`telemetry-on-runs-privacy.e2e.test.ts`'s `git add -A` test, new) โˆ’4 (`telemetry-identity.e2e.test.ts`'s six parity tests collapsed to two fixture pins) โˆ’4 (`telemetry-plugin-standalone.e2e.test.ts`'s three switch tests and the script-line-count test removed) | + +## Phase 4 โ€” `aidd telemetry check`, the local claims + +### The port's shape: reuse over re-implementation + +`diagnose.cjs`'s six claims split cleanly into two routes. This phase settles the local +route (`hook fired` -> `session journalled` -> `tool files readable` -> `records join`) and +states the export route's two claims (`export configured`, `identifier joinable`) as +`unjudged` โ€” a fourth verdict, distinct from `unknown`: an `unknown` was checked and came +back inconclusive, an `unjudged` is a fact this build does not attempt to check at all. + +Most of `readers.cjs` (540 lines) and `attribution.cjs` needed no port at all. `aidd +telemetry read` already reads every covered tool's own files through the exact same +`SessionCostReader` map, and `domain/models/step-attribution.ts`'s `buildStepIntervals`/ +`attributeMoment` already compute the same join `attribution.cjs` does โ€” both built in +phase 1 and unchanged here. `domain/ports/run-journal-reader.ts`'s `list()` already returns +what `journal.cjs`'s `listJournals` returns. `DiagnoseTelemetryUseCase` depends on all three +directly rather than wrapping them in a second "evidence" adapter, which is why +`telemetry-evidence-adapter.ts` ended up covering only what neither of those already +promised: the switch, the unrecognised-payload marker, and Codex's own hook-trust state. +`isRepository` landed on `VersionControl`/`GitAdapter` instead, beside `listTrackedFiles` +phase 3 already added there, rather than a second git-shelling implementation. + +### A deviation from task 2's own list, and why + +Task 2 names four things for the evidence adapter to port: "switch, repository, journal and +marker." Codex's hook-trust state is not in that list, and it is ported here anyway โ€” +`hook fired`'s own FAIL can mean "never observed firing" or "Codex has not trusted this +plugin's hook," two claims task 1 itself requires to stay distinguishable ("no two distinct +reasons share one verdict"). Reading `~/.codex/config.toml` was the only way to keep that +promise for the one claim it is exercised by all through, so it is included in +`telemetry-evidence-adapter.ts` alongside the four the task names โ€” read as an +under-specification in the task list, not a deliberate exclusion, and confirmed against +`telemetry-check.test.js`'s own "naming whether the hook fired" suite, which exercises it +as core `hook-fired` coverage, not export-route coverage. + +### Confronted with the script + +Three parity tests in `cli/tests/e2e/telemetry-check.e2e.test.ts` run `telemetry-check.cjs` +and `aidd telemetry check` over the same starting state and diff their first four printed +lines verbatim: a healthy fixture (switch on, a journalled Claude Code session, the real +transcript fixture beside it), the hook never having fired at all, and the two gates +(switch off; outside a git repository). All three came back byte-identical on the first +run โ€” nothing was recorded to fix. `render.cjs`'s own column widths (`LABEL_WIDTH = 22`, +verdict padded to 4) are reproduced exactly in `telemetry-check-display.ts` for this reason: +the parity claim is a printed line, not just a verdict. + +### Net test counts + +``` +cli unit 2100 passed (was 2068; +25 telemetry-claim.unit.test.ts, +7 diagnose-telemetry-use-case.unit.test.ts) +cli integration 608 passed (unchanged) +cli e2e 210 passed (was 201; +9 telemetry-check.e2e.test.ts) +plugin suite 337 passed (unchanged โ€” no plugin file touched this phase) +tsc, biome, check-cli-layering, check-markdown-links clean +``` + +## Phase 5 โ€” `aidd telemetry check`, the export, the trust, and the join + +### The port's shape + +`export-config.cjs`, `export-sink.cjs` and `hook-trust.cjs` each became one adapter behind +one new port (`export-config-reader.ts`, `export-sink-reader.ts`, `hook-trust-reader.ts`). +`telemetry-evidence-adapter.ts` lost `readCodexHookTrust` to its own port rather than +growing a fourth responsibility โ€” `hook-fired`'s two FAIL reasons (never observed, untrusted +hook) are read from two different files (the run journal, Codex's `config.toml`) and belong +behind two different ports on that basis alone. `diagnoseLocalTelemetryClaims` became +`diagnoseTelemetryClaims`: the `unjudged` verdict phase 4 introduced for the two export +claims is gone entirely now that both are judged, leaving the three-verdict set +(`ok`/`fail`/`unknown`) task 1 of phase 4 originally specified. + +### Confronted with a machine's real configuration (task 4) + +Run against this development machine's own, never-authored-for-this-test configuration: +a real `~/.codex/config.toml` carrying a genuine `[hooks.state...]` table with a real +`trusted_hash` entry for this plugin, and a real `~/.claude/settings.json` (~22KB, this +machine's actual accumulated settings). Anchored once as a Codex session +(`CODEX_THREAD_ID` set) and once as a Claude Code session (`CLAUDE_CODE_SESSION_ID` set), +`telemetry-check.cjs` and `aidd telemetry check` were run side by side over both anchors. + +**The side-by-side run against these real files caught one divergence.** The ported +`export-config-reader-adapter.ts` rendered its detail strings with an em dash (`โ€”`) in +three places โ€” `codexMissingDetail`'s two messages and `readClaudeExportConfig`'s "not +together in one file" message โ€” where `export-config.cjs` writes a plain hyphen (` - `). +A synthetic fixture would have agreed with either punctuation, since both sides would have +been written to match the same test; only comparing against this machine's real files, +whose exact bytes no test fixture chose, made the mismatch visible. **Settled as a defect +in the CLI port**, fixed to match the script's punctuation exactly (see +`export-config-reader-adapter.ts`'s `codexMissingDetail` and `readClaudeExportConfig`). +The re-run after that fix came back **byte-identical on all six claim lines, on both +anchors** โ€” nothing left unsettled. This confrontation is now unreproducible by design โ€” +`telemetry-check.cjs` is deleted as of this same phase's task 5 โ€” which is why it is +recorded here rather than left as a test CI could ever rerun again. + +### Task 5's collateral: three tests that named the deleted script + +Deleting `02-check/scripts/telemetry-check.cjs` (13 files, 1,664 lines) broke three tests +that were not on the plan's own file list, none of which the plan's acceptance criteria +mention, all fixed as part of leaving no debt from the deletion itself: + +- **`cli/tests/domain/models/plugin-asset-translation.unit.test.ts`**'s "installing the + plugin carries its measurement script" describe block read `telemetry-check.cjs` off + disk to prove the translator carries a skill-nested script byte for byte. Since no skill + in this plugin ships a script any more (00-init, 01-cost, and now 02-check all moved to + `aidd`), it now borrows real bytes from `hooks/journal.cjs` โ€” a file this plan does not + delete โ€” placed at a synthetic skill-nested path (`skills/02-check/scripts/example.cjs`). + The path is the fixture; the content is still real, which is what tells "carried + verbatim" apart from "compared a string to itself." +- **`scripts/__tests__/plugin-install-shape.test.js`**'s `KNOWN_INVOCATIONS` map held only + `telemetry-check.cjs`; emptying it would have left "discovers the scripts known today" + iterating zero keys and passing vacuously โ€” silently indistinguishable from the walk + never running at all. Inverted to a direct assertion, once per install shape: `ships no + skill scripts, now that every skill calls the CLI instead`, which fails loudly + (confirmed red before the deletion, green after) rather than passing by omission. +- **`scripts/__tests__/aidd-telemetry-cost-skill.test.js`** carried two tests keyed to the + soon-to-be-deleted script: `"each skill finds its own script..."` searched + `02-check/actions/01-locate.md` for a `find` command across plugin directories โ€” gone now + that locate just runs `aidd --version` โ€” replaced with a positive assertion that no + skill's actions search for a script that way any more. `"the check skill calls the + plugin's own binary, never the CLI"` asserted the opposite of what phase 5 does on + purpose; inverted to `"the check skill calls the CLI, never a script of its own"`. + +### A gap the plan's own acceptance criteria did not name + +`telemetry-check.e2e.test.ts` pins `aidd telemetry check`'s *behaviour* against fixed +fixtures; it never reads `02-check`'s own markdown, so it could not have caught the +markdown naming a command the CLI does not accept โ€” exactly the class of defect +`telemetry-where-things-live.test.js`'s own header describes (a README naming a deleted +script for two phases, undetected by both existing guards). `00-init` and `01-cost` are +each held to this by their own `telemetry-*-skill-commands.e2e.test.ts`; `02-check` had no +equivalent. Added `cli/tests/e2e/telemetry-check-skill-commands.e2e.test.ts`, mirroring the +same shape: every `` `aidd telemetry โ€ฆ` `` command `02-check`'s markdown names is extracted +and actually run through the CLI, and the skill is pinned to name no `.cjs` path any more. + +`commandsNamedBySkill`'s regex only matches `` `aidd telemetry โ€ฆ` `` โ€” `aidd --version`, +which `01-locate.md` now depends on, is not covered by this guard. Same limitation as the +two sibling files it mirrors, so this is consistent with existing coverage, not a new gap; +worth stating so the guard is not read as broader than it is. + +### Net test counts + +``` +cli unit 2107 passed (was 2100 at phase 4's close; โˆ’1 verified โ€” + registry-conformance.unit.test.ts's cross-check against + readers.cjs, deleted with its `require`-based helper + telemetry-cost-readers.ts once the file it required is gone; + the remaining +8 is telemetry-claim.unit.test.ts (31 tests, + up from phase 4) and diagnose-telemetry-use-case.unit.test.ts + (9 tests) growing earlier in this phase to cover the export + route, not from task 5's own collateral fixes โ€” + plugin-asset-translation.unit.test.ts: 20 tests today; the + repoint changed fixture content, not test count) +cli integration 608 passed (unchanged) +cli e2e 214 passed (was 210 at phase 4's close, documented; grew earlier in + phase 5 to 217 per the prior session's own account, not a + count this session measured directly; task 5 itself, + measured: โˆ’5 telemetry-check.e2e.test.ts's script-parity + describe block, its comparison subject deleted; +2 new + telemetry-check-skill-commands.e2e.test.ts) +plugin suite 233 passed (was 337; โˆ’104 scripts/__tests__/telemetry-check.test.js, + the checker it exercised deleted whole) +tsc, biome, check-cli-layering, check-markdown-links clean +node cli/dist/cli.js telemetry check run once post-build: exits 0, prints the expected + "measurement is off" gate line โ€” the exact step + cli-ci.yml's Windows job now runs +``` + +## Phase 6 โ€” the promise, and the absent CLI + +### Task 1: one wording, pinned + +The three locating actions' absent-CLI wording (`00-init/actions/01-check.md`, +`01-cost/actions/01-locate.md`, `02-check/actions/01-locate.md`) was two hyphens and one +em dash before this task: phase 5's rewrite of `02-check/actions/01-locate.md` used `โ€”` +in "**recording is unaffected** โ€” the hooks..." where the other two use ` - `. Fixed to +match. `scripts/__tests__/telemetry-cli-required.test.js` (new, 4 tests) pins the block +character for character across all three, confirmed red before the fix and green after by +temporarily reintroducing the em dash and re-running. + +### Task 2: the promise, corrected + +- `plugins/aidd-telemetry/README.md` already carried the three-act table from earlier work + in this plan, but one sentence still lied: "`02-check` still runs a script this plugin + ships, and needs nothing installed" โ€” true before phase 5, false after it deleted + `02-check/scripts/`. Corrected to "All three reach the CLI...". +- `docs/FAQ.md` claimed "No account, no server, no second tool to install" โ€” false since + phase 1: turning measurement on and reading it back both need `aidd`. Corrected to name + the CLI as required for those two acts, recording excepted. +- `docs/CATALOG.md`'s `aidd-telemetry` entry named no dependency at all; added the same + one-line split. +- Searched the whole repository (every `.md` file mentioning "telemetry", plus a grep for + "no CLI" / "without...CLI" / "nothing installed" / "no second tool") for any remaining + false promise. One more found, worse than a stale name: + `aidd_docs/product/cost-report-contract.md`'s "Filters" section didn't just say "the + plugin script" for a script gone since phase 1 โ€” it asserted `aidd telemetry report` + **refuses `--axis`** outright + (`error: unknown option '--axis'`), which stopped being true the same phase, when + `--axis` was ported onto the CLI directly (`cli/src/application/commands/telemetry.ts`). + Verified live against the built CLI before rewriting: `aidd telemetry report --axis + bogus` exits `1` with `Error: Unknown axis 'bogus'. Expected one of: total, day, step, + model, tool, project.`, and `--axis total --json` together print the JSON object โ€” `--json` + wins, `--axis` is silently ignored, never the reverse. The paragraph now states both. + +### Task 3: recording survives the CLI's absence โ€” proven end to end + +Phase 6's architecture projection lists no new or modified file under `cli/tests/e2e/` for +this task. On inspection, one existing test proved 3.1 outright, and nothing proved 3.2 in +the journey's own terms โ€” `telemetry-lifecycle.e2e.test.ts`'s "lives the whole sequence..." +journals and reads with `aidd` stripped from `PATH` throughout, but every call there +invokes `dist/cli.js` by its built path, including the switch (`switchTo("on")`); the CLI +was present and doing the switching the entire time, never absent. Closed by extending +`telemetry-plugin-standalone.e2e.test.ts`'s existing describe block with a second test, +"reads a session's figures complete, though the CLI did not exist when it ran": + +- Journals a whole session (`session_start`, a skill, a file write into a task folder, + `turn_end`) exactly as the first test in that file does, with `aidd` nowhere on `PATH` + and no CLI invoked at any point during the write. +- Only then calls `runCli` โ€” the first and only invocation of `dist/cli.js` in the test, + after every write has already happened โ€” to `read`, then `report --json`, `report`, and + `report --task `. +- The load-bearing assertion is the `--task` one, not the token totals: task identity + exists only in the journal's own `file_written` line, which the transcript fixture has + no notion of at all. `--task` narrowing to the session's real figures rather than + "nothing in this selection" is possible only because `read` consulted that line. + `ReadLocalCostOptions`'s own doc comment independently confirms the mechanism: absent a + session id, `read` "reads every session the run journal knows about" โ€” the file just + written with no CLI present โ€” so even the plain `requests > 0` assertion already implies + the journal was consulted, since nothing else tells `read` a session exists at all. + Confirmed empirically too: re-running the same fixture with the journal step skipped + entirely (transcript present, no run file) returns `requests: 0` / "nothing in this + period" even for the plain, unfiltered report โ€” `read` never opens the transcript at + all, because the journal is the only thing that tells it a session exists to read. + +Full re-run: `pnpm exec vitest run --project=e2e +tests/e2e/telemetry-plugin-standalone.e2e.test.ts tests/e2e/telemetry-lifecycle.e2e.test.ts` +โ†’ 5 passed (was 4; the new test alone โ€” this was already the full pair of files, not +just one). + +### Task 4: Windows resolves `aidd` on its own PATH, not just by path + +Every existing suite on the Windows job โ€” including the "Chain - diagnose" step this +phase's predecessor added โ€” invoked `node cli/dist/cli.js ...` directly, which proves +nothing about whether `aidd` resolves as a command on that platform's `PATH`. The Windows +job's "Chain - diagnose" step became two steps: build, `pnpm pack`, and +`npm install -g` the tarball (the same shim generation a real +`npm install -g @ai-driven-dev/cli` produces), then `aidd --version` followed by +`aidd telemetry check` โ€” both lifted verbatim from what every skill's own locate action +names, run through the globally-resolved command rather than a path. A missing shim fails +`aidd --version` with "command not found" (exit 127), which fails the step and the job. + +`cli/package.json` already carries an `install:local` script doing the equivalent (build, +`pnpm pack`, `npm install -g --force`) for a developer's own machine, and the +CI step was written to call it at first. Not used: `install:local`'s tarball path is +resolved with `$(node -p "require('./package.json').version")`, bash command +substitution, but a package.json script runs through pnpm's own configured shell โ€” `cmd.exe` +on Windows unless `script-shell` says otherwise, which this repository never sets. Under +`cmd.exe` that substitution is a literal string, not a version lookup, and the install would +fail in a way no macOS test could surface. The two commands are inlined into the workflow's +own `run:` block instead, which the job's `defaults.run.shell: bash` guarantees is bash +regardless, with a glob (`ai-driven-dev-cli-*.tgz`) standing in for the version lookup. + +Verified on this machine (macOS, not Windows โ€” the platform-specific `.cmd`/PATHEXT +resolution this task exists for can only be confirmed by an actual Windows CI run, which +this session cannot trigger): `npm pack` on the built CLI produces +`ai-driven-dev-cli-5.2.1.tgz` (matching the workflow's own glob), and +`npm install -g --prefix ./dist/ai-driven-dev-cli-*.tgz --force` followed by +`aidd --version` / `aidd telemetry check` resolved and ran correctly (exit 0 both times), +against a throwaway prefix โ€” never this machine's real global `aidd`. `pnpm pack` itself +(the same call the workflow's own inlined step makes) could not be exercised locally: it +runs the package's `prepare: lefthook install` script regardless of who calls it, and this +checkout is a git worktree whose `core.hooksPath` lefthook refuses by design โ€” a local +environment quirk this specific machine hits, not something a fresh CI checkout would; a +plain, non-worktree checkout carries no such override. Substituted +`npm pack --ignore-scripts` locally to verify the pack-and-install mechanics regardless of +that one blocked step. + +### Net test counts + +``` +cli unit 2107 passed (unchanged from phase 5's close โ€” no cli/src file changed) +cli integration 608 passed (unchanged) +cli e2e 215 passed (was 214 at phase 5's close; +1 + telemetry-plugin-standalone.e2e.test.ts's new test, closing + the task 3.2 gap this phase's first pass had only flagged) +plugin suite 237 passed (was 233; +4 telemetry-cli-required.test.js, new) +tsc, biome, check-cli-layering, check-markdown-links clean +``` + +Both follow-ups this phase's first pass flagged and deferred are closed, not carried +forward: the `cost-report-contract.md` `--axis` claim (task 2) and the task 3.2 recording- +survives-the-CLI gap (task 3) are both fixed and tested above, in the same phase. No open +follow-up remains from this phase. + +## Assert pass โ€” `/aidd-dev:03-assert`, run after phase 6 + +A consolidated sweep against `cli/aidd_docs/memory/coding-assertions.md`'s six requirements +and five before-commit/before-push commands, across every file this whole plan (phases 1-6) +touched โ€” not just phase 6's own diff. + +**One real finding, fixed.** `pnpm jscpd` flagged a clone inside this plan's own files: +`asObject` โ€” an identical `(value: unknown) => Record | null` narrowing +helper โ€” duplicated verbatim between `infrastructure/adapters/telemetry-evidence-adapter.ts` +(phase 4) and `infrastructure/adapters/export-config-reader-adapter.ts` (phase 5), each with +its own private copy. Extracted to `src/domain/formats/plain-object.ts` (`asPlainObject`, +with a doc comment naming exactly this problem), both adapters now import it, and +`tests/domain/formats/plain-object.unit.test.ts` (4 tests) pins its four cases (object, +array, null, primitive). Verified: `jscpd`'s clone count dropped 81 โ†’ 80, and the remaining +80 are all pre-existing, none touching any file this plan added or modified. + +**Left alone, on purpose.** `infrastructure/adapters/person-identity-adapter.ts` carries its +own third copy of the same shape, pre-existing and outside this plan's diff โ€” not touched, +since its contract differs (`{}` on failure, never `null`) and touching a file no phase of +this plan otherwise names would be scope creep beyond "leave no debt from this plan's own +changes." + +**Checked and judged compliant, no fix needed.** The two adapters' bare `catch { return +null }` / `catch { return false }` (reading a project's optionally-absent settings or config +file) were checked against "no silent errors โ€” throw early, fail loudly." The codebase +already carries a deliberate dual pattern for this exact tension, in the same +`person-identity-adapter.ts`: a lenient `read()` (bare catch, `null` on anything wrong) beside +a strict `readStrict()` (rethrows a named `UnreadableIdentityFileError` once the file is +confirmed to exist but fails to parse). This plan's two adapters follow the lenient shape, +which matches `aidd telemetry check`'s own design: every claim is a verdict, never a crash, +so a diagnostic that threw on a malformed `settings.json` would defeat its own purpose. +**One real UX edge this does accept, named rather than left implicit:** a genuinely +malformed `.claude/settings.json` (bad JSON, not merely absent) currently reads as "export +not configured" โ€” the same line as a settings file that was never touched โ€” rather than +"your settings file is corrupt." Defensible for a diagnostic that must always answer, but a +person debugging why their own export never turned on would get a less specific message +than the file's own error could give them. Worth a small follow-up issue if that +distinction ever matters in practice; not fixed here, since nothing in any of the six +phases' acceptance criteria asks for it. + +**Everything else, verified clean, nothing to fix:** `tsc --noEmit`, `pnpm lint` (biome), +zero new runtime dependency imported anywhere in this plan's files (the 6-dependency cap +stands untouched), every domain file this plan added imports only from `domain/` (checked +by hand and by `check-cli-layering.mjs`), `pnpm build` (554.7 KB, within the 560 KB budget). + +### Final sweep, one pass, nothing regressed + +``` +cli unit 2111 passed (was 2107; +4 plain-object.unit.test.ts) +cli integration 608 passed (unchanged) +cli e2e 215 passed (unchanged) +pnpm test (all three projects together) 2934 passed +plugin suite (node --test) 237 passed (unchanged) +tsc, biome, knip:production, check-cli-layering, check-markdown-links clean +jscpd 80 clones (was 81; the one inside this plan's files fixed, the rest pre-existing + and outside every file this plan touches โ€” informational in CI, not a hard gate) +``` + +## Review pass โ€” phases 4-6, and the one contract it found unguarded + +The phases-4-6 review (`review.md`, verdict **approved**) started from what was *deleted* +rather than from what shipped: the 99 test titles of `scripts/__tests__/telemetry-check.test.js` +mapped against their new homes. The deleted suite held three identity guards โ€” `switch.cjs`, +`repo.cjs` and `unrecognised.cjs` each "stays identical to the hook's own". Two are moot now +(the CLI reimplements them in TypeScript, and `telemetry-plugin-standalone.e2e.test.ts` drives +the real hook end to end, which is stronger than a byte comparison). The third was not, and +nothing had replaced it. + +**Found by mutation, not by reading.** `unrecognised_payload` is written in +`hooks/lib/record.cjs:268` and read in `telemetry-evidence-adapter.ts:30` โ€” two packages, two +languages, one string. Renaming the hook's literal to `unknown_payload` left +`telemetry-check.e2e.test.ts` **11/11 green** and `aidd-telemetry-journal.test.js` **186/186 +green**: the plugin side asserts only that the marker *file* exists, never its `type`, and the +CLI side typed the same literal into its own fixture, so it was checking itself against itself. + +The cost was never a failed run โ€” it was a wrong answer. With the marker unread, a payload +that *did* arrive reports as "the hook has never been observed firing": an unknown printed as +a nothing, the one thing this layer promises never to do. + +**Fixed by making the hook produce the fixture.** A twelfth case in +`telemetry-check.e2e.test.ts` spawns `hooks/journal.cjs session-start` with a payload matching +no declared host and reads whatever file the hook writes. Re-mutated to prove the guard bites: + +``` +ร— names an unrecognised payload the real hook wrote, not one this test typed + โ†’ expected ' hook fired FAIL no run ...' to match /matched no known host/u +``` + +That failure message *is* the degradation. The cheaper stopgap โ€” asserting the literal from +`record.cjs` in the plugin suite โ€” was deliberately not taken: it pins the string, not the +contract. + +``` +cli e2e 216 passed / 30 files (215 before; +1) +tsc, biome clean +plugins/aidd-telemetry/hooks/ restored byte-identical after each mutation, tree clean +``` + + +## Review pass 2 โ€” `/aidd-dev:05-review`, independent, all 6 phases + +Run by a fresh `aidd-dev:checker` agent, not the executor who wrote the code โ€” the +executor's own guardrail forbids judging its own work. Corrected three things about the +brief it was given: the branch is `claude/telemetry-cli-owns-read`, not +`claude/aidd-telemetry-layer-e403uf` (a merge-base); the work is already committed +(`7fe3e101..d08c3d57`, 6 commits) โ€” only the `plain-object.ts` extraction and its adopters, +the twelfth `telemetry-check.e2e.test.ts` case, and this file were still uncommitted; e2e +was 216, not 215 (this file's own phase-6 section undercounted by one, written before the +prior review pass's twelfth case landed). + +**Verdict: changes-requested โ€” 0 critical, 4 warning, 2 minor.** All six fixed in this +pass; each verified independently below, not just re-asserted. + +1. **๐ŸŸก rot** โ€” `scripts/__tests__/aidd-telemetry-cost-skill.test.js`'s `reportCommands()` + still matched `` `node โ€ฆ` ``, a pattern phase 1 deleted every + instance of. Zero matches, empty array, the loop asserting "every report call names + `--json` or `--axis`" passed by iterating nothing โ€” enforced nowhere. Fixed: the pattern + now matches `` `aidd telemetry report ` ``, anchored so a bare mention of the + command in prose (SKILL.md's transversal rules) still cannot match, plus an explicit + `assert.ok(commands.length > 0)` floor so this exact silent-emptying cannot recur + unnoticed. Confirmed non-vacuous: the four real commands in `01-cost`'s markdown are + found and checked. +2. **๐ŸŸก code** โ€” `hook-trust-reader-adapter.ts`'s `describeError` read `error.message` + where the deleted `hook-trust.cjs:57` read `error.code || error.message`. Live before + the fix: `Codex's own hook trust state could not be read either (.../config.toml could + not be read (ENOENT: no such file or directory, open '.../config.toml')` โ€” the path + twice in one sentence. Fixed to prefer `.code`; live after: `... could not be read + (ENOENT))`. Left distinct from `person-identity-adapter.ts`'s own `describeError` + (`.message` only) rather than merged into one shared helper โ€” that one describes a JSON + parse error, which carries no useful `.code`; unifying them would paper over a real + difference in what the two are describing. +3. **๐ŸŸก rot** โ€” `docs/FAQ.md` still said "nothing ever leaves your machine" unqualified in + two places, while `README.md`'s own rewrite (this plan) qualified it with "on its own" + โ€” the `aidd telemetry endpoint` exception. Both FAQ lines now read "on its own" too. +4. **๐ŸŸก fit โ€” issue #617.** Real, stale rationale, not a lost capability: #617 argues "The + CLI keeps one job only: turning the export on. Everything that reads belongs to the + plugin," which this plan's own `aidd telemetry check` contradicts by design, and no file + in the diff answers that argument. The reviewer independently confirmed the mechanism + still works from inside a session (`resolveSessionAnchor` reads the inherited + `CODEX_THREAD_ID`/`CLAUDE_CODE_SESSION_ID`) โ€” so #617's *acceptance criteria* are met, + only its stated design preference is overridden. **Not resolved by the executor**: + commenting on a live GitHub issue is an external side effect outside this session's + authority to take unprompted, the same as a commit. A reconciling comment is drafted and + held for the user's go-ahead, not posted. +5. **๐ŸŸข rot** โ€” `aidd-telemetry-cost-skill.test.js`'s `scriptFlags()` was dead: defined, + never called, parsed a deleted script's source. Removed. +6. **๐ŸŸข code** โ€” noted alongside finding 2 above (`describeError` "duplication" was a + byproduct of finding 2's bug, not a separate defect โ€” fixing the semantics diverged the + two functions, which is what should have been true from the start). + +### Re-verified after the fixes, one pass + +``` +cli unit 2111 passed (unchanged) +cli integration 608 passed (unchanged) +cli e2e 216 passed (unchanged โ€” the fixes touched no e2e assertion's shape) +pnpm test (all three projects together) 2935 passed +plugin suite (node --test) 237 passed (unchanged โ€” same 19 tests in the + fixed file, no longer vacuous) +tsc, biome, knip:production, check-cli-layering, check-markdown-links clean +jscpd 80 clones (unchanged โ€” the describeError pair was already below threshold; fixing + it for correctness diverged the two functions rather than removing a counted clone) +``` + +Outstanding before merge, unchanged from before this pass: the diff review is now done +(`review.md`, this section); #617 needs the user's decision on the drafted comment before +it is posted. diff --git a/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/phase-1.md b/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/phase-1.md new file mode 100644 index 000000000..9aedf35f5 --- /dev/null +++ b/aidd_docs/tasks/2026_08/2026_08_26_cli-owns-the-read/phase-1.md @@ -0,0 +1,126 @@ +--- +status: implemented +--- + +# Instruction: `01-cost` calls the CLI + +## Architecture projection + +> Tree of the final files. โœ… create ยท โœ๏ธ modify ยท โŒ delete +> +> First because it needs no new CLI command: `read` and `report` already exist. It deletes +> the most (2,413 lines) and proves the whole pivot before a line of new surface is written. + +```txt +. +โ”œโ”€โ”€ cli +โ”‚ โ””โ”€โ”€ tests +โ”‚ โ””โ”€โ”€ e2e +โ”‚ โ”œโ”€โ”€ telemetry-plugin-matches-cli.e2e.test.ts โŒ it compared two implementations; one remains +โ”‚ โ””โ”€โ”€ telemetry-cost-skill-commands.e2e.test.ts โœ… every command 01-cost names, accepted by the CLI +โ”œโ”€โ”€ plugins +โ”‚ โ””โ”€โ”€ aidd-telemetry +โ”‚ โ””โ”€โ”€ skills +โ”‚ โ””โ”€โ”€ 01-cost +โ”‚ โ”œโ”€โ”€ SKILL.md โœ๏ธ names aidd, not a script beside it +โ”‚ โ”œโ”€โ”€ actions +โ”‚ โ”‚ โ”œโ”€โ”€ 01-locate.md โœ๏ธ locating a script becomes requiring the CLI +โ”‚ โ”‚ โ”œโ”€โ”€ 02-collect.md โœ๏ธ aidd telemetry read +โ”‚ โ”‚ โ””โ”€โ”€ 03-report.md โœ๏ธ aidd telemetry report --axis --from --to --json +โ”‚ โ”œโ”€โ”€ package.json โŒ no script left to declare commonjs for +โ”‚ โ””โ”€โ”€ scripts/ โŒ 8 files, 2,413 lines +โ””โ”€โ”€ scripts + โ””โ”€โ”€ __tests__ + โ”œโ”€โ”€ aidd-telemetry-cost-skill.test.js โœ๏ธ asserts the commands, not the script's flags + โ”œโ”€โ”€ telemetry-cost-readers.test.js โŒ the readers it exercised are deleted + โ”œโ”€โ”€ telemetry-cost-report.test.js โŒ the report it exercised is deleted + โ”œโ”€โ”€ telemetry-cost-sink.test.js โŒ the sink it exercised is deleted + โ””โ”€โ”€ telemetry-where-things-live.test.js โœ๏ธ keeps the sink-location tests, loses the copy guards +``` + +## User Journey + +```mermaid +flowchart TD + A[Person asks 01-cost what August cost] --> B{aidd answers?} + B -- no --> C[Stop, and say the CLI is required to answer] + B -- yes --> D[aidd telemetry read] + D --> E[Every journalled session is swept into the sink] + E --> F[aidd telemetry report --axis step --from --to] + F --> G[One answer, from one implementation] +``` + +## Test Scope + +```mermaid +--- +title: Test scope +--- +journey + section Setup + Journal a real multi-step session in a fixture project => a session with figures to read exists: 5: system + Run the scripts one last time and keep the envelope => a reference answer exists: 5: cli + Build the CLI and put aidd on the PATH => the skill's commands can run: 5: system + section Happy path + Run aidd telemetry read => the journalled sessions are swept: 5: cli + Run aidd telemetry report --axis step => the same step rows as the reference: 5: cli + Run aidd telemetry report --json => every breakdown reconciles to its own total: 5: cli + section Edge case - a command the skill names is not accepted + Extract each command from 01-cost's markdown => run it => the CLI accepts every one: 1: cli + section Edge case - the CLI is absent + Remove aidd from the PATH => ask the skill to answer => it stops, names the CLI, and says recording is unaffected: 1: cli + section Teardown + Remove the fixture project and its sink => the machine's own figures untouched: 5: system +``` + +## Tasks to do + +### `1)` Capture the reference, in two artefacts that prove different things + +> Once the scripts are gone there is nothing left to compare against. A committed fixture has +> to be reproducible in CI, and it must not be somebody's real usage: this repository is +> public, and the layer's own rule is that nothing leaves the machine. + +1. **Committed, synthetic.** Build a sink covering every shape the readers produce โ€” `request` + and `session` kinds, a record with a model and one without, a stated step and an + unattributed one, several tools and several days. Run `telemetry-report.cjs report --json` + over it and commit both the sink and the envelope. +2. Assert the fixture carries at least two distinct steps and a non-zero total, so a later + vacuous pin fails loudly rather than passing on emptiness. +3. **Not committed, real.** Run the script and the CLI over the machine's own sink and compare + the two envelopes. A synthetic fixture agrees with the code that reads it; only data nobody + authored for this test can disagree. Record the outcome in the phase's notes, as evidence + rather than as a test CI can rerun. + +### `2)` Rewrite what `01-cost` tells the agent to run + +> Every `node