Review rounds 2-3 + self-review on --attribution:
Credential egress (round 2):
- normalizeRemoteUrl: scp userinfo expressed as an optional regex group
let backtracking re-parse a credential prefix as host:path
(x-access-token:ghp_...@host/repo -> token in git.repo). Userinfo is
now split off at the first @ BEFORE any host matching.
- Positive validation (allow-list) as the final gate on EVERY branch:
host must be hostname-shaped, every path segment repo-shaped, total
identity <= 200 chars. Kills transport-helper remotes (ext:: leaks
local SSH key paths, codecommit:: leaks AWS profile names), residual
@, spaces/colons, and unbounded strings.
- sanitizePrLinks: links are rebuilt from origin + pathname — userinfo,
query strings, and fragments are dropped instead of passed through;
collapsed duplicates dedupe.
Attribution correctness (round 3 + self-review):
- Double-count fix with precise retraction semantics: when a commit
migrates to a later-parsed tighter-window session, the loser re-emits
git.commit_count=0. Empty records are emitted ONLY on a true loss in
THIS computation (lostCandidacy) — a commit that merely aged out of
the --since range was lost to nobody, and retracting it would
permanently zero a still-correct server-side count. The sync layer
additionally requires a prior ledgered state for the session.
- Session dedup key includes project + both window timestamps, so
ongoing sessions re-emit with corrected span times.
- Span end times clamped like the usage builder (never 0, never
earlier than start + 1ms).
- CLI mirrors the usage path on attribution push failures instead of
claiming success.
- Identity normalization: case-insensitive .git strip, doubled path
slashes collapse.
AI-Origin: human
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
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
- Catch errors in 'sync setup' action: AuthError/DiscoveryError (login
timeout, port exhaustion, discovery failures) now print a clean one-line
message and exit 1 instead of crashing Node with an unhandled rejection
stack trace. Also guard the double rejection when all callback ports are
in use (both 'ready' and the callback promise reject).
- Windows: open the auth URL via 'rundll32 url.dll,FileProtocolHandler'
instead of 'cmd /c start'. cmd.exe splits its command line on '&', which
truncated the OAuth query string at the first parameter, so the IdP
received a bare /authorize request ('no parameters'). rundll32 receives
the URL as a plain process argument with no shell parsing.
- Theme the OAuth callback landing pages to match the dashboard (warm
paper background, ink text, forest-green success / terracotta failure),
fully inline with no network or asset dependencies.
AI-Origin: human
Before-merge items:
1. HTTPS enforced on every remote endpoint (RFC 8252 §8.3): baseUrl,
issuer, authorization/token/revocation endpoints, and the traces
endpoint all reject non-https, with a loopback (127.0.0.1/::1/
localhost) exception for offline tests and local dev. Enforcement is
central (assertHttps) — the browser-open guard is no longer the only
check whose failure was swallowed.
2. Credential store test isolation: CODEBURN_SYNC_TOKEN_STORE=file
forces the file store (honors HOME) so the offline suite never
touches the real macOS login keychain. Set in the e2e suite.
3. Golden pins for deriveSpanId/deriveTraceId/deriveDeviceId with fixed
inputs and expected hex — the idempotency contract depends on these
encodings being stable across releases; a green-tests encoding change
would silently double-count history on span-ID-keyed backends.
getDeviceId refactored over a pure deriveDeviceId(host, user).
Smaller review items:
- Callback server: ready promise resolves the actually-bound port from
the listening event (kills the 100ms-sleep race after port fallback);
Connection: close on all responses + closeAllConnections() on
shutdown (pooled keep-alive sockets from a closed server could
swallow requests aimed at a later server on the same port); error
handler guarded so a post-bind error can never rebind to a different
port than advertised; optional ports param ([0] = ephemeral) removes
fixed-port contention between parallel test workers.
- fetchOidcConfig verifies the issuer claim matches the fetch origin
(OIDC Discovery §4.3 mix-up defense).
- partialSuccess.rejectedSpans wrapped in Number() — proto3 int64 JSON
mapping sends strings from strict protojson servers; += would
concatenate.
- Ledger writes are atomic (temp + rename); corrupt-ledger recovery and
no-tmp-left-behind tests added; XDG_CACHE_HOME honored (ledger is
reconstructible state, not config).
- Mock IdP now implements /oauth2/authorize (registers PKCE challenge,
302s to redirect_uri) and verifies S256 code_verifier + single-use
codes at the token endpoint. The e2e drives the real redirect flow
and asserts wrong-verifier and code-reuse are rejected — PKCE binding
is now exercised end to end.
- sync reset calls clearLedger() instead of reimplementing the path.
- push sets exit code 1 on rate-limited/server-error outcomes so cron
and script callers can detect incomplete pushes.
Deferred (noted for fast-follow): macOS 'security -i' stdin mode
(untestable on this Linux box), ai.cost_estimated as a real
ParsedApiCall flag (touches core parser types).
Sync suite: 81 passing (5x stable), 5 developer-only.
AI-Origin: human
Adds an opt-in `codeburn sync` command group: setup, push, status,
logout, reset. Developers authenticate via standard OIDC browser login
(Authorization Code + PKCE, provider-agnostic — Cognito, Auth0, Okta,
Keycloak, etc.) and push per-call usage metadata to a team-operated
OTLP/HTTP endpoint.
Privacy is structural: the payload builder has no field that can carry
prompts, responses, file contents, or bash commands. Only metadata
leaves the machine (provider, model, tokens, cost, project, tool names,
timestamps). There is deliberately no flag to include more.
Setup derives everything from a single URL:
GET {base}/.well-known/codeburn-export.json
→ issuer, client_id, traces_path, max_batch_size
Modules:
- discovery.ts discovery doc fetch/validation (version-gated)
- auth.ts OIDC discovery, PKCE, localhost callback (:19876-78),
token exchange/refresh/revoke
- credentials.ts refresh token in OS keychain (macOS security / Linux
libsecret / Windows DPAPI / 0600 file fallback). All
subprocess calls use execFileSync arg arrays; tokens
flow via stdin or env var — never shell-interpolated
- config.ts ~/.config/codeburn/sync.json
- ledger.ts client-side sent-ledger for exact dedup (no watermark
races), 6-month prune
- otlp.ts ParsedApiCall[] → ExportTraceServiceRequest JSON with
deterministic IDs: span=SHA256(dedupKey)[:8],
trace=SHA256(sessionId)[:16] — every retry idempotent
- push.ts orchestration with typed outcomes; pushes run to
completion — 429 Retry-After honored (≤120s/wait,
3 retries/batch) before deferring; partial_success is
batch-atomic (nothing ledgered, whole batch retries);
50K safety valve, no routine cap
- cli.ts command layer; --dry-run reports exact counts/cost
without sending
Preview feature: protocol may change between releases.
AI-Origin: human