Three review follow-ups.
1. The rotation guarantee is now pinned by a real parse, not by two unit
tests that both stub the fingerprint. The original defect was a wiring
bug — right value, wrong selector — and a stubbed fingerprint cannot see
a wiring bug. The new case warms the cache under key K1 (one JetBrains
turn, one call), rotates the key file to K2, appends a second turn, and
re-parses in a fresh module registry: exactly two calls, no K1-era dedup
key anywhere, on disk or in the result. Restore the substring-sniff
selector and it reports three calls for two turns — the user-visible
shape of the bug.
2. The test-helper guard compared UNRESOLVED paths, so a symlink at a
sandbox-shaped location (HOME=$TMPDIR/codeburn-test-env-x -> /Users/me)
passed the prefix test and the write landed in the real home anyway. Both
sides are realpath'd before comparing, which also stops a sibling
directory sharing the tmpdir prefix from matching. Same refusal message.
3. CHANGELOG records the two API breaks that were missing: decodeCopilot
throws on an empty privacyKey for JetBrains records, and
SOURCE_REF_KEYED_PARSE_VERSION is no longer exported (KEY_DERIVED_PROVIDERS
replaces it).
fixed-privacy-key.ts writes <home>/.config/codeburn/privacy-key
unconditionally. Under vitest that is the throwaway sandbox
env-isolation.ts mints, and harmless. Imported from anywhere else — tsx,
a stray node -e, a REPL — homedir() is the developer's real home and the
write silently replaces their key, re-keying every resource fingerprint
and orphaning whatever was already synced. privacy-key.ts refuses to
cause that on its own; a test helper must not cause it either.
The sandbox has a recognizable shape (a mkdtemp dir under tmpdir() named
codeburn-test-env-*), so the guard needs no new marker: anything else
throws with the reason.
createHmac accepts '' happily and returns a digest anyone can recompute
from the reply text — exactly the weakness the HMAC replaced. Every
other keyed primitive in the codebase throws instead of degrading
(fingerprint.ts's hmac, sanitizeDetail); this one now matches, with the
same message.
It called raw.trim() unconditionally. Two decoders resolve their model
through a plain-object lookup — warp's modelAliases[model],
antigravity's modelMap[usage.model] — so a session whose model names a
prototype member ('__proto__', 'constructor', 'toString', 'valueOf')
gets an object or a function back off the prototype chain, and .trim()
on that is a TypeError that takes the whole parse down with one hostile
record.
That is a regression against what this replaced: `z.string()` rejected a
non-string gracefully. Anything that is not a string is simply not an
identifier, so it collapses to 'unknown' like every other
non-conforming value.
Guarded at the shared boundary rather than at the two lookups: all 34
producers route through here, so one guard covers the sites that exist
and any lookup added later.
Tests drive the four reachable prototype names end to end through the
real warp decoder (non-vacuously: the decode must still produce calls),
plus the unit-level statement for every type a lookup can return.
computeEnvFingerprint decided which providers re-parse on a privacy-key
change by sniffing 'source-ref-fingerprint-v1' out of the parse-version
string. That silently missed copilot. Copilot's dedup keys are just as
key-derived — createHmac(privacyKey) over the JetBrains reply text — but
its parse version reads '…-dedup-key-hmac-v1', so its fingerprint held
still across a key rotation.
Copilot is the sole DURABLE provider: its union-merge never deletes
cached turns, it appends any turn whose key is not already cached. A
fingerprint that does not move therefore does not merely re-parse — it
keeps the old key AND appends the new one for the same turn. Two real
turns became three calls. And a corrupt or unreadable key file yields a
fresh ephemeral key in EVERY process, so that inflation compounded on
every run.
KEY_DERIVED_PROVIDERS lists the seven explicitly. A provider is in the
set because of what its decoder does, which no substring of a version
label can know. The fold also moves out from under `if (parseVersion)`:
the key dependency comes from the decoder, not from having a parse
version entry.
The bridge comment claimed the fold covered "the affected providers" and
that the ephemeral-key fallback only happens when the cache cannot
persist. Both were false — the config dir (~/.config/codeburn) and the
cache dir (~/.cache/codeburn, or CODEBURN_CACHE_DIR) are different,
separately overridable directories, so a writable cache plus an
unreadable key file means a new key every process. Corrected.
Tests: the rotation pair now covers all seven key-derived providers and
five unaffected ones, plus a dedicated copilot case that pins the exact
regression (parse version has no source-ref token, provider is durable,
fingerprint must still move).
Maintainer call: the diagnostics/observation breaking changes are
documented in the CHANGELOG now, but the version number moves at the
next actual publish (core before cli), not on the branch. The 0.x bump
owed for the core API break is recorded in the CHANGELOG heading.
Conflict resolution rules applied:
- packages/cli/src/daily-cache.ts (the only textual conflict): kept the BASE
side of the version constants and their coordination comment verbatim
(DAILY_CACHE_VERSION / MIN_SUPPORTED_VERSION stay at 26 — this change moves
when the watermark advances, not what any cached day contains, so no
re-derive is owed). Kept the BASE widened re-derive window
(DAILY_CACHE_RETENTION_DAYS, with its straddle/tz comment) and layered this
branch's completeness logic on top of it: capture the parse result
(freshProjects) so sessionComplete() can read the tag off the exact array,
priorWatermark hold-back on a partial parse, complete: parseWasComplete, and
the watermarkTrusted stamp.
- packages/cli/src/parser.ts: auto-merged; #929's per-call slicing and #930's
discovery changes are intact, and this branch's read-only-path additions
(readOnlyServedStale on the stale/missing-cache-entry arms, network providers
served from cache instead of re-fetched, hydration completeness tagged onto
the result array) attach to the post-#929 code paths. Re-read end to end
after the merge.
- packages/cli/tests/cache-refresh-lock*.test.ts, vitest.config.ts,
package.json: kept the base config values (testTimeout 30s, retry 2, the
lock-quarantine test/test:locks scripts, #1068's inode fix, #921's retries)
and added this branch's corrupt-lock arms on top. The new describe's retry
is 3, not 6, per review. Added the new corrupt-body file to test:locks:
the base `test` script excludes tests/cache-refresh-lock*, so the new file
would otherwise run in no suite.
- Everything else auto-merged as a union; no hunk from either side dropped.
Resolves daily-cache version coordination: base landed v25 (#930); this
PR's kiro chat-file fix takes the next number, v26, since it changes
historical kiro chat-file numbers and needs its own one-time
re-derivation. Session-cache kiro parse fingerprint bump (v1 -> v2) is
kept alongside base's opencode fingerprint addition. CHANGELOG entries
combined (keep-both), with the kiro entry's cache-version reference
updated to v26.
@codeburn/core takes a minor bump under 0.x semver: the diagnostics API
is a breaking change (DIAGNOSTIC_DETAIL_MAX deleted, sanitizeDetail
signature, DiagnosticDetail retightened) and observation-0.2.0.json is
tightened in place, so an archived pre-hardening envelope whose model
held a display name no longer validates against that same version string.
check-workspace-versions requires root, cli and core to move together, so
all three (and package-lock.json) go to 0.10.0. Publish order at the next
publish is core before cli, as always.
The smuggling guardrail asserted a planted prompt DID survive into the
envelope ("identifier-exemption convention") — an inverted assertion
certifying the hole as correct. It is gone. In its place: the model field
is asserted clean, a positive control proves a legitimate identifier
still crosses unchanged, and every hostile case asserts non-vacuously
(the decode really produced calls) before asserting secret-freedom.
Where a secret still ships, it says so instead of hiding it.
`serializeWithoutDedupKeys` excludes exactly one field, names why
(dedup-key hashing is the schema-wide follow-up), and is used only where
a planted value rides a key component — so the exclusion is visible in
the diff rather than buried in a passing assertion.
New end-to-end coverage for the six source-ref providers: the raw path
appears nowhere in the envelope and each key matches its fingerprint
shape.
import-smoke additionally calls one representative parser, decoder and
detector body under the blocked-I/O preload, and the banned-module sanity
check now covers every entry in the blocklist rather than fs alone. Its
docstring states the scope honestly: import purity for every subpath,
call-time purity for three representatives — a spot check, not a sweep.
The architecture gate walks the published schemas structurally, so a
loosened constraint fails the gate instead of passing unnoticed, and its
synthetic self-test proves the walker can actually detect one.
A diagnostic detail was "any string without a path separator, capped at
200 chars". That let a path with no slashes, a command fragment, a prompt
line, or an API key through verbatim — the rule was structural but it
guarded the wrong structure. A detail is now the 16-hex HMAC-SHA256
fingerprint of the offending input under the host privacy key: identical
failures dedupe, distinct failures differ, and no substring of the input
survives. A caller with no key omits the field entirely rather than
emitting an unkeyed digest, which is what `keyedDetail` is for.
isolateRecords is the only place a detail is derived. Diagnostics a
caller RETURNS are trusted for index and code only; any detail they carry
is stripped, so an unkeyed or caller-invented digest cannot cross the
boundary through a loose cast. The three decoders that reported
malformed-json (opencode-session, vscode-cline, zed) are wired to it.
Copilot's JetBrains dedup key embedded an unkeyed sha256 of the
assistant's REPLY TEXT — dictionary-attackable for short replies ("OK",
"Done.") — and it crosses into the envelope and the CLI ledger. It is now
an HMAC under the host privacy key, with a
`cli-shutdown-cost-v1-skills-dedup-key-hmac-v1` parse version: copilot is
the sole durable provider, so its union-merge would otherwise keep the
old-shape keys and append the new ones for the same records.
Breaking for consumers: DIAGNOSTIC_DETAIL_MAX is gone, sanitizeDetail
takes a key, DiagnosticDetail accepts only the fingerprint, and
RecordOutcome.diagnostics may no longer carry a detail.
dedupKey is a CallObservation field: it ships on the envelope. Six
decoders — codebuff, zerostack, pi, omp, grok and lingtai-tui — were
folding the absolute source path (a chat directory, a session file) into
it raw, so a host path rode every payload derived from those sessions.
They now fold `sourceRefFingerprint`, a keyed HMAC-SHA256 under a new
`source` domain. Like every other fingerprint in that module the key is
required and an empty key throws, so a source ref can never degrade to an
unkeyed, dictionary-attackable digest.
That means the CLI bridge has to supply a real key. It passed `''`, which
was correct when minimization happened only on the sync path; it now
threads getHostPrivacyKey() — per-install stable, so dedup keys stay
stable across runs.
The keys change VALUE, so the six carry a `source-ref-fingerprint-v1`
parse version that forces one re-parse and drops the cached raw-path keys
instead of re-ingesting the same records under two shapes. A parse
version cannot see the privacy key change, though, and a lost, rotated,
or ephemeral (unwritable config dir) key would silently produce keys that
never match the cached ones — so computeEnvFingerprint folds a digest of
the key in for exactly those providers.
The five bridge parity goldens used to compute their expected keys with
the same function and key production uses, which pins nothing. They now
re-derive the fingerprint longhand under a pinned test key, assert the key
SHAPE, and assert the raw fixture path appears nowhere in it.
`model` on a CallObservation was `z.string().min(1)` — unbounded free text
on a field that ships in the envelope. Bound it (and the optional
`pricingModel`) to the identifier charset real model slugs use,
`[A-Za-z0-9._:/@-]`, max 128, in both the zod schema and the published
schemas/observation-0.2.0.json.
Providers report display names ("Gemini 3.5 Flash (High)"), so a hard
reject would fail a whole multi-session batch over one call. Every
provider's toObservations now normalizes through
`normalizeModelIdentifier`: conforming values pass through untouched,
everything else collapses to 'unknown' — the same fallback the decoders
already use when no model resolves.
BOUNDARY ONLY. The normalizer is never called while building a dedup key:
a key is an identity, not an observation field, and collapsing two
distinct raw models to 'unknown' inside one would merge two real calls
and re-key every cached key for a partial win. The key's free-text
surface is schema-wide (every provider's key carries raw components) and
belongs to the dedup-key-hashing follow-up.
CLI output is unaffected — normalization happens at the observation
boundary only, so no terminal, dashboard, menubar or desktop number or
label moves.
Strengthens the --attribution wire assertions to confirm the local
repo directory and a fixture commit subject never appear in the raw
OTLP payload, not just that known-sensitive markers are absent.
codeburn sync now derives device/span/trace ids as domain-separated
HMAC digests under the per-install privacy key instead of bare hashes,
a one-time identity re-key on upgrade. sync push also hard-aborts on
an unwritable config dir or a corrupt privacy-key file rather than
falling back to an unstable ephemeral key.
#929 merged mid-task and claimed DAILY_CACHE_VERSION 24 for the codex
structural-discovery + midnight-straddle fixes. This PR's pi/omp/cline/
opencode discovery fix now takes 25, the next free number, so it forces
its own one-time re-derivation on top of #929's. Test literal updated to
match (pre-fix pinned at 24, the base's actual value before this bump).
No test exercised the reparse gate for the PROVIDER_PARSE_VERSIONS entries
this PR bumps. Reproduce each provider's pre-bump fingerprint and assert it
now differs, proving a cache keyed under the old fingerprint is treated as
stale.
The seeded old-cache version was a guessed 16 that was never this branch's
real pre-fix version, so the re-derivation test passed even with the
version bump reverted. Pin it to 23 (the base's DAILY_CACHE_VERSION right
before this PR's bump), verified to fail when the bump is reverted.
Base moved to DAILY_CACHE_VERSION 23; this PR's discovery fix now takes 24
to force its own one-time re-derivation on top of the codex structural-
discovery bump. CHANGELOG keeps both entries.
unlink-then-recreate at the same path lets ext4 hand the freed inode
straight back, which CI runners reproduce reliably; the assertion then
fails as 'expected N not to be N'. Write the replacement beside the
original while the old inode is still allocated, then rename over it.
The "anchor sits before the range" test ran on the codex path, where the
turn anchor is derived from the first call — the scenario it describes cannot
occur there, so it passed with parser.ts fully reverted. Moved to the claude
path, where turn.timestamp is the user-message time and can precede the first
assistant call. Verified: fails on pre-fix parser.ts with the 23:57 anchor,
passes with the fix.
Added a `report --format json` test for a turn whose two calls straddle local
midnight, asserting both days appear with a one-call-each split and that the
per-day costs still sum to the headline.
Also drop the `?? classifiedFull` fallback in both parseProviderSources loops.
turnSlicedToRange has already proved a call is in range over the same call
list, so the fallback is unreachable — and if it were ever reached it would
silently restore the whole turn and reintroduce the straddle bug rather than
fail loudly.
Conflict: packages/cli/src/daily-cache.ts. #926 landed
DAILY_CACHE_VERSION/MIN_SUPPORTED_VERSION at 23 for the structural codex
discovery re-derivation; this branch had claimed 17 for the midnight-straddle
re-derivation. Kept #926's rationale as the foundation and took 24, the next
free number, with the straddle reason appended: per-call day slicing changes
which day a historical straddling call lands on, so it needs its own one-time
re-derivation and cannot ride on a 23 cache written by a #926 binary.
Base has moved since this branch opened: main shipped 17 in v0.9.20 and
now sits at 20, with 21 (#946) and 22 (#1056) claimed on the main-side
pipeline. Bumping to only 16 would let a main-built cache pass the
version check unchanged, so the widened Codex discovery re-derivation
this PR depends on would never fire for those users. Take 23 to stay
above every value a real cache file can carry on either line of
history, and update the test literal and MIN_SUPPORTED_VERSION to
match.
Also documents, next to CODEX_CACHE_VERSION, why that constant is
deliberately not bumped alongside it: the guarded record shapes were
measured at 0 occurrences across 136k real events, so forcing a full
re-parse of multi-GB rollout corpora for them is a bad trade. The
daily-cache bump alone already propagates the discovery widening.
The packages/cli move (dc97ab49, Jul 26) forked vitest.config.ts and the test
script off main before three since-merged CI-stability fixes existed there,
and #921's later "port upstream fixes" pass (f3f5814b) missed all three since
it only cross-referenced one specific main commit:
- 30037b33: global vitest testTimeout raised from the 5s default to 30s
(real-I/O tests exceed 5s under CI runner load)
- 8c758ddf: file-level 30s timeout on cli-status-menubar.test.ts (spawns the
real CLI; individual cases observed needing 6-8s on a shared 2-core runner)
- 4ca2d482: cache-refresh-lock tests excluded from the parallel `test` run and
quarantined behind a serial `test:locks` script (they contend for the fork
pool with the spawned-subprocess tests and starve everyone under load)
None of this showed up until #923 wired `npm test --workspace=codeburn` into
CI for the first time, which then failed deterministically at the vitest
default 5000ms on exactly the three test categories these fixes cover.
Ports all three, matching main's current config/script.
Two pre-existing CLI-suite failures blocked #923's new cli CI job on Linux:
- tests/models.test.ts DeepSeek v4 pricing (x3): loadPricing() fetched the
live LiteLLM table during tests, and live data wins over the bundled
snapshot, so the DeepSeek v4 assertions went red when upstream repriced.
Ports main's fix (4866332b): CODEBURN_PRICING_SNAPSHOT_ONLY skips the fetch
and prices purely off the bundled snapshot; env-isolation.ts sets it for
the whole suite.
- tests/parser-proxy-pricing.test.ts case-insensitive isProxiedPath: the
branch's isProxiedPath/normalizeProxyPath is byte-identical to main's
(case folding is deliberately darwin/win32-only, per the function's own
comment). The test hardcoded the macOS-only expectation, so it fails by
design on Linux CI. Main hit and fixed the same thing (bcf11552); this
ports that fix verbatim: assert the platform-correct behavior instead of
a hardcoded `true`.
Ports three upstream commits this branch never received: the attribution
feature (1bf7206), the review hardening on top of it (ccee28a), and the
security follow-up that closed credential-leak paths and added session
retraction (50c8251).
They are ported as an end state rather than in sequence. Two and three revise
one, so replaying them in order would have introduced the very issues they fix
and then removed them again — and anything missed in the third pass would have
shipped a feature with a reopened hole, which is the specific way this port
could have gone wrong. The credential-leak paths that commit closes are
enumerated and checked off individually against the result.
One correction to an earlier draft of this message, which claimed no new
unkeyed digest is introduced. That was wrong: stateHash in sync/otlp.ts is a
new unkeyed sha256, and it feeds deriveSpanId, so it is an input to a value
that goes on the wire. It is not a D1 violation — D1 governs core's fingerprint
module and its caller-supplied key, while stateHash is a local ledger
discriminator computed over data that is itself sent in cleartext, so it hides
nothing and leaks nothing. But the sentence was false and is worth correcting
rather than quietly dropping.
This branch now carries #931's commit (c467548, "fix(sync): key the device,
span and trace digests") beneath this one — cherry-picked onto the shared base
so the history stays two clean commits. That ordering is load-bearing:
reconciliation is mandatory in every merge order, not optional. git merge-tree
reports no conflict against #931 in either direction, yet the merged file does
not compile: #931 drops the createHash import and gives the derive functions a
privacyKey first argument, so an unreconciled attribution section leaves
stateHash with an undefined symbol and two one-argument call sites. Rebasing
replays the same breakage, which is why the earlier "land #931 first, OR
reconcile" framing was wrong.
The two call sites are reconciled INTO #931's keyed signatures, in the
direction #931 demands: buildAttributionOtlpPayload obtains the persisted host
privacy key exactly as buildOtlpPayload does — one getPersistedHostPrivacyKey
call per builder, no second source of the key — and threads it into
deriveTraceId and deriveSpanId. This is the security point of the
reconciliation: loosening the signatures back to one argument would
reintroduce exactly the unkeyed span and trace ids #931 exists to remove, in
new code.
stateHash stays unkeyed, deliberately: it is a local ledger discriminator over
One more merge-compat fix, in #931's own test file (sync-privacy-key.test.ts):
the concurrency fixture path was built from process.cwd(), which is the repo
root under `--root packages/cli` — the worker then exited on a nonexistent
file before writing its ready file and the race test timed out. The path is
now anchored to the test file's own location (fileURLToPath(import.meta.url)).
This is the only line of #931's tree this branch touches; 37a5b46 remains a
verbatim copy of c467548.
Second fix in #931's tree, same motivation: the concurrency race test
adopted with only a 50ms budget. createKeyFileExclusive polled the winner's
file 5x10ms after EEXIST, and the strict entry check refused an 'invalid'
file INSTANTLY — but the winner's create (open) and write are separate
syscalls, and under load the loser can read the still-empty file either at
entry or inside the poll. Both windows now share one bounded awaitValidKey
(500ms) that ADOPTS the winner's key when it lands and otherwise throws the
same refusal. Nothing is ever overwritten; a file left invalid by a crash or
truncated write still fails loudly. This is the second #931 file this branch
touches; 37a5b46 remains a verbatim copy of c467548.
