Skip to content

feat(sync): opt-in git attribution spans on sync push --attribution - #848

Open
Enclavet wants to merge 3 commits into
getagentseal:mainfrom
Enclavet:feat/sync-yield-attribution
Open

feat(sync): opt-in git attribution spans on sync push --attribution#848
Enclavet wants to merge 3 commits into
getagentseal:mainfrom
Enclavet:feat/sync-yield-attribution

Conversation

@Enclavet

Copy link
Copy Markdown

What

Adds an opt-in --attribution flag to codeburn sync push that sends the session→commit correlation codeburn yield already computes locally, so a telemetry backend can join AI usage to git outcomes (merged / never merged / reverted) — without teams having to install per-developer git hooks.

codeburn sync push --attribution
codeburn sync push --attribution --dry-run   # counts only, sends nothing

Why

codeburn sync today answers how much AI was used (tokens, cost, model, project). It cannot answer where that AI output landed: which commits came out of a session, whether they shipped to main, or whether they were later reverted. Teams that want outcome metrics (AI-to-merge ratio, defect/revert rates on AI-assisted code) currently have to bolt on commit-trailer git hooks — a per-developer install with its own drift and maintenance burden.

But codeburn already has the hard part. codeburn yield resolves each session's project to a canonical repo (monorepo subdirs and worktrees collapse via git-common-dir), attributes commit SHAs to sessions by tightest timestamp window, and computes inMain / wasReverted per commit. That analysis just never left the local CLI report. This PR exposes it through the existing sync channel.

What gets sent (only with the flag)

Two new span types, sharing the session's traceId with the existing usage spans (deriveTraceId(sessionId)), so receivers correlate cost and attribution with no extra key:

codeburn.session.attribution — one per session with joinable evidence:

Attribute Example
ai.session_id abc123…
ai.project my-app
git.repo github.com/acme/widget (normalized origin remote)
git.pr_links ["https://github.com/acme/widget/pull/12"]
git.commit_count 2

The span's start/end times are the session window itself.

codeburn.commit — one per commit attributed to a session:

Attribute Example
git.sha 4f2a…
git.in_main true
git.was_reverted false
git.repo, ai.session_id, ai.project (join keys)

A resource attribute codeburn.attribution_methodology: timestamp-window marks the attribution as inferred (same self-declared heuristic as codeburn yield), so backends can label it honestly versus declared sources like commit trailers.

Design decisions

  • Remote normalization happens client-side. git remote get-url origin is normalized to host/org/repo: scp-like (git@host:org/repo.git), ssh:// (user + port dropped), and https:// (embedded credentials stripped — a token in an https remote must never leave the machine) all collapse to the same key. Local-only repos and file:// remotes have no server-side identity: their commits are never sent. A session in such a repo still emits a record when it carries PR links (the PR URL embeds the repo).
  • State-encoding dedup keys make updates flow through the existing ledger. A commit's key encodes inMain/wasReverted (and the session key hashes repo + PR links + commit states). Identical facts dedupe via the sent-ledger exactly like usage spans; a state transition (commit merges to main, or gets reverted) mints a new key and the updated fact is re-sent on the next push. Receivers should upsert by (git.repo, git.sha).
  • Attribution rides after the usage push, same endpoint, same auth, same 429/partial-success handling. It is skipped when the usage push hit rate limits or server errors (both retry next push). Zero-cost items don't inflate the $ summary; a separate Attribution: N facts synced line is printed.
  • No new privacy defaults. Without --attribution, behavior is byte-identical to today. With it, what leaves the machine is: normalized repo remote, commit SHAs + timestamps, PR URLs, and the merged/reverted booleans. Never code, diffs, paths, prompts, or bash commands. Documented in docs/sync/README.md.

Implementation

  • src/yield.ts: the repo-grouping loop inside computeYield is extracted into buildRepoGroups (verbatim; grouping semantics unchanged) and shared with a new computeAttributionRecords(projects, range, cwd). Also exports normalizeRemoteUrl. computeAttributionRecords takes already-parsed projects, so sync push doesn't re-parse.
  • src/sync/otlp.ts: flattenAttributionRecords, dedup key builders, buildAttributionOtlpPayload, batchAttributionItems.
  • src/sync/push.ts: the send loop is generalized into sendBatchesCore (the public sendBatches signature and behavior are unchanged); adds collectUnsentAttribution + sendAttributionBatches.
  • src/sync/cli.ts: the --attribution flag, dry-run reporting, and summary output.

Testing

  • 16 new tests (tests/sync-attribution.test.ts): remote normalization (ssh/scp/https/credentials/local), record computation against real temp git repos (remote join, tightest-window ownership, no-remote handling, empty-session omission), dedup key state transitions, OTLP span shape, and the send+ledger pipeline against a mock OTLP server (success ledgering, 5xx not ledgered).
  • All existing yield/sync suites pass unchanged (93 tests across yield*.test.ts, sync*.test.ts) — computeYield behavior and the sendBatches contract are untouched.
  • Manual E2E: scratch repo with a real history (merged commit, never-merged branch commit, git reverted commit) pushed through the full pipeline to a live local HTTP server. Verified: correct in_main/was_reverted flags from real git, credential stripped from an https://user:token@… remote with a full-payload scan, idempotent re-collect after ledgering, and correct 2-fact re-send after merging the feature branch (state transition).

Compatibility

  • Opt-in flag; no change to default sync push, payload shape, discovery doc, or auth.
  • Receivers that don't recognize the new span names can ignore them (they're ordinary OTLP spans). Strict receivers that reject unknown spans would surface via the existing partial-success path; such teams simply don't pass the flag.
  • tsc clean; no new dependencies.

Caveats (by design)

  • Attribution is heuristic. Timestamp-window correlation can mis-assign a commit when a human commits unrelated work mid-session in the same repo — the same tradeoff codeburn yield documents. The methodology resource attribute exists so dashboards can label inferred vs. declared attribution.
  • The wasReverted signal only detects standard git revert bodies ("This reverts commit <sha>"), matching yield's existing detection.

Possible follow-ups (not in this PR)

  • A sync.attribution: true config option so scheduled pushes don't need the flag.
  • Fork-workflow refinement: prefer the PR-link repo identity over origin when both exist (origin may point at a fork).

Expose the yield session-to-commit correlation through codeburn sync so
backends can join AI usage to git activity without local git hooks.

- yield: export normalizeRemoteUrl (host/org/repo; credentials, ports,
  and .git stripped) and computeAttributionRecords, which reuses the
  exact repo-grouping + tightest-window attribution from computeYield
  (extracted into a shared buildRepoGroups) and joins in the normalized
  origin remote and session prLinks.
- otlp: two new span types sharing the session traceId —
  codeburn.session.attribution (git.repo, git.pr_links, git.commit_count)
  and codeburn.commit (git.sha, git.in_main, git.was_reverted). Resource
  attribute codeburn.attribution_methodology=timestamp-window marks the
  attribution as inferred.
- push: generic send core reused by usage and attribution batches. Dedup
  keys encode mutable state (inMain/wasReverted), so a state transition
  re-sends the updated fact while identical states dedupe via the
  existing sent-ledger.
- cli: opt-in --attribution flag on sync push (dry-run aware); commits
  in repos with no network remote are never sent.

AI-Origin: human
@Enclavet

Copy link
Copy Markdown
Author

Intentionally made this opt-in. As the remote repo name and pr-links might be sensitive.

@iamtoruk iamtoruk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the core of this is strong work, and the review went deep because this is the product's most privacy-sensitive surface. What held up very well under adversarial testing: the credential stripping survived 48 hostile remote forms (userinfo tokens, GitLab subgroups, scp-forms with colons in usernames, query-string tokens — zero leaks, no ReDoS), --dry-run is provably silent on the wire (zero /v1/traces POSTs against a live collector), flag-off behavior is byte-identical to main, the yield.ts extraction is behavior-preserving across 4,186 real sessions, and reusing the #660 plumbing (backoff, assertHttps, ledger) was exactly right.

Two egress bugs block the merge — both demonstrated end-to-end against a live mock collector:

1. The cwd fallback egresses unrelated (possibly confidential) repos. buildRepoGroups does identity = projectIdentity ?? cwdIdentity (src/yield.ts:390-392, 574). When a session's project path no longer resolves to a work tree (deleted/renamed dir, or sessions run outside a repo), attribution inherits whatever repo the user's shell is in at push time. Reproduced: pushing from inside a private repo emitted repo: "github.com/secret-org/nda-client-repo" plus its commit SHAs, attributed to a session that never touched that repo. In local yield this fallback is a harmless heuristic; on the sync path it's both a privacy leak and false attribution. Suggested fix: on the attribution path, drop the group (or at minimum repo + commits) whenever identity came from the cwd fallback rather than the project's own path.

2. Windows drive-letter paths defeat the "local repos are never sent" guarantee. The scp-like branch matches C: as a host (src/yield.ts:159-160): "C:/Users/alice/private/repo"repo: "c/Users/alice/private/repo", and because repo is non-null, commits are sent — directly contradicting the doc line "Commits in repos with no network remote are never sent," with the user's local filesystem path as the emitted identity. Suggested fix: reject single-character hosts, or match /^[a-zA-Z]:[\\/]/ before the scp-like branch.

3. Tests for the above. The new test file is genuinely good (live collector, wire assertions), but it covers only 3 basic normalize cases — none of the adversarial forms, no Windows path, no --dry-run no-network assertion, no flag-off assertion. All the findings above would have been caught by that corpus.

Should-fix (non-blocking but please consider in the same pass):

  • git.pr_links from host-emitted journal entries passes only a truthy-string check (src/parser.ts:1265 upstream) — arbitrary strings of arbitrary length reach the endpoint. A shape check (URL, allowlisted hosts) + per-session cap would close it. Relatedly, PR links are sent even when repo is null (deliberate and defensible, but the docs' "never sent" sentence reads more absolute than reality — worth reconciling).
  • Attribution items have no MAX_PER_PUSH-style cap; a first --since all --attribution push on a long history has no valve.
  • A CHANGELOG ## Unreleased entry — a privacy-relevant opt-in flag belongs in release notes.
  • The PR body's attribute table under-declares vs the wire: codeburn.device_id, codeburn.attribution_methodology, and commit timestamps (span start times) also ride along. Your docs/sync/README.md discloses the timestamps honestly — the PR body should match it.

Nice-to-have: case-insensitive .git strip and slash collapse for join-key stability (…/Repo.GIT and doubled slashes currently split one repo into two keys); document that only origin is read.

Happy to re-review promptly — the shape of this feature is right and the plumbing reuse makes it an easy yes once the egress edges are closed.

…paths, PR-link validation

Review findings on the --attribution PR:

- Privacy: sessions whose project path no longer resolves inherited the
  cwd-fallback repo identity, egressing whatever (possibly confidential)
  repo the user pushes from and falsely attributing its commits.
  buildRepoGroups now tracks per-session identity provenance; the
  attribution path excludes fallback sessions from commit attribution
  entirely (no repo, no commits, PR links only) — they also can no
  longer steal a commit from a genuine session's window.
- Privacy: Windows drive-letter paths (C:/..., C:\..., drive-relative)
  parsed as scp-like remotes, emitting local filesystem paths as repo
  identities. normalizeRemoteUrl rejects drive letters and
  single-character hosts (dotless intranet hosts still accepted).
- Hardening: PR links are shape-checked before sending (https,
  /org/repo/pull/N path, <=256 chars, max 20 per session) — upstream
  parsers only truthiness-check them.
- Safety valve: MAX_ATTRIBUTION_PER_PUSH (10k) caps a first
  --since all --attribution push; dry-run reports the cap.
- Tests: adversarial normalize corpus, cwd-fallback egress repro,
  commit-stealing prevention, PR-link sanitization, and CLI-level tests
  (mock IdP + collector): dry-run sends nothing to the traces endpoint,
  flag-off emits no attribution span names on the wire.
- Docs: reconciled the 'never sent' wording with reality (PR links ride
  even when repo is null; device_id/methodology/timestamps disclosed).
  CHANGELOG Unreleased entry added.

AI-Origin: human
@Enclavet

Copy link
Copy Markdown
Author

Thanks for the thorough review — the two privacy findings were real, and both are now fixed in ce800d8, along with all the should-fixes. Point by point:

1. cwd-fallback egress — fixed at the source. buildRepoGroups now tracks per-session identity provenance (ownIdentity[], parallel to sessions[]): true only when the session's own project path resolved to the repo. On the attribution path, fallback sessions get no repo and no commits — they only emit a record if they carry PR links, which are session-native and safe. Your repro is now a test (never egresses the cwd repo for sessions whose project path did not resolve), asserting the private repo identity appears only on the genuinely-cwd session's record.

One consequence worth calling out: I went further than dropping repo+commits. Fallback sessions are excluded from the tightest-window competition entirely — otherwise a fallback session with a tighter window could still steal a commit from the genuine session that owns it (the genuine session would then read as abandoned, a false negative your suggested fix wouldn't have caught). That's also tested (fallback sessions cannot steal a commit from a genuine session). codeburn yield behavior is unchanged — the fallback remains a harmless local heuristic there.

2. Windows drive letters — fixed. normalizeRemoteUrl now rejects ^[a-zA-Z]:[\\/] before the scp-like branch, and the scp host class requires ≥2 chars (catches drive-relative C:repo) and excludes @ — while writing the tests I found git@C:/foo could backtrack into matching host git@C, so that hole is closed too. Dotless intranet hosts (gitserver:team/repo.git) still normalize. Covered: forward/backslash drive paths, lowercase drives, drive-relative, single-char hosts.

3. Tests. The corpus you asked for is in:

  • Adversarial normalize forms (both must-fix classes would now fail CI)
  • The cwd-fallback egress repro + commit-stealing prevention (real temp git repos)
  • CLI-level tests (tests/sync-attribution-cli.test.ts) driving the real commander action against the mock IdP, which I extended with a /v1/traces collector: --dry-run --attribution results in zero traces POSTs; push without the flag emits no attribution span names and no git.sha anywhere on the wire; push with the flag emits usage + attribution spans.

Should-fixes — all taken:

  • PR link validation: new sanitizePrLinks — https only, pathname must match /org/repo/pull/N, ≤256 chars, capped at 20/session. I chose path-shape over a host allowlist so GitHub Enterprise hosts keep working; happy to tighten to an allowlist if you'd prefer.
  • Docs reconciled: the absolute "never sent" sentence in docs/sync/README.md is replaced with an explicit sent/not-sent list, including that PR links ride even when the repo is null (with the rationale: the URL names the repo the session already recorded).
  • Cap: MAX_ATTRIBUTION_PER_PUSH = 10_000 mirrors MAX_PER_PUSH; both the real push and --dry-run report when it engages, and the ledger resumes on the next push.
  • CHANGELOG: ## Unreleased### Added entry with the privacy guarantees summarized.
  • PR body: updated to disclose codeburn.device_id, codeburn.attribution_methodology, and commit timestamps (span start times), matching the docs.

Full suite: tsc clean, 118 tests green across the yield/sync/attribution suites (25 new in this PR).

@Enclavet
Enclavet requested a review from iamtoruk July 28, 2026 23:31
@Enclavet

Enclavet commented Aug 1, 2026

Copy link
Copy Markdown
Author

@iamtoruk Can you take a look at this PR when you get a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants