feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging - #138

Open
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375
Open

feat(shadow)!: mismatchLogging content controls, value projection, and diff logging#138
lan17 wants to merge 9 commits into
mainfrom
claude/shadow-mismatch-logging-36a375

Conversation

@lan17

@lan17lan17 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Why

Shadow validation compares a cached value against the source of truth and emits a mismatch metric when they disagree. Until now the only way to see what disagreed was shadow.logMismatches: true, which logged the cache key plus the full JSON of both values — all or nothing. That's a problem for use cases whose values carry user data: one dynamic-config flip could put whole records into logs.

This PR splits the warning into parts you opt into individually at runtime, and adds two per-use-case hooks so values can be redacted (or summarized as a diff) before they ever reach the logger.

The new runtime config

ShadowConfig.logMismatches is gone. In its place:

shadow: {ramp: 5,mismatchLogging: {key: true,// log which key mismatchedvalue: false,// log the two compared valuesdiff: true,// log a structural diff of the two values},}
FieldAdds to the warningCap
keycacheKey — the logical DialCache URN2 KiB
valuecachedValueJson + sourceValueJson8 KiB each
diffdiffJson — which paths differ and how8 KiB

Semantics worth knowing:

  • Everything defaults to off. A warning is emitted only when at least one field is true, and it always carries cacheNamespace / useCase / keyType / outcome for routing, plus cachedValueAgeSeconds — how long the stale value had been readable when validation caught it (the same age the observeShadowValueAge metric records).
  • Fields merge leaf-wise like the rest of shadow config, so dynamic config can turn value off mid-incident while keeping key on — no deploy needed.
  • An invalid value for a known field (say value: "yes" from a bad config push) acts as false and records one config_resolution error; valid sibling fields keep logging. The cache result is never affected.
  • Unknown own fields anywhere in key config are ignored while recognized fields still apply. Each observed config containing one or more unknown fields records one bounded error=config_unknown_field metric; field names and values never become labels. This includes legacy shadowRamp and shadow.logMismatches from mixed-version providers.
  • DialCacheKeyConfig.disabled() sets all three to false, so the kill switch still kills logging.

Shaping what gets logged (code-level hooks)

Two optional hooks live next to shadowComparator on cached() / getOrLoad():

constgetUser=dialcache.cached(fetchUser,{useCase: "GetUser",keyType: "user_id",cacheKey: (id)=>id,// Runs once per side on a confirmed mismatch. Whatever it returns is what// `value: true` logs AND what the built-in diff compares — so sensitive// fields stripped here can't leak through either output.shadowMismatchLogValue: (user)=>({id: user.id,updatedAt: user.updatedAt}),// Optional: replace the built-in diff entirely. Receives the RAW values.shadowMismatchLogDiff: (cached,source)=>({versions: [cached.version,source.version],}),});
  • No projector?value: true and diff: true log the raw values, same material as the old behavior. Define the projector for any use case whose values can carry sensitive fields before enabling those flags.
  • Hooks fail closed. A projector throw or promise-like result logs null for that side (and a null built-in diff) — it never falls back to raw values. A diff-hook throw or promise-like result logs diffJson: null. Promise-like settlements are consumed but never awaited, so an accidentally async callback cannot leak an unhandled rejection or silently create a second hook contract.
  • Hooks run only after terminal mismatch confirmation, inside detached shadow work — never on the request path. Same discipline as the comparator: synchronous, bounded, non-mutating.

What a warning looks like

DialCache shadow validation mismatch {
cacheNamespace: "urn",
useCase: "GetUser",
keyType: "user_id",
outcome: "mismatch",
cachedValueAgeSeconds: 5243.7,
cacheKey: "{urn:user_id:123}#GetUser",
diffJson: '[{"type":"CHANGE","path":["updatedAt"],"value":"2026-08-14T01:02:03.000Z","oldValue":"2026-08-13T22:10:00.000Z"}]'
}

The built-in diff

No new dependencies: the built-in diff is a small own-key differ over the native-JSON forms of both sides — each side is rendered once and the value fields and diff derive from the same snapshot, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging. Entries are { type: "CREATE" | "REMOVE" | "CHANGE", path, value, oldValue }, oriented cached → source (oldValue is what Redis had). A side with no JSON rendering (top-level undefined, cycles, bigint, or a thrown or promise-like hook result) fails the diff closed to null.

  • The diff is computed over the loggable forms: both sides render to native JSON first, so toJSON redaction and serializer normalization bound the diff exactly as they bound value logging — no leaking fields toJSON hides, and no phantom entries when a deserialized ISO string meets a live Date of the same instant.
  • Roots of the same container kind diff recursively; primitives and mixed object/array roots collapse to a single root-level change entry.
  • diffJson: "[]" means the loggable inputs held no visible difference — with a projector, that reads as "the difference is inside fields you chose not to log", which is itself a useful signal.
  • Known noise, documented and pinned by tests: arrays compare by index (a shift reports every later index), and the diff shows structural differences even in fields a custom comparator ignores. It's debugging evidence, not the comparator's verdict.

Migration — breaking API, compatible runtime

BeforeAfter
shadow: { logMismatches: true }shadow: { mismatchLogging: { key: true, value: true } }
shadow: { logMismatches: false }omit mismatchLogging (or set fields to false)

shadow.logMismatches is no longer read, but older default or runtime config carrying it does not break cache resolution. It is ignored like any unknown field, recognized fields still apply, and DialCache emits the bounded config_unknown_field signal. The old true value therefore does not enable mismatch logging; migrate to mismatchLogging to retain that behavior.

Review

A six-lane review (correctness, tests, simplicity, architecture, contracts, security, plus a two-stage holistic audit) ran against the feature; all 10 accepted findings are fixed in the follow-up commit: diff-input normalization (the toJSON/serializer asymmetries above), one cap owner for diffJson, own-property-only runtime shadow config reads (prototype pollution and Object.create-carried leaves can neither enable payload logging nor admit shadow work), documented-and-pinned config_error semantics for a malformed mismatchLogging group (matching the layer-map precedent), an exhaustive compile-checked leaf list, and removal of the dead emit-time fallback (every warning field now fails closed to null independently).

A follow-up correctness and simplicity review closed the async-hook rejection leak. A later compatibility pass made unknown key-config fields permissive across defaults and runtime overlays: normalization keeps only recognized fields, one bounded metric reports each affected config, and known malformed values retain their existing validation.

Testing

  • pnpm typecheck · pnpm test (559 passing, ~97.7% coverage) · pnpm build · pnpm test:package (packed ESM + CJS consumers) · CI pnpm test:integration (all 139 Redis and cluster tests passing).
  • Warning content per flag combination, projection on both sides, per-side projector throw or promise-like return → null, projection failure nulling the built-in diff, one projection per side feeding value + diff together, custom diff hook receiving raw values (and staying idle when diff is off), promise-like custom-diff results failing closed without unhandled rejection, hooks staying idle on superseded, per-leaf invalid runtime config, unknown fields at every key-config scope (including explicit undefined and both legacy shadow fields) being ignored with one bounded metric while recognized fields remain effective, leaf-wise merge, and clone/freeze.
  • Diff edge behavior pinned: CREATE entries, nested Date → ISO strings, array-shift noise, mixed-kind nodes, unrenderable sides and cyclic inputs failing closed to null, own-member traversal, and single-render toJSON consistency.

🤖 Generated with Claude Code

BREAKING CHANGE: ShadowConfig.logMismatches was replaced by the shadow.mismatchLogging content group ({ key, value, diff }); the previous logMismatches: true behavior is mismatchLogging: { key: true, value: true }. Legacy logMismatches fields are ignored at runtime and emit config_unknown_field rather than failing cache resolution, but they no longer enable logging, so migrate config stores to retain that behavior.

…rols and log hooks
Shadow mismatch warnings are now composed field by field through the
runtime ShadowConfig.mismatchLogging group ({key, value, diff}, each
default-off, merged leaf-wise) instead of the removed all-or-nothing
logMismatches boolean. Two per-use-case hooks shape the logged content:
shadowMismatchLogValue projects both sides before value logging and the
built-in diff, and shadowMismatchLogDiff replaces the built-in diff and
receives the raw compared values. The built-in structural diff uses
microdiff over the projected-or-raw forms, oriented cached-to-source,
with non-plain-object roots collapsing to one root-level change entry.
All rendering happens eagerly at mismatch confirmation, fails closed to
null fields, and keeps the existing byte caps; raw compared values are
no longer retained until log time.
The removed shadow.logMismatches field is rejected like shadowRamp:
defaults throw at registration and stale runtime configs fail
resolution as config_error, so live configs must migrate to
shadow.mismatchLogging before adopting this release.
Covers the gaps in diff-logging coverage: CREATE entries, nested Date
leaves rendering as ISO strings, index-wise array-shift noise, cyclic
inputs failing closed to a null diff, a projection throw nulling the
built-in diff, one projection per side feeding value and diff output
together, an idle shadowMismatchLogDiff hook when diff logging is off,
and hooks staying uninvoked for superseded candidates.
…ty, and hardening
Resolves the accepted findings from the multi-lane review of the
mismatchLogging feature:
- The built-in diff now renders both sides to native JSON before
diffing, so toJSON redaction and serializer normalization bound the
diff exactly as they bound value logging: no more leaking fields that
toJSON hides, no phantom entries for serializer-normalized Dates, and
mixed object/array roots collapse to one root-level change entry as
documented. Identical loggable forms short-circuit to [].
- diffJson has one cap owner: previewShadowLogJson takes a byte budget,
the hook path and built-in path both clamp with the diff cap, and
previewShadowLogDiff delegates instead of duplicating the body.
- Runtime shadow config is read by own properties only (group, leaves,
and ramp, in both the merge and admission reads), so prototype-carried
values can neither enable payload logging nor admit shadow work.
- A non-object runtime mismatchLogging group is documented as malformed
config shape that fails resolution as config_error, matching the
layer-map precedent; the unreachable admission-time branch is deleted
and the behavior pinned by runtime-overlay tests alongside the
previously uncovered removed-logMismatches rejection.
- The leaf set is derived from one exhaustive, compile-checked list;
ShadowLogPlan aliases Required<ShadowMismatchLoggingConfig>; the
disabled() kill-switch literal is annotated exhaustive.
- Dead emit-time fallback deleted: every preview fails closed to a null
field (previewShadowLogKey included), the warning path is throw-free
by construction, and the warning payload is built fresh so a mutating
metrics adapter cannot contaminate it.
- New tests: both-hooks projection/raw split, prototype pollution,
Object.create-carried leaves, per-field fail-closed, toJSON-bounded
diff, serializer-normalization phantom, mixed-kind roots, explicit
byte budgets, and the runtime rejection rows.

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Blocking review: I found six issues that should be addressed before merge. Five are attached inline.

The remaining release blocker is:

[P1] Add an exact BREAKING CHANGE: footer to the PR body. The migration heading is useful to human readers, but this repository's conventionalcommits release generator only recognizes the footer form called for in README.md. Because squash commits use the PR body, the generated release notes will otherwise omit the breaking-change section. That is especially risky here: stale shadow.logMismatches runtime config fails resolution and bypasses caching for each affected invocation, so operators need the config-before-code deployment order surfaced in release notes.

All seven checks are green, and I also ran the full local check successfully (typecheck, 551 tests, build/declarations, and packed ESM/CJS package validation). The exact-head CI integration run passed 139 Redis/Valkey tests. The targeted cases in the inline findings are not covered by those checks.

Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/internal/runtime-config.ts
Comment threadsrc/internal/shadow-log-json.ts Outdated
Comment threadsrc/dialcache.ts Outdated
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
lan17 added a commit that referenced this pull request Aug 15, 2026
…and render-once own-key diff
Review fixes for PR #138:
- The shadow group itself is now an own-property read at the constructor,
defaults-snapshot, and runtime-merge boundaries, so a prototype-inherited
group can no longer activate logging policy (its leaves are own properties
and passed every inner gate).
- Unknown mismatchLogging fields fail closed: defaults reject them at
registration, and a runtime override carrying one (a typo'd emergency
shutoff like `vaule: false`) turns the whole logging group off with one
config_resolution error instead of silently inheriting enabled leaves.
The merge carries unknown own keys through so admission can see them.
- Mismatch warnings render each side to native JSON exactly once and derive
the value fields and the built-in diff from the same snapshot, so a
stateful toJSON cannot put data in diffJson that value logging redacts.
A side with no JSON rendering (top-level undefined, cycles, bigint, a
thrown hook or projection) fails the diff closed to null instead of
emitting a self-contradictory root entry.
- The built-in diff is now a small own-key differ over the parsed JSON
snapshots, replacing microdiff: prototype-carried data (enumerable
Object.prototype or Array.prototype pollution) can never reach diffJson,
and the microdiff runtime dependency is removed. Entry format, orientation,
and array index-wise semantics are unchanged and remain pinned by tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lan17lan17 changed the title feat(shadow): mismatchLogging content controls, value projection, and diff loggingfeat(shadow)!: mismatchLogging content controls, value projection, and diff loggingAug 15, 2026

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

One blocking finding remains after the follow-up commit. The other five findings from the prior review are fixed, and the exact-head local/CI validation is green, but the prototype-data boundary for the built-in diff is still incomplete. Details are attached inline.

Comment threadsrc/internal/shadow-log-json.ts
…N hooks
The own-key differ kept prototype data out of the entries, but the finished
entry tree was still handed to native JSON.stringify, whose inherited-toJSON
lookup let a polluted or legacy Array.prototype.toJSON replace the whole
diff. The diff tree is now serialized by a closed-domain walker that only
gives primitives to native JSON, so toJSON runs solely while rendering user
data into the side snapshots, never over the internally generated entries.
@lan17
lan17force-pushed the claude/shadow-mismatch-logging-36a375 branch from 1699bbb to 7aa731cCompareAugust 15, 2026 06:10

@lan17lan17 left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Correction to my previous review: I withdraw the remaining inherited-prototype blocker. It applied a hostile-intrinsics threat model that does not match this feature’s trusted-value boundary.

The current head now keeps the design deliberately simple: one native JSON snapshot per side, a private parsed JsonValue, own-member structural traversal, and ordinary bounded JSON serialization. The custom prototype-hardening code, tests, and claims have been removed.

The other five findings remain addressed. Full local validation passed (typecheck, 560 tests, build/declarations, and packed ESM/CJS package checks); Redis integration passed 137 tests with the two GLIDE Cluster cases skipped by the local Docker environment. All current GitHub checks pass. I have no remaining findings from this review.

lan17and others added 3 commits August 15, 2026 12:05
Every opted-in mismatch warning now carries cachedValueAgeSeconds, the
same coarse mixed-clock age observeShadowValueAge records for the
verdict, so a single log line distinguishes a seconds-old race from a
days-old invalidation bug without consulting the histogram.
Treat promise-like logging-hook results as unavailable and consume their settlements so detached shadow work cannot leak unhandled rejections. Preserve unknown runtime logging keys even when explicitly undefined so closed-schema admission rejects the whole group.
Apply known config fields while ignoring unknown own keys across defaults and runtime overlays, including legacy shadow fields. Emit one bounded config_unknown_field error label per observed config without exposing field names or values.
Sign up for freeto 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.

1 participant

@lan17