5 commits

Author	SHA1	Message	Date
iamtoruk	dbeae9b48c	fix(sync,parser): review B — symmetric shape freeze, honest settle, stable residual keys ... B-1 The freeze was one-directional and left the reverse hole wide open. Rows sync; at the 90-day durable age-out the cached rows are pruned, the rollup stops being dropped and serves again under a key that was never sent; the session is long past settle, so it pushes an aggregate on top of the per-request spans the receiver already holds. Reframed around the thing that actually matters: a copilot session's input/cache leaves this machine in one of two SHAPES — the raw rollup (`copilot:<sid>:shutdown:`), or reconciled output (rows plus `:shutdown-residual:`, which are disjoint by construction and together are exactly the rollup re-expressed). The receiver must never hold both. Whichever shape a session was first synced in, it stays in, and the other is frozen for that session permanently — in both directions. Growth WITHIN the sent shape is untouched, because same-shape output is additive, never substitutive: a resumed session's new rows and residuals still push if rows were sent, a new leg's rollup still pushes if rollups were. This also corrects the previous commit, which classed a residual as aggregate and so froze it for a session whose rows had gone out — the residual is reconciled output and belongs with the rows. B-2 A session with an unparseable timestamp was held forever while the CLI promised it would "push once it settles". Nothing could ever settle it. Settle now reads the newest moment a session can be SHOWN active; a session with nothing datable at all is sent rather than held. Mixed sessions still settle on their datable stamps. B-3 A stamp implausibly far in the future is broken data, not evidence the session is live, so it no longer counts toward that newest moment. One year-ahead row can no longer hold a month-old session hostage. Ordinary clock skew is absorbed by a one-hour grace in both directions and still reads as live. B-4 The residual dedup key carried the leg's POSITION. Legs sort across every cached file for a session, so an earlier leg arriving later renumbers every residual after it — and a renamed key is a span the receiver takes a second time, since there is no retraction for a usage span. Keyed by the leg's own instant instead: append-only files mean it never moves, and the equal-timestamp coalescing above makes it unique per leg. Residuals are new in this change and have never shipped, so no CACHE_VERSION concern. B-5 The dry-run "already synced" count now subtracts frozen too, matching the nothing-to-push line. Tests: symmetric freeze from the rows side (rollup frozen, residual and new rows still sent, per-turn untouched); all-unparseable sent, partly-unparseable still held; a year-ahead stamp ignored while a one-minute skew still holds; residual keys named after their leg instant with the positional names gone. The key-prefix pin picks up the residual's new tail. (c2) pins the property rather than one insertion scenario, because a second file's leg currently collides on the rollup's own dedup key before it can reach the residual sweep — the reachable repro would prove nothing about the next one, and that is stated at the test. Flakes: (f2) and (sc) re-bucket each run 10x isolated and 10x with four busy loops pinning cores — 40/40 clean. Neither has clock or ordering dependence at its margins: afterEach removes both tmpHome and the cache dir, and the two relative timestamps in the re-bucket test are 35 days apart so they cannot share a month. The likeliest cause of the transient failures is this branch's own commits rewriting src/parser.ts under a concurrent vitest. (f2) gains a self-check that its fingerprint sentinel really differs from the computed one, which is the one way it could have passed while exercising nothing.	2026-08-19 13:25:15 -07:00
iamtoruk	9ebd509c44	fix(sync,cache,ci): review A — forward-only sync, a guard that bites, call-level union ... F1 — the upgrade-path guard was measuring the wrong layer. providerDetails come from the menubar payload, and on a version bump adoptOlderDailyCaches carries the superseded daily file forward as the baseline for exactly the days this scenario creates, so a session-cache loss was mostly masked: reverting the carry-forward showed a 10% dip where the real loss is 50%. It now sums export.json's per-call copilot records (straight out of the parse), deletes the baseline daily cache before the upgrade run so nothing else can answer, and asserts EQUALITY rather than "did not shrink" — the failure mode on the other side of the fix is a double. Proven: reverting the carry-forward now reports 80 calls -> 40, LOST history, and fails. (The double direction is asserted but not demonstrated; a faithful re-keying simulation is collapsed by serve-time dedup before it can be counted, which is noted at the step.) F2 — sync was forward-only in the wrong direction. A receiver that took `copilot:<sid>:shutdown:<model>:<n>` rollup spans from 0.9.20 keeps them forever (append-once, no retraction for a usage span) and would now also receive `copilot-store:` rows for the same sessions: permanent doubling on every synced receiver, on the first push after upgrade. Sessions whose rollup is already in the ledger are now FROZEN — their rows and residuals never go out, and the receiver keeps the older, lossier number. A bounded under-count in place of an unbounded over-count. Per-turn output spans are untouched either way (the rollup never held output, reconciliation never touches them), and sessions new to sync take the per-row path in full. `frozen` is reported by --dry-run and the nothing-to-push line; docs/sync/README.md gains an FAQ pointing at `codeburn sync reset --confirm` with the caveat that the receiver must be cleared too. F3 — docs/providers/copilot.md gains a Sharp edges section saying plainly that a day sealed on a short store snapshot is a permanent UNDER-REPORT the watermark advances past, not a stall, and that both realizable causes are now closed (read ordering, compaction anchor). F4 — the copilot dedup-key shapes are pinned by reading them back out of the source, minted keys and discriminator prefixes alike, with the reason: the durable union recognises history by key, so a changed prefix is a CACHE_VERSION bump, not a parse-version bump. F5 — the carry-forward comment now states its price: a bump no longer re-derives a call it carried forward, so cost-shaped fixes land (cost is recomputed from cached usage) but metadata and day-attribution fixes reach only newly parsed calls. F6 — the durable union filters at CALL level. A turn holding one cached call beside a new one used to be dropped whole; nothing enforced the one-call- per-turn assumption that made that safe. F7 — the retain-source fence no longer skips a store it cannot fingerprint. Present-but-unreadable defers, matching the classification path; only a genuinely absent one stays a skip.	2026-08-19 13:12:18 -07:00
iamtoruk	fb1bde94b7	fix(sync): hold a copilot session until its reconciliation can no longer change ... The sent-ledger is append-once and the OTLP span id derives from the same deduplication key, so the pipeline assumes a served call is immutable: same key, same value, forever. Copilot's serve-time reconciliation is the first producer that breaks that (#988), three ways inside one session — a shutdown residual shrinks as the store rows covering it land, a rollup is dropped once rows cover its leg, and an unpaired row becomes supplementary when its journal call appears. Sent at an intermediate state, the receiver keeps that state forever AND receives what supersedes it. Local reports re-reconcile every pass and were never affected; this is only about what leaves the machine. Value-versioned keys were the other option, but usage spans have no upsert contract — receivers sum them, so a superseding span adds rather than replaces. (Attribution can do it because its spans are documented as upserts by `ai.session_id`.) Giving usage spans one is a receiver protocol change. So: hold, do not re-key. Every input to the reconciliation is written during the session, so a session quiet for 24 hours cannot reconcile further and its first send is also its last word. The whole session is held, not just its residual — holding the residual alone still ships a row whose pairing can flip. Nothing is dropped: `held` is reported by --dry-run and by the nothing-to-push line, and the next push after the window sends it. Scoped by provider because copilot is the only reconciling producer; every other provider's calls go out unchanged and undelayed.	2026-08-19 12:41:40 -07:00
Andrew Lee	d8a7b2a95f	fix(sync): address PR review — https enforcement, keychain test isolation, golden ID pins ... 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	2026-07-13 16:45:39 +00:00
Andrew Lee	163842edac	test(sync): unit, mock-IdP e2e, and developer-only integration tests ... Offline, CI-safe (70 tests): - sync.test.ts (26): discovery parsing, PKCE, auth URL, scope negotiation, callback server (state mismatch, IdP error, timeout, port fallback), config round-trip - sync-ledger-otlp.test.ts (23): deterministic ID derivation, OTLP structure/attributes, batching, ledger append/dedup/prune/clear - sync-push.test.ts (15): pipeline against a scriptable mock OTLP server — success+ledger+cost, partial-success batch NOT ledgered, 401 stop, 429 wait-and-retry / persistent-429 give-up / Retry-After parsing (delta-seconds, HTTP-date, garbage), 5xx deferral, idempotent failure recovery - sync-e2e.test.ts (6) + fixtures/mock-idp.ts: full setup→refresh→ rotate→logout round-trip against an in-process mock IdP Developer-only (skip unless env vars set, never CI): - sync-headless-e2e.test.ts (1): real browser PKCE flow via Playwright - sync-infra-e2e.test.ts (4): push/auth-reject/batch/idempotent re-push against a deployed backend AI-Origin: human	2026-07-12 16:05:34 +00:00