Review finding, pinned red-first: the SDK accepts
processor.maxExportBatchSize <= 0 (or fractional), but its shutdown
drain then splices empty batches without consuming the queue —
disposing telemetry hangs forever whenever records are queued. The
constructor now rejects a non-positive-integer batch size before
building the SDK processor, per the misconfiguration-fails-loud rule;
everything else in the processor block remains the SDK's verbatim
passthrough.
Review finding: synthetic turn closers written by SessionPersistence
.load() at crash-resume sit below firstLiveSeq and are never exported,
leaving the remote turn unbalanced. Kept deliberately — exporting a
synthetic closer cannot complete a turn whose real tail records died
in the crashed process's queue; it can only make an incomplete turn
look closed. The OTel README now states the receiver rule (a
never-closed turn on a resumed stream marks the previous process dying
inside it; a later clean shutdown marker attests only to the resumed
process's exit), and the revival Agent Note records why the repair
suffix stays local.
A cursor-less adoption (process restart + resume, fork, seam-module
reload) replayed the session's full log from seq 0, re-exporting
history that already left the process — a resume re-billed its entire
stored log on every restart, and a fork re-shipped the parent's prefix
under the child's id, doubling query-time counts on OTLP backends with
no native ingest dedupe.
dsh-session now exposes the fact the constructor already validated but
discarded: Session.firstLiveSeq, the constructor-seed length — the
first seq appended in this process. header.seedLength cannot serve
here: it is the durable fork-lineage boundary, and a resumed session's
constructor seed is its full stored log while the header keeps the
original fork value (llm-replay and session-query-sqlite depend on
that meaning). Constructor seeds also never publish on the
session/event firehose, so adoption replaying them was inconsistent
with the system's own publication semantics.
Adoption's cursor-less fallback starts at firstLiveSeq; seed events
still feed the chunk projection, so mid-step continuations re-drop
after a resume. Fork streams are no longer self-contained: records now
carry session.seed_length (with the existing session.parent_id) so
receivers stitch the child's stream onto the parent's. Accepted cost,
consistent with at-most-once delivery and recorded in the revival
Agent Note: a resume no longer backfills records a previous process
failed to deliver — a deployment with that requirement needs the
deferred outbox, not replay.
Pinned red-first: seeded adoption exports nothing (assertion reversed
from the prior seed-readback test, obsolete behavior changed with its
test), resume-shaped seed rebuilds the projection without exporting,
and fork records carry the stitch attributes.
Three review rounds each found a new silent-loss path in the same
wrapper state (dispose racing an in-flight flush, overlapping hints
displacing the retained promise, the provider's fixed 30s flush timeout
rejecting while the processor still drains). Every path exists only
because forwarding the seam's turn-boundary hint to forceFlush() made
this backend the process's second flusher against undocumented SDK
internals from the upstream experimental tree.
The backend now implements no flush(): the batch processor is the only
flusher, its scheduledDelayMillis (already deployment-tunable through
the processor passthrough) governs export cadence, and shutdown()'s
drain is complete by construction. The two race-pin tests collapse into
one dispose-during-in-flight-batch drain pin; the seam's optional
flush() contract now tells implementers they own the concurrent-flush/
shutdown interaction. Removal rationale and the reinstatement trigger
(a stated turn-boundary latency requirement scheduledDelayMillis cannot
meet — and then via the processor's own forceFlush(), never the
provider's timeout-wrapped one) are recorded in the revival Agent Note,
both languages.
Resolutions: regenerate docs/capability-seams.md (the only conflict);
master widened the bilingual pairing scope to package READMEs, so this
merge adds the Chinese counterparts and i18n records for the three
telemetry READMEs and patches packages/README.zh.md with the telemetry
group row.
Review finding (Codex P1), pinned red-first: the marker was tied to
telemetry-plugin lifetime, but receivers key crash detection on its
absence per session. A normally closed session in a long-running host
retired silently (classified as a crash once stale), while a telemetry
reload marked every still-live session as cleanly ended.
The session/disposed handler now emits the marker at the session's own
termination edge before retiring it; the dispose-time sweep only marks
sessions still alive at application teardown (their own edge would fire
unobserved). READMEs restate the marker semantics: telemetry stopped
observing cleanly — a marker followed by more session events is a
telemetry reload, not a session restart.
Second review round, both pinned red-first:
- Overlapping turn-boundary flush hints now JOIN the outstanding flush
promise (Promise.all) instead of displacing it: the SDK's
concurrent-flush guard resolves an overlapping forceFlush()
immediately, so retaining only the latest promise let shutdown()
proceed while the first export was still in flight — the same silent
drop the single-flush fix closed.
- Adoption replay contains failures per event, matching the firehose:
one rejected record is withheld fail-closed while the rest of the
historical log still hands off. Wrapping the whole loop let a single
failure silently skip the remainder on an already-adopted session.
Three review findings, each pinned by a red test first:
- The OTel backend retains the latest turn-boundary flush promise and
shutdown() awaits it before provider.shutdown(): the SDK's
concurrent-flush guard makes the shutdown-internal flush return early
while one is in flight, silently dropping everything enqueued after
the flush snapshot (including the coordinator's dispose-time shutdown
markers).
- The coordinator retires sessions from the adopted set on
session/disposed (mirroring the persistence coordinator): a long-lived
backend no longer retains closed sessions and their event logs, and
final unload no longer stamps shutdown markers for dead sessions.
- The exporter config passes through whole to OTLPLogExporter and its
type/JSDoc now advertise the full OTLPExporterNodeConfigBase shape:
rebuilding url/headers only silently dropped documented SDK options
(timeoutMillis, compression, keepAlive, ...).
tests/otel.e2e.ts self-skipped on $DSH_OTLP_E2E_ENDPOINT, which nothing in
the repo sets — it never ran; the mock-collector wire spec and the keyless
Loader-composition e2e already cover the pipeline both ways.
The severityOf compact/end probe parsed another package's merged event
shape by string comparison — an untyped cross-package contract that breaks
silently — and its only consumer was the test's own stand-in declaration.
Unknown event types now uniformly fall through as info; outcome semantics
stay with the owning package.
The seam keeps the telemetry/redact scrubbing interface but ships no rules
of its own: the innermost next() passes records through unchanged, and
deployments mount their rules as waterfall listeners. As an SDK we cannot
know which patterns are secrets in a given deployment; a shipped list
invites false confidence while catching only known shapes, and false
positives would corrupt exported bodies. Mechanism stays with the seam,
policy moves to the deployment; both READMEs and the Agent Note state the
raw-export default plainly.
The loader-composition e2e now mounts a deployment-style rule fixture and
pins the same wire behavior: secret absent, placeholder present, canonical
log untouched.
Revive the reviewed session-telemetry packages from the closed
session-telemetry-otlp-rfc branch (PR #222/#231) on current master, renamed
to @deepseek-ai/dsh-session-telemetry{,-otel} (the SDK component-telemetry
package holds the dsh-telemetry name).
Delta over the branch version: every record now passes a telemetry/redact
waterfall between projection and emit() — the innermost next() applies a
non-configurable conservative credential-shape rule set, listeners stack
stricter rules, a throwing rule withholds the record fail-closed, and the
canonical log is never rewritten. This answers the export-side concern that
closed PR #222; the boundary axiom (our aspect ends at emit(); delivery is
the reporting SDK's) is unchanged, and the runtime-telemetry RFC's outbox /
readCommitted lane is recorded as deferred in the Agent Note.
Covered by seam/redact/OTel-wire unit tiers (100% per-file) and a keyless
Loader-composition e2e that boots the examples fixture against a mock OTLP
collector and pins redaction on the wire plus the untouched canonical log.
