diff --git a/aidd_docs/product/usage-contract.md b/aidd_docs/product/usage-contract.md index 54a4207c2..804df61ea 100644 --- a/aidd_docs/product/usage-contract.md +++ b/aidd_docs/product/usage-contract.md @@ -149,11 +149,11 @@ Directory: `AIDD_TELEMETRY_DIR`, else `AIDD_USER_CONFIG_DIR/telemetry`, else `XD | `ledger/.lock` | CLI | CLI | `{pid, created_at}`; dead process or older than ten minutes: cleared | | `bindings/.lock` | CLI | CLI | same format. Declarations and snapshots run under it; `task` never takes the ledger lock, which an ingest holds while reading transcripts | | `bindings/sessions.jsonl` | `task` | hooks, `report`, `task` | `{session_id, task, ticket, none, declared_at, by}`, `by` = `command` \| `hook-intercept` | -| `bindings/branches.jsonl` | `task`, `ingest`, `report` | `report`, `forget` | branch declaration at a moment, with branch creation time from its reflog | +| `bindings/branches.jsonl` | `task`, `ingest`, `report` | `report`, `forget` | branch declaration at a moment, with when the branch took its name, from its reflog | | `bindings/carries.jsonl` | `SessionStart` hook | hooks, `report` | `{session_id, from, at}` | | `bindings/processes.jsonl` | `SessionStart` hook | `SessionStart` hook | `{pid, session_id, source, at}` | | `identity.json` | `identity`, `on` | `report`, `status`, `on` | `{"person_id": ""}`, owner-only | -| git config `--local` `branch..aiddTask`, `aiddTicket`, `aiddDeclaredAt` | `task` | hooks, `task`, `report` via snapshots | branch declaration; follows a rename, leaves with the branch | +| git config `--local` `branch..aiddTask`, `aiddTicket`, `aiddDeclaredFor`, `aiddDeclaredAt` | `task` | hooks, `task`, `report` via snapshots | branch declaration, `aiddDeclaredAt` written last, `aiddDeclaredFor` = ``; see [Branch declaration](#branch-declaration) | | git config `--local` `aidd.telemetry` | `on`, `off`, `forget` | hooks, CLI | `2:` or `off`. Half of consent: alone it grants nothing | - A line not exactly its store's format is skipped, never guessed. Every JSON Lines store is append-only except the ledger. @@ -178,8 +178,25 @@ flowchart TD - A declaration of no task is `declared-none`. - Declaring a branch late moves its earlier calls onto the task. +- A branch snapshot with no `declared_at` attributes nothing. - Sub-agent and advisor calls follow their parent session. +### Branch declaration + +A branch is declared only when the command declared it on that branch. Both readers, the CLI (`parseBranchConfig`) and the hook (`lib/git.cjs`), apply the same rule, pinned by the shared fixture. + +| git config of the branch read | Declared? | Why | +| --- | --- | --- | +| `aiddDeclaredAt` set, `aiddDeclaredFor` = this branch | yes | written by `task` on this branch | +| `aiddDeclaredAt` set, no `aiddDeclaredFor` | yes | written by cli-v5.5.0 or earlier, which did not stamp. Kept readable: reports and the gate do not change for data already written. Declaring again stamps it | +| `aiddDeclaredAt` set, `aiddDeclaredFor` = another branch | no | carried by `git branch -c` (copy) or `git branch -m` (rename), which move every `branch..*` key | +| `aiddTask` or `aiddTicket` without `aiddDeclaredAt` | no | set by hand, or a declaration cut short: `task` writes `aiddDeclaredAt` last | +| any declaration key listed twice | no | left by `git branch -C` or `-M` onto a branch that had keys: the copied declaration and the branch's own are mixed | + +- **Rename** (`git branch -m A B`): B is undeclared (unless A's declaration is unstamped, see the limits), so the gate asks on B. One declaration on B covers B from when it took the name: the newest reflog entry renaming or copying a branch to B (git or libgit2), else B's oldest entry. Calls made while it was A keep A's task, and on a name used before, the earlier B keeps its own. git moves the keys the same way for a copy and a rename, so the stamp cannot tell them apart; the reflog could, but its entries expire and the declaration would change meaning with them: one question after a rename is the cost. +- **Copy** (`git branch -c A B`): B is undeclared until declared on B. +- **Copy or rename onto an existing branch** (`-C`, `-M`): git keeps both sets of keys, so some are listed twice and the two declarations mix. A branch with any declaration key listed twice is undeclared; declaring again replaces every value. + ## Ask `UserPromptSubmit` (`hooks/prompt-gate.cjs`) blocks a prompt before the model when all hold: @@ -193,7 +210,8 @@ flowchart TD | CLI usable | `aidd` on `PATH` and knows `telemetry task` | - Anything else passes. A crash exits 0: the hook never fails a prompt. -- Asks once per working branch; switching to an unbound branch asks again. +- Re-checked on every prompt: each prompt on an undeclared working branch is blocked until the branch or the session is declared (`--none` included). A declared branch is never asked again, unless a copy or rename onto it changes its keys; switching to an undeclared one asks. +- Why every prompt and not once: the gate exists so that no token of the work is spent before it has a task ([spec](../tasks/2026_10/2026_10_09_deterministic-tokens/spec.md), [target](../tasks/2026_10/2026_10_09_deterministic-tokens/context/target.md)). "Once per working branch" there meant answered once and remembered; letting the next prompt through would spend tokens unattributed. Answering never blocks and never reaches the model: - `! aidd telemetry task ...` in the terminal fires no `UserPromptSubmit`. @@ -304,6 +322,11 @@ None observable by the CLI. | Call read while git config, `.git` or git dir unreadable | `unreadable-consent`, offsets move on: not recovered on repair | - | | pid reused within one boot by a Claude process whose `startup` was never recorded (e.g. started before `on`) | can take the dead process's last session as predecessor, so `/clear` carries its task | announced, replaced by first declaration | | `AIDD_TELEMETRY=0` | stops hooks too; a key turned off meanwhile is observed at the next run without it | - | +| Declaration keys edited by hand: every key including the stamp, `aiddTask` and `aiddDeclaredAt` alone (read as unstamped), or a stamp removed or emptied | the branch reads as declared: no reader can tell who wrote a git config key | - | +| Copy or rename of an unstamped (cli-v5.5.0) declaration | the new name inherits it | declare on the new name, or on the original to stamp it before copying or renaming | +| Calls on a name used before, after a copy, rename or recreation and before it is declared again | the earlier generation's declaration still answers in the report, while `status` and the gate say undeclared: a branch with no declaration takes no snapshot, so nothing marks the new generation. Only calls the gate did not hold (unattended runs, IDE extensions) can land there | declare right after reusing a name | +| Reflog entry of the rename or copy expired before B's first declaration | B's start is its oldest remaining entry; on a name used before, an older start can let the earlier B's declaration speak for later calls | declare soon after a rename or copy | +| Branch snapshots recorded before stamping (a copied branch, a hand-set task with a `declared_at`) | keep attributing: attribution reads snapshots, `branches.jsonl` is append-only | - | ## Never leaves the machine diff --git a/aidd_docs/tasks/2026_10/2026_10_09_deterministic-tokens/spec.md b/aidd_docs/tasks/2026_10/2026_10_09_deterministic-tokens/spec.md index ed0382b0b..3f89444a6 100644 --- a/aidd_docs/tasks/2026_10/2026_10_09_deterministic-tokens/spec.md +++ b/aidd_docs/tasks/2026_10/2026_10_09_deterministic-tokens/spec.md @@ -39,6 +39,9 @@ coming only from what a person declared, with the previous telemetry removed ent > **Deviation, 2026-10-12, maintainer decision.** IDE extensions are not asked: VS Code shows > no hook message, so a block would eat the prompt. Declare in a terminal; it binds the branch. > Evidence: [target.md](context/target.md). + > **Reading, 2026-10-16.** "At most once" means answered once and remembered for the branch: + > the gate holds every prompt until then, as "before any token of that work is spent" requires. + > See the usage contract, section Ask. - A session started by `/clear` keeps the task of the session it replaced, says so to the person, and a later declaration in it re-attributes the whole session. - Work with no declared task is its own row, with the reason it has none, and is attributed diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/contract.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/contract.md new file mode 100644 index 000000000..9cdbf5cff --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/contract.md @@ -0,0 +1,51 @@ +--- +status: done +--- + +# A declaration comes only from the command + +## Need + +The product rule ([usage-contract](../../../product/usage-contract.md#attribution)): a task and a ticket come only from `aidd telemetry task`. The R1 telemetry audit found two ways around it, and one sentence of the contract that the hook does not do. + +| Finding | Observed | +| --- | --- | +| B1 | Both readers of a branch declaration (CLI `parseBranchConfig`, hook `lib/git.cjs` + `lib/binding.cjs`) bind a branch on a hand-set `aiddTask` with no `aiddDeclaredAt`. Attribution treats a snapshot with `declared_at: null` as speaking from the branch's creation. | +| B1 | `git branch -c A B` copies every `branch.A.*` key, `aiddDeclaredAt` included: B inherits a declaration it never received, and its calls go to A's task. | +| M8 | The contract says the gate "asks once per working branch". It blocks every prompt until the branch is declared. | + +## Decisions + +| # | Decision | Why | +| --- | --- | --- | +| 1 | The command also writes `branch..aiddDeclaredFor = `, before `aiddDeclaredAt`, which stays last. | Ties the declaration to the branch it was made on. A copy or a rename moves the keys under another name; the value keeps the old one. | +| 2 | A branch is declared only when `aiddDeclaredAt` is set and `aiddDeclaredFor`, when present and non-empty, equals the branch read. Otherwise it is undeclared: no task, no ticket, no `none`. | `aiddDeclaredAt` is the last key the command writes, so it alone says the command finished. The stamp rejects a copied declaration. | +| 3 | A declaration with no `aiddDeclaredFor` (written by cli-v5.5.0 and earlier) is read as before, if `aiddDeclaredAt` is set. Declaring again stamps it. | Beta, but data written by a released CLI keeps its meaning. Rejecting it would make `status` and the gate ask again on every branch already declared, for a hole only a copy opens. | +| 4 | `git branch -m A B` leaves B undeclared (a stamped declaration; see the limits for an unstamped one): the gate asks on B, and one declaration on B covers it from when it took the name B. Calls made on A keep A's task. | git moves the config keys the same way for a copy and a rename, so the stamp cannot tell them apart; the reflog could, but its entries expire, and a declaration that holds only while an entry lasts would change meaning on its own. The cost is one question after a rename, and nothing is lost: calls on B all come after B took its name. This replaces "follows a rename" in the usage contract. | +| 5 | A branch snapshot with `declared_at: null` attributes nothing. | Such a snapshot comes only from keys set by hand or a declaration cut short: every release wrote `aiddDeclaredAt`. | +| 6 | M8: the doc is wrong, not the hook. The gate blocks every prompt on an undeclared working branch until it is declared (or declared `--none`). A declared branch is never asked again, unless a copy or rename onto it changes its keys. | The spec that introduced the gate (`2026_10_09_deterministic-tokens/spec.md`) requires asking "before any token of that work is spent", and its target (`context/target.md`, "When to ask") re-checks every prompt. "Once" meant "remembered once answered". Letting later prompts through would spend tokens unattributed and need per-branch state. | +| 7 | A branch with any declaration key listed twice is undeclared, and the command replaces every value of a key (`--replace-all`, `--unset-all`). Added after review rounds 1 and 2. | `git branch -C` or `-M` onto an existing branch keeps both sets of keys: reading the last value of each mixed the copied declaration into the branch's own (a ticket or task the own one never set), and a plain `git config` set refuses a key with two values, so declaring again failed. | +| 8 | A branch's start is the newest reflog entry where it took its name by a copy or a rename (git's `Branch: renamed|copied`, libgit2's `branch: renamed`), else its oldest entry. Added after review rounds 3 and 4. | A copy or a rename brings the source's reflog, so the oldest entry is the source's creation. On a name used before, that made the older branch's declaration look newer than the one made after the copy, and the report kept it while `status` and the gate showed the new one. | + +## Limits (stated, not fixed) + +| Limit | Why it stays | +| --- | --- | +| Hand edits of the keys declare a branch: every key including the stamp, `aiddTask` and `aiddDeclaredAt` alone (read as unstamped), or a stamp removed or emptied. | git config is the person's own; no reader can tell who wrote a key. | +| A copy or a rename of an unstamped (legacy) declaration is inherited until the new name is declared again. | Decision 3. | +| Calls on a name used before, after a copy, rename or recreation and before it is declared again, still go to the earlier generation's declaration in the report. | A branch with no declaration takes no snapshot; only calls the gate did not hold can land there. Usage contract, residual limits. | +| A rename or copy entry expired from the reflog before the first declaration: the oldest remaining entry is the start. | Same class as any reflog expiry. Usage contract, residual limits. | +| Snapshots recorded before this change (a copied branch, a hand-set task with a `declared_at`) keep attributing. | Attribution reads only snapshots; `branches.jsonl` is append-only and its format is unchanged. | + +## Acceptance + +| # | Criterion | +| --- | --- | +| 1 | Sandboxed e2e: declare on A, `git branch -c A B`, calls on B: `report --axis ticket --json` does not give B's calls A's ticket. | +| 2 | Sandboxed e2e: `aiddTask` set by hand without `aiddDeclaredAt`: the branch's calls are `no-binding`. | +| 3 | Sandboxed e2e: after `git branch -m A B`, B's calls are `no-binding`; declaring on B attributes them. | +| 4 | Sandboxed e2e: an unstamped declaration with `aiddDeclaredAt` still attributes. | +| 5 | The hook applies the same rule, tested in `scripts/__tests__/` through real git, from the shared fixture both readers run. | +| 6 | `forget` removes `aiddDeclaredFor`. | +| 7 | Each guard is shown by a mutation that turns its test red. | +| 8 | The usage contract, the CLI telemetry memory, the plugin README and skill say decisions 1 to 6 and the limits. | diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/evidence.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/evidence.md new file mode 100644 index 000000000..0048b85d5 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/evidence.md @@ -0,0 +1,168 @@ +# Evidence: A declaration comes only from the command + +Every run below is sandboxed (temp `HOME`, `XDG_CONFIG_HOME`, `AIDD_USER_CONFIG_DIR`, `AIDD_TELEMETRY_DIR`, `CLAUDE_CONFIG_DIR`, `GIT_CONFIG_NOSYSTEM=1`), as the suites set them. + +## Probe: what git does on copy and rename + +`git branch -c A B` and `git branch -m A C` in a scratch repository (git 2.x, macOS): + +```txt +branch.A.aiddtask t +branch.B.aiddtask t <- copy carries the key +B@{2026-10-01T08:00:00Z} <- and A's reflog, so A's creation time +Branch: copied refs/heads/A to refs/heads/B +C@{...} Branch: renamed refs/heads/A to refs/heads/C <- rename moves the key and the reflog +``` + +So a copied branch looks created when A was: the copy e2e below is red for the real reason, not for a creation time after the calls. + +## Red: e2e, before any code change + +`npx vitest run --project=e2e tests/e2e/telemetry-report-declaration.e2e.test.ts` + +```txt +× does not give a copied branch the declaration of the branch it was copied from + AssertionError: expected 3 to be 2 +× does not bind a branch on a task set by hand with no declaration time + AssertionError: expected { kind: 'value', key: 'by-hand', …(7) } to be undefined +× asks again after a rename, and one declaration covers the renamed branch + AssertionError: expected 3 to be 2 +Tests 3 failed | 1 passed (4) +``` + +The one passing is the legacy case (an unstamped declaration still attributes): green before, kept green by decision 3; its guard is a mutation below. + +## Red: CLI unit and integration, before any code change + +`npx vitest run` on the parse, attribution, source, store and forget specs: + +```txt +× reads nothing from a task or a ticket with no declaration time +× reads nothing from a declaration made for another branch, as a copy or a rename leaves it +× tells a stamp apart from a branch whose name differs only in case +× keeps the declared branches when another one is not declared +× lets a snapshot with no declaration time attribute nothing: no command wrote it +× reads nothing from a declaration the command did not make on that branch +× writes the keys the reader reads back, stamped with the branch it was made on +× leaves a renamed or copied branch undeclared: git moves the keys, the stamp keeps the old name +× removes exactly those keys, and says how many +Tests 9 failed | 116 passed (125) +``` + +## Red: the shared fixture, both readers, before any code change + +CLI, `npx vitest run tests/contexts/telemetry/telemetry-bindings-fixture.unit.test.ts`: + +```txt +× reads task-without-declared-at as the fixture says +× reads stamped-for-another-branch as the fixture says +× reads stamp-without-declared-at as the fixture says +× binds branch-task-without-declared-at-is-unbound +× binds branch-stamped-for-another-branch-is-unbound +× binds branch-stamp-without-declared-at-is-unbound +Tests 6 failed | 107 passed (113) +``` + +Hook, `node --test scripts/__tests__/aidd-telemetry-hooks.test.js`: + +```txt +✖ a branch declaration read back from real git follows the fixture +✖ binding follows the fixture, through real git and the shared files +✖ parseBranchConfig cuts the known suffix off a dotted branch name +✖ a branch the command did not declare is asked: a task set by hand, a declaration copied from another branch +ℹ tests 65 ℹ pass 61 ℹ fail 4 +``` + +`the gate asks on every prompt until the branch is declared, then never again` passes before any change: it pins what the hook already does (M8, decision 6), and the doc is what changes. + +## Green after the change + +- `npx vitest run tests/contexts/telemetry`: `Tests 1223 passed (1223)` (with the e2e file above). +- `npx vitest run --project=e2e tests/e2e/telemetry`: `Test Files 8 passed (8)`, `Tests 76 passed (76)`. +- `node --test scripts/__tests__/aidd-telemetry-hooks.test.js`: `ℹ tests 65`, `ℹ fail 0`. + +## Guards: each mutation turns its test red + +Each mutation applied alone to the source, the named suite run, the file restored. + +| # | Mutation | Suite | Red | +| --- | --- | --- | --- | +| M1 | `parseBranchConfig` keeps every branch (filter removed) | `vitest tests/contexts/telemetry` | 8+ tests, incl. `reads nothing from a task or a ticket with no declaration time` | +| M2 | stamp ignored (`declared_at !== null` only) | same | 7: `reads nothing from a declaration made for another branch…`, `binds branch-stamped-for-another-branch-is-unbound`, … | +| M2 | same | e2e declaration file | 2: copy, rename | +| M3 | `declared_at` not required | `vitest tests/contexts/telemetry` | 7: `reads nothing from a task or a ticket with no declaration time`, `binds branch-task-without-declared-at-is-unbound`, … | +| M3 | same | e2e declaration file | 0: the attribution guard (M6) still drops the snapshot. With M3 and M6 together: `× does not bind a branch on a task set by hand with no declaration time` | +| M4 | unstamped (legacy) rejected | `vitest tests/contexts/telemetry` | 8+: `still reads a declaration written before declarations were stamped`, … | +| M4 | same | e2e declaration file | 1: `still reads a declaration written before the branch was stamped` | +| M5 | store writes no `aiddDeclaredFor` | `vitest tests/contexts/telemetry` | 2: `writes the keys … stamped …`, `leaves a renamed or copied branch undeclared…` | +| M5 | same | e2e declaration file | 2: copy, rename | +| M6 | attribution keeps snapshots with no `declared_at` | `vitest tests/contexts/telemetry` | 1: `lets a snapshot with no declaration time attribute nothing…` | +| M7 | forget regex without `aidddeclaredfor` | same | 1: `removes exactly those keys, and says how many` | +| M8 | source regex without `declaredfor` | same | 2: `reads nothing from a declaration the command did not make on that branch`, rename/copy | +| H1 | hook: stamp ignored | `node --test …hooks.test.js` | 3: fixture via git, binding fixture, `a branch the command did not declare is asked…` | +| H2 | hook: `declared_at` not required | same | 3: same three | +| H3 | hook: legacy rejected | same | 3: fixture via git, binding fixture, `a bound branch, session or carried session passes` | +| H4 | hook regex without `declaredfor` | same | 3 | +| H5 | hook: `branchDeclaration` returns what it read, declared or not | same | 3 | + +## Mutation (`pnpm test:mutation:telemetry`, under the lock) + +- Run 1: `Report: reports/mutation/telemetry/ (score 96.3, floor 96)`. On changed lines, one survivor: `attribution.ts` `?? []` to `?? ["Stryker was here"]`, equivalent (a string is no snapshot of any generation, so the branch still binds nothing). +- The `?? []` was replaced by an explicit `if (snapshots === undefined) return null;`, which leaves no equivalent mutant. +- Run 2: `score 96.3, floor 96`. Mutants on the lines this change touches: `branch-binding.ts` 17 killed, `attribution.ts` 12 killed, store, source and forget adapters 1 killed each. None survived. + +## Gates + +| Gate | Shortest decisive line | +| --- | --- | +| `cd cli && pnpm test` | `Test Files 511 passed \| 1 skipped (512)`, `Tests 6668 passed \| 1 skipped (6669)` | +| `node --test 'scripts/__tests__/*.test.js'` | `ℹ tests 459`, `ℹ fail 0` | +| bundle budget | before: `693.2 KB / budget: 695 KB`; after: `693.6 KB / budget: 695 KB`, `OK: within budget` | +| `pnpm smoke` (`XDG_CONFIG_HOME`, `AIDD_TELEMETRY_DIR`, `CLAUDE_CONFIG_DIR` temp, `GIT_CONFIG_NOSYSTEM=1`; the script moves `HOME` and `AIDD_USER_CONFIG_DIR`) | `PASS: 132 FAIL: 0 SKIP: 1 · coverage 100%` | +| `node scripts/check-claude-tokens-end-to-end.cjs cli/dist/cli.js` | `2 calls, 393 tokens, all on task demo, ticket T-1, and on task demo in this repository` | +| same with `--attended` | `…; asked, answered by the hook, kept across /clear` | + +## Review round 1 fixes + +- Red first, `git-branch-binding-store-adapter.integration.test.ts`: `× declares again over a branch copied onto with -C, which leaves every key twice`, `Error: git config branch.feat/y.aiddTask failed`. Green after `--replace-all` / `--unset-all`: `Tests 13 passed (13)`. +- Probe: after `git branch -C x y` the config holds `[branch "y"]` twice, the copied section first; both readers kept the last value of a key. Round 2 showed this mixes the two declarations when the branch's own left a key unset (see below). +- Guards: plain set (S1) and plain unset (S2) each turn `declares again over a branch copied onto with -C…` red; the stamp mutation (M2) turns `asks again after a rename, and one declaration on the new name covers it` red (now checked on a call made on the renamed branch, `feat/c`). +- Gates on 703b646e: `cd cli && pnpm test` `Tests 6669 passed | 1 skipped (6670)`; `node --test 'scripts/__tests__/*.test.js'` `ℹ tests 459`, `ℹ fail 0`; bundle `693.5 KB / budget: 695 KB`; mutation `score 96.5, floor 96`, changed lines: `branch-binding.ts` 17, `binding-resolution.ts` 4, `attribution.ts` 15, store adapter 11, source and forget adapters 1 each, all killed; smoke `PASS: 132 FAIL: 0 SKIP: 1`; tokens end to end and `--attended`: same lines as above. + +## Review round 2 fixes + +- Red first: CLI `× reads nothing from a branch copied onto with -C, which lists a key twice`, `× reads nothing from a branch whose task alone is listed twice`, `× declares again over a branch copied onto with -C…`, `× reads copied-onto-a-branch-declared-none as the fixture says`, `× binds branch-copied-onto-a-declared-branch-is-unbound` (`Tests 5 failed | 1220 passed (1225)`); hook `✖ a branch declaration read back from real git follows the fixture`, `✖ binding follows the fixture…`, `✖ the gate asks on a branch copied onto with -C, whose keys are listed twice` (`ℹ fail 3`); e2e `× does not mix a declaration copied with -C into the one the branch already had`, `AssertionError: expected 3 to be 2`. +- Green: `Tests 1225 passed`, hook `ℹ fail 0`, e2e `Tests 5 passed (5)`. +- Guards: T1 (CLI ignores a key listed twice) turns 6 unit tests and the e2e red; T2 (`||=` to `=`) turns `reads nothing from a branch whose task alone is listed twice` red; HT1 (hook ignores it) turns 3 hook tests red; HT2 (`||=` to `=`) survived until the fixture case `task-listed-twice` was added, then `✖ a branch declaration read back from real git follows the fixture`. +- Gates on 0c5eab26: `cd cli && pnpm test` `Tests 6676 passed | 1 skipped (6677)`; `node --test 'scripts/__tests__/*.test.js'` `ℹ tests 460`, `ℹ fail 0`; bundle `693.6 KB / budget: 695 KB`; mutation `score 96.4, floor 96`, changed lines: `branch-binding.ts` 24, `attribution.ts` 15, store adapter 11, `binding-resolution.ts` 4, source and forget adapters 1 each, all killed; smoke `PASS: 132 FAIL: 0 SKIP: 1`; tokens end to end and `--attended`: unchanged lines. + +Count note (round 3, S2): after the round 2 changes the CLI telemetry suites hold `Tests 1226 passed (1226)` and the hook suite `ℹ tests 66`, `ℹ fail 0`; the 1225 above was before the `task-listed-twice` fixture case. + +## Review round 3 fixes + +- Probe, dated git in a scratch repository: every way a branch takes a name writes one reflog subject, and the source's reflog comes along. + +```txt +-C: b@{2026-10-05T00:00:00Z} | Branch: copied refs/heads/a to refs/heads/b + b@{2026-10-01T00:00:00Z} | branch: Created from main <- a's creation +-M: d@{2026-10-05T00:00:00Z} | Branch: renamed refs/heads/c to refs/heads/d +-D;-m: f@{2026-10-05T00:00:00Z} | Branch: renamed refs/heads/e to refs/heads/f +-c;-m: i@{2026-10-06T00:00:00Z} | Branch: renamed refs/heads/h to refs/heads/i + i@{2026-10-05T00:00:00Z} | Branch: copied refs/heads/g to refs/heads/h +``` + +- Red first, e2e: `× lets a declaration on a branch copied onto with -C reach the report, the source older`, `× lets a declaration on a name renamed onto, after the old branch was deleted, reach the report`, both `AssertionError: expected undefined to be 1` (the new declaration's row is missing: its call went to `old-b`). Integration: `× is when the branch took its name, for a copy or a rename onto a name used before`. Unit tests red on the missing `branchCreatedAt` export. +- Green: `vitest tests/contexts/telemetry` `Tests 1229 passed (1229)`; `vitest --project=e2e tests/e2e/telemetry` `Tests 79 passed (79)`. The rename e2e now dates its rename (10-08) before the call it adds, as it happens in real use. +- Guards: R1 (always the oldest entry) turns both new e2e tests, the integration test and `takes the newest entry where the branch took its name…` red; R2 (copies not matched) turns the integration and that unit test red; R3 (any target name) turns `ignores an entry that moved another name, or that names this one only as a prefix` red. +- Mutation, first run after the fix: `score 96.4, floor 96`; one survivor on a changed line, `selectorTime` `endsWith("}")` to `endsWith("")`, which no test told apart (a selector missing its brace also lost the last digit of its time). A test with a valid time and another last character now kills it: `× is none for a selector without its closing brace` under that mutation. +- Gates on the final code: `cd cli && pnpm test` `Tests 6681 passed | 1 skipped (6682)`; `node --test 'scripts/__tests__/*.test.js'` `ℹ tests 460`, `ℹ fail 0`; bundle `694.0 KB / budget: 695 KB` (1.0 KB headroom left); mutation `score 96.4, floor 96`, changed lines: `branch-binding.ts` 72, `attribution.ts` 15, store adapter 11, `binding-resolution.ts` 4, source adapter 2, forget adapter 1, all killed; smoke `PASS: 132 FAIL: 0 SKIP: 1 · coverage 100%`; tokens end to end `2 calls, 393 tokens, all on task demo, ticket T-1, …` and with `--attended` `…; asked, answered by the hook, kept across /clear`. + +## Review round 4 fixes + +- Red first: `× reads the rename libgit2 writes, in lower case, as git's own` (`Tests 1 failed | 60 passed (61)`). Green: `vitest tests/contexts/telemetry` `Tests 1230 passed (1230)`. Guard: the case-sensitive `/^Branch: (renamed|copied) /` turns that test red. +- Gates on the final code: `cd cli && pnpm test` `Tests 6682 passed | 1 skipped (6683)`; `node --test 'scripts/__tests__/*.test.js'` `ℹ tests 460`, `ℹ fail 0`; bundle `694.0 KB / budget: 695 KB`; mutation `score 96.4, floor 96`, changed lines: `branch-binding.ts` 67, `attribution.ts` 15, store adapter 11, `binding-resolution.ts` 4, source adapter 2, forget adapter 1, all killed, none survived; smoke `PASS: 132 FAIL: 0 SKIP: 1 · coverage 100%`; tokens end to end `2 calls, 393 tokens, all on task demo, ticket T-1, …`, with `--attended` `…; asked, answered by the hook, kept across /clear`. + +## Review round 5 fixes + +- Red first: `× lets a newer generation with only a snapshot no command wrote attribute nothing, not the older one`, `AssertionError: expected { state: 'attributed', …(2) } to deeply equal { state: 'unattributed', …(1) }`. Green: `vitest tests/contexts/telemetry` `Tests 1231 passed (1231)`. Guard: letting the undeclared snapshots of the chosen generation speak turns it and 2 other attribution tests red. +- Gates on the final code: `cd cli && pnpm test` first run `Tests 2 failed | 6681 passed | 1 skipped (6684)`, failing tests not captured; two reruns `Tests 6683 passed | 1 skipped (6684)`. The two failures did not recur and are not identified. `node --test 'scripts/__tests__/*.test.js'` `ℹ tests 460`, `ℹ fail 0`; bundle `694.0 KB / budget: 695 KB`; mutation `score 96.4, floor 96`, changed lines: `branch-binding.ts` 67, `attribution.ts` 15, store adapter 11, `binding-resolution.ts` 4, source adapter 2, forget adapter 1, all killed; smoke `PASS: 132 FAIL: 0 SKIP: 1 · coverage 100%`; tokens end to end and `--attended`: unchanged lines. diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-1.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-1.md new file mode 100644 index 000000000..b4581a8ff --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-1.md @@ -0,0 +1,22 @@ +--- +status: done +--- + +# Phase 1: The CLI writes the stamp and reads only the command's declaration + +## Architecture projection + +```txt +cli/src/contexts/telemetry/ +├── domain/branch-binding.ts ✏️ parseBranchConfig keeps only declared branches +├── domain/report/attribution.ts ✏️ a snapshot with no declared_at attributes nothing +├── infrastructure/git-branch-binding-source-adapter.ts ✏️ reads aiddDeclaredFor +├── infrastructure/declaration/git-branch-binding-store-adapter.ts ✏️ writes aiddDeclaredFor before aiddDeclaredAt +└── infrastructure/forget/repository-declarations-adapter.ts ✏️ removes aiddDeclaredFor +cli/tests/ ✏️ unit, integration, e2e (contract acceptance 1-4, 6) +``` + +## Tasks + +1. Red first: e2e copy, hand-set, rename then redeclare, legacy; unit parse cases; attribution null snapshot; store and forget adapters. Record red in `evidence.md`. +2. Code to green. One mutation per guard, recorded. diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-2.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-2.md new file mode 100644 index 000000000..98b21a189 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-2.md @@ -0,0 +1,19 @@ +--- +status: done +--- + +# Phase 2: The hook reads by the same rule + +## Architecture projection + +```txt +plugins/aidd-telemetry/hooks/lib/git.cjs ✏️ parseBranchConfig reads declared_for; branchDeclaration returns null unless declared by the command +plugins/aidd-telemetry/hooks/lib/binding.cjs ✏️ a branch binds when present +scripts/__tests__/fixtures/telemetry-bindings/ ✏️ cases: hand-set task unbound, copied stamp unbound, legacy bound, own stamp bound +scripts/__tests__/aidd-telemetry-hooks.test.js ✏️ gitConfig test reads expected answers, keys written under the case's branch +``` + +## Tasks + +1. Red first: fixture cases flipped and added; both the CLI fixture test and the hook test go red. Record. +2. Code to green. Mutation per guard, recorded. diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-3.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-3.md new file mode 100644 index 000000000..74f0ead03 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/phase-3.md @@ -0,0 +1,11 @@ +--- +status: done +--- + +# Phase 3: Contract, memory, plugin docs, gates + +## Tasks + +1. `aidd_docs/product/usage-contract.md`: store row, attribution rule, rename, legacy, limits, gate wording (M8). +2. `cli/aidd_docs/memory/telemetry.md`, `plugins/aidd-telemetry/README.md`, skill text where it describes declarations, `prompt-gate.cjs` header comment, fixture README. +3. Gates: `cd cli && pnpm test`, `pnpm test:mutation:telemetry` (under the lock), `node --test 'scripts/__tests__/*.test.js'`, bundle budget, `pnpm smoke`, `node scripts/check-claude-tokens-end-to-end.cjs cli/dist/cli.js` with and without `--attended`. diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/plan.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/plan.md new file mode 100644 index 000000000..8733c55b8 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/plan.md @@ -0,0 +1,29 @@ +--- +objective: "A branch is bound only by a declaration the command made on that branch, in the CLI and in the hook alike, and the contract says what the gate does." +status: done +--- + +# Plan: A declaration comes only from the command + +## Overview + +| Field | Value | +| ---------- | ----- | +| **Goal** | Hand-set, copied or renamed declaration keys bind nothing; legacy declarations still read; M8 doc fixed | +| **Source** | [`contract.md`](./contract.md), R1 telemetry audit (B1, M8), no issue | + +## Phases + +| # | Phase | File | +| --- | ----- | ---- | +| 1 | The CLI writes the stamp and reads only the command's declaration | [`phase-1.md`](./phase-1.md) | +| 2 | The hook reads by the same rule, from the shared fixture | [`phase-2.md`](./phase-2.md) | +| 3 | Contract, memory, plugin docs, gates | [`phase-3.md`](./phase-3.md) | + +## Decisions + +| Decision | Why | +| --- | --- | +| The stamp is a filter at config read, never stored in `BranchConfigBinding` or a snapshot | `branches.jsonl` keeps its format, no migration | +| An undeclared branch is left out of `parseBranchConfig`'s result | Nothing to snapshot, nothing to bind; `status` and attribution need no change of their own | +| The shared fixture `scripts/__tests__/fixtures/telemetry-bindings/` pins the rule | Both readers already run it; one case file, two tests | diff --git a/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/review.md b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/review.md new file mode 100644 index 000000000..1c8703746 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_16_declaration-comes-only-from-the-command/review.md @@ -0,0 +1,85 @@ +# Review: A declaration comes only from the command + +- Started: 2026-10-16 +- Reviewer: a fresh `aidd-dev:checker` agent per round, read-only, sandboxed runs, no mutation run (recorded in `evidence.md`) + +## Round 1 + +- Diff: `origin/next...ed999f81` +- Verdict: request changes, 7.5/10 + +| # | Class | Finding | Resolution (703b646e) | +| --- | --- | --- | --- | +| 1 | deal breaker | `docs/ARCHITECTURE.md:118` still said "Ask for a task once per branch" | reworded to the gate's real behaviour | +| 2 | suggestion | forget adapter comment said "three keys" | "four keys" | +| 3 | suggestion | usage contract: "Sub-agent and advisor calls…" fell under the new `Branch declaration` heading | moved back under Attribution | +| 4 | suggestion | change history sentence inside the contract | removed | +| 5 | suggestion | rename e2e proved coverage with a deleted branch's call | rewritten: rename `feat/a` to `feat/c`, a call made on `feat/c` is `no-binding`, then attributed by one declaration | +| 6 | suggestion | `-C`/`-M` onto a declared branch leaves keys twice; redeclaring fails | store uses `--replace-all` / `--unset-all`, red test first; rule written in the usage contract and as contract decision 7 | +| 7 | suggestion | always-true conditions left in `binding-resolution.ts` and `attribution.ts` | dropped; `BranchConfigBinding.declared_at` stays nullable because `BranchSnapshot` extends it and stored snapshots can hold `null` | +| 8 | suggestion | fixture JSON reformatted unrelated entries | fixture re-applied as text insertions; diff now only the new cases | + +Acceptance after round 1: 1, 2, 4, 5, 6 fulfilled; 3 and 8 partial (findings 5, 1, 3); 7 not re-run by the checker. + +## Round 2 + +- Diff: `origin/next...fd7a31d6` +- Verdict: request changes, 7/10. Round 1 findings 1-5, 7, 8 confirmed resolved; 6 partial. + +| # | Class | Finding | Resolution (0c5eab26) | +| --- | --- | --- | --- | +| D1 | deal breaker | after `git branch -C` onto a declared branch, both readers kept the last value per key, mixing the copied task or ticket into the branch's own declaration | any declaration key listed twice makes the branch undeclared, in both readers; red tests in each, fixture case listed in order, e2e; usage contract and contract decision 7 corrected | +| S1 | suggestion | README and skill said a copied branch is always asked again; legacy copies are inherited | both name the cli-v5.5.0 exception | +| S2 | suggestion | forget count test did not include `aiddDeclaredFor` | count test now has four keys | +| S3 | suggestion | spec.md:34 "at most once" unexplained | reading note added under it, pointing to the usage contract | + +## Round 3 + +- Diff: `origin/next...88755b10` +- Verdict: request changes, 6.5/10. Rounds 1 and 2 confirmed resolved; readers agree; M8 decision confirmed. + +| # | Class | Finding | Resolution (6c52b68c, fd94dbc1) | +| --- | --- | --- | --- | +| D1 | deal breaker | after `-C`/`-M` onto a declared branch, or a rename onto a reused name, declaring again did not reach the report: the copied reflog made the old generation look younger (`attribution.ts` `generationAt`) | the branch's start is now read as the newest reflog entry where it took its name (`branchCreatedAt`), else its oldest; two e2e tests red first; contract decision 8, usage contract rename bullet and a reflog-expiry limit | +| S1 | suggestion | forget over-counts duplicated keys after `-C` | kept: forget counts the config entries it removes, so the number is what it deletes | +| S2 | suggestion | evidence counts stale (1225, hook count missing) | re-recorded in `evidence.md` | + +## Round 4 + +- Diff: `origin/next...fd94dbc1` +- Verdict: request changes, 7/10. Rounds 1-3 hold, no regression found in six scenarios run against the built binary. + +| # | Class | Finding | Resolution | +| --- | --- | --- | --- | +| D1 | deal breaker | a rename by a libgit2 client writes `branch: renamed …` in lower case, which `branchCreatedAt` did not read | 9fa7ff9d: subject matched case-insensitively (`/^branch: (renamed\|copied) /i`), red unit test first | +| S1 | suggestion | before a reused name is declared again, the report still gives its calls to the earlier generation while `status` says undeclared | stated as a limit in the usage contract: only calls the gate did not hold can land there | +| S2 | suggestion | `branch-binding-source.ts` port comments stale | reworded | +| S3 | suggestion | `generationAt` comment spoke only of deleted names | now names copies and renames | +| S4 | suggestion | decision 4's reason said only the reflog tells copy from rename, yet the reflog is now read | reason reworded: the stamp cannot tell them apart; a declaration held only while a reflog entry lasts would change meaning on its own | + +## Round 5 + +- Diff: `origin/next...9fa7ff9d` +- Verdict: request changes, 8/10. Rounds 1-4 hold; readers agree; M8 confirmed. + +| # | Class | Finding | Resolution (c7ae6e38) | +| --- | --- | --- | --- | +| D1 | deal breaker | usage contract and decision 4 said a rename always leaves the new name undeclared; an unstamped (cli-v5.5.0) declaration is inherited by a rename too | limit row and decision 4 now say copy or rename | +| S1 | suggestion | dropping no-time snapshots before choosing the generation let an older generation answer for a newer one | generation chosen over every snapshot, then only declared ones speak; red test first | +| S2 | suggestion | task contract limits missed the two added in rounds 3 and 4 | added, pointing to the usage contract | +| S3 | suggestion | round 3 and 4 resolutions named no commit | hashes added | + +## Round 6 + +- Diff: `origin/next...c7ae6e38` +- Verdict: approve, 9/10, no deal breaker. Rounds 1-5 hold; CLI and hook agree on six cases through real git; the flaky run in `evidence.md` judged honestly reported. + +| # | Class | Finding | Resolution | +| --- | --- | --- | --- | +| S1 | suggestion | the hand-edit limit named only "every key"; `aiddTask` and `aiddDeclaredAt` alone, or a removed stamp, also declare | limit widened in the usage contract and the task contract | +| S2 | suggestion | decision 2 said "when present"; an empty stamp counts as absent | "when present and non-empty" | +| S3 | suggestion | "a declared branch is never asked again" without the `-C`/`-M` case | qualified in the usage contract, README, hook header and decision 6 | +| S4 | suggestion | round 5 resolutions named no commit | c7ae6e38 added | +| S5 | suggestion | keep the reporter output of gate runs | noted for the next runs | + +The resolutions are wording only, applied after the approval (no code changed). diff --git a/cli/aidd_docs/memory/telemetry.md b/cli/aidd_docs/memory/telemetry.md index c59350692..ec6413e85 100644 --- a/cli/aidd_docs/memory/telemetry.md +++ b/cli/aidd_docs/memory/telemetry.md @@ -49,6 +49,7 @@ flowchart LR | Why a call is counted once | `domain/usage-fold.ts` | | Where a call's repository comes from | `application/directory-resolver.ts`, `domain/repository-identity.ts` | | Which task a call belongs to | `domain/report/attribution.ts`, `domain/declaration/binding-resolution.ts` | +| Whether a branch is declared: only by `task`, on that branch (`aiddDeclaredAt` set, `aiddDeclaredFor` absent or equal to the branch); the hook's twin is `plugins/aidd-telemetry/hooks/lib/git.cjs` | `domain/branch-binding.ts` `parseBranchConfig`; rule and limits in the [usage contract](../../../aidd_docs/product/usage-contract.md#branch-declaration) | | What counts as consent | `domain/telemetry-consent.ts`, `infrastructure/git-consent-adapter.ts` | | Whether a call is stored | `application/directory-resolver.ts` | | What a clone is | `domain/consent/clone-identity.ts` | diff --git a/cli/src/contexts/telemetry/domain/branch-binding.ts b/cli/src/contexts/telemetry/domain/branch-binding.ts index eadd93fc7..8c6598ef0 100644 --- a/cli/src/contexts/telemetry/domain/branch-binding.ts +++ b/cli/src/contexts/telemetry/domain/branch-binding.ts @@ -17,7 +17,8 @@ export interface BranchConfigBinding { * the work it carried. */ export interface BranchSnapshot extends BranchConfigBinding { readonly repository_id: string; - /** The oldest reflog entry when first read: the work before it was not on this branch. */ + /** When the branch took its name, from its reflog when first read (see `branchCreatedAt`): + * the work before it was not on this branch. */ readonly branch_created_at: string | null; readonly snapshot_at: string; } @@ -31,22 +32,40 @@ const PREFIX = "branch."; const FIELDS = { task: ".aiddtask", ticket: ".aiddticket", + declared_for: ".aidddeclaredfor", declared_at: ".aidddeclaredat", } as const; interface DeclaredFields { task: string | null; ticket: string | null; + /** The branch the command declared on: a copy or a rename moves the keys, not this name. */ + declared_for: string | null; declared_at: string | null; + /** The keys read so far: one read again was left twice by `git branch -C` or `-M`. */ + seen: Set; + listedTwice: boolean; +} + +/** Only the command declares, and it writes `aiddDeclaredAt` last: a key without it was set by + * hand or cut short. A stamp naming another branch was carried here by `git branch -c` or + * `-m`. A key listed twice mixes the declaration copied by `-C` or `-M` with the branch's own: + * neither is whole. A declaration with no stamp was written before stamping existed and still + * counts. */ +function declaredByTheCommand(branch: string, fields: DeclaredFields): boolean { + return ( + !fields.listedTwice && fields.declared_at !== null && (fields.declared_for ?? branch) === branch + ); } function textOrNull(value: string): string | null { return value === "" ? null : value; } -/** Reads `git config --local -z --get-regexp` output: entries end in NUL, a key and its value - * are split by a newline, and a key given with no value has no newline at all. A branch name - * can hold dots, so the known suffix is cut off the end, never split on. */ +/** Reads `git config --local -z --get-regexp` output into the branches the command declared: + * entries end in NUL, a key and its value are split by a newline, and a key given with no value + * has no newline at all. A branch name can hold dots, so the known suffix is cut off the end, + * never split on. */ export function parseBranchConfig(output: string): BranchConfigBinding[] { const branches = new Map(); for (const entry of output.split("\0")) { @@ -55,35 +74,64 @@ export function parseBranchConfig(output: string): BranchConfigBinding[] { const key = newline === -1 ? entry : entry.slice(0, newline); const value = newline === -1 ? "" : entry.slice(newline + 1); const lower = key.toLowerCase(); - for (const [field, suffix] of Object.entries(FIELDS) as [keyof DeclaredFields, string][]) { + for (const [field, suffix] of Object.entries(FIELDS) as [keyof typeof FIELDS, string][]) { if (!lower.endsWith(suffix)) continue; const branch = key.slice(PREFIX.length, key.length - suffix.length); - const held = branches.get(branch) ?? { task: null, ticket: null, declared_at: null }; + const held = branches.get(branch) ?? { + task: null, + ticket: null, + declared_for: null, + declared_at: null, + seen: new Set(), + listedTwice: false, + }; + held.listedTwice ||= held.seen.has(field); + held.seen.add(field); held[field] = textOrNull(value); branches.set(branch, held); } } return [...branches.entries()] + .filter(([branch, fields]) => declaredByTheCommand(branch, fields)) .sort(([a], [b]) => compareText(a, b)) - .map(([branch, fields]) => ({ + .map(([branch, { task, ticket, declared_at }]) => ({ branch, - ...fields, - none: fields.declared_at !== null && fields.task === null, + task, + ticket, + declared_at, + none: task === null, })); } -/** `git reflog show --date=iso-strict --format=%gd` lists newest first, each as - * `refs/heads/@{