[COMPAT] feat(metrics): profile sqlite storage

[NathanFlurry]

• Profile native SQLite statements and storage.
• Export bounded Prometheus metrics and sampled diagnostics.
• Add end-to-end native SQLite metric coverage.

[claude]

Code Review

Reviewed the diff for correctness, performance, and consistency with CLAUDE.md conventions (SQLite package rules, per-actor isolation, Rust test layout).

High-confidence issues

1. Failed COMMIT/ROLLBACK abandons the SQLite transaction while releasing the lock (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite.rs:209)
   SqliteTransaction::finish() unconditionally sets state.finished = true and drops state._guard before returning. SQL-level errors (e.g. a failed COMMIT from a deferred FK violation) travel inside SqliteWorkerResult.result, not the outer Result, so the caller's follow-up rollback() call hits anyhow::ensure!(!state.finished, ...) and is silently discarded (let _ = ... in SqliteDbExt::transaction()). No ROLLBACK is ever sent to SQLite, yet the lock is already released. Later unrelated calls on the same connection can then run while SQLite still thinks it's mid-transaction. Consider only marking finished/dropping the guard on genuine terminal outcomes, or forcing a ROLLBACK attempt when COMMIT fails before releasing the lock.

2. Per-actor profiling caps enforced against process-global counters (rivetkit-rust/packages/rivetkit-core/src/actor/metrics.rs:2066, 716, 602)
   Several new admission/rate-limit mechanisms mix a process-wide static (SQLITE_PROFILE_ADMISSION.series, SQLITE_DIAGNOSTIC_RATE_STATE, SQLITE_DIAGNOSTIC_SENDER via OnceLock) with per-actor configuration (maxPrometheusSeries, maxDiagnosticEventsPerMinute, diagnosticEventQueueCapacity):

   • reserve_sqlite_series compares the shared global series count against each actor's own cap, so a busy actor can starve a differently-configured actor's admission entirely (20_000 + cost <= 500 never succeeds for actor B even though B reserved 0 series itself).
   • The diagnostic rate limiter bucket is process-wide, so one actor's traffic consumes another actor's configured burst allowance.
   • The diagnostic-event channel capacity is fixed by whichever actor's config runs first through get_or_init; every subsequently-constructed actor's diagnosticEventQueueCapacity is silently ignored.
     Given multiple actors run in one process, these should be keyed per-actor rather than shared singletons.

3. Fingerprint hash stored as ASCII hex text instead of decoded bytes (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite/profiling.rs:130)
   fingerprint_hash() returns a 16-char hex string; record_statement copies source.as_bytes() (the ASCII string) straight into a [u8; 16], instead of hex-decoding into the raw 8-byte digest. Any consumer that treats this array as a binary digest (re-encoding for display, comparing against another binary digest) will get wrong or garbled results.

4. transaction_wait_ns hardcoded to 0 (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite.rs:233, also line 571)
   started_at is captured before acquiring transaction_lock, but the emitted transaction_wait_ns metric is always 0, so real lock-contention time gets folded into application_time_ns/local_work_ns instead. This corrupts profiling data specifically during the contention scenarios the feature is meant to surface.

5. Profiling-disabled actors still pay profiling overhead inside explicit transactions (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite.rs:131)
   SqliteTransaction::exec/execute/finish and begin_named_transaction always go through the _profiled path with no check of profiling.config.enabled, unlike the autocommit SqliteDb::exec/query/run/execute methods, which correctly branch to the cheap non-profiled path when disabled. Statements run inside a transaction on an actor with sqliteProfiling.enabled = false still incur per-bind/per-column profiling bookkeeping.

Other findings

6. Default-on profiling changes behavior for existing actors (rivetkit-rust/packages/rivetkit-core/src/actor/config.rs:78)
   SqliteProfilingConfig::default() sets enabled: true. Every pre-existing actor using db: sqlite() will start paying for the new fingerprinting/histogram/diagnostic-event machinery and emitting Prometheus series on upgrade, with no opt-in. This looks intentional (covered by tests/config.rs), but it's worth confirming this is the desired rollout behavior rather than defaulting new observability features off until explicitly enabled.

7. Duplicated profiled/non-profiled query execution paths (engine/packages/depot-client/src/query.rs:190)
   execute_single_statement_profiled/exec_statements_profiled are near-total copies of execute_single_statement/exec_statements, unlike bind_params/bind_params_profiled, which share a single bind_param primitive. Future prepare/step/finalize fixes risk being applied to only one twin. Worth factoring the shared control flow the same way bind_param was factored.

8. Inline #[cfg(test)] mod tests in src/ (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite/profiling.rs:200)
   Per CLAUDE.md, Rust tests belong under tests/. The sibling new files sqlite.rs and metrics.rs correctly use the #[path = "../../tests/..."] shim into rivetkit-core/tests/, but profiling.rs embeds full test bodies inline instead of a matching tests/profiling.rs.

Suggestions

• Items 2 and 5 both stem from treating what should be per-actor state as process-global. Given the codebase runs multiple actors per process, it may be worth a quick audit for other new static/OnceLock state introduced by this PR to confirm it's either genuinely process-scoped or explicitly keyed by actor.
• Item 1 is the most severe. A silently-abandoned SQLite transaction with a released lock can lead to hard-to-diagnose data corruption or "database is locked" failures downstream.

---

Update: ci(release): enable npm oidc publishing commit

This commit (.github/workflows/publish.yaml, scripts/publish/src/{ci/bin.ts,lib/npm.ts,lib/version.ts}) switches npm publishing to trusted (OIDC) publishing. Reviewed separately since it landed after the section above.

• Good fix: publishAll now throws when counts.failed > 0 (scripts/publish/src/lib/npm.ts:384). Previously a partial publish failure was only logged, so the CI job would exit 0 and report success even when some packages failed to publish. This closes that gap.
• Worth double-checking in a live run: the publish step sets NODE_AUTH_TOKEN: "" rather than leaving it unset (.github/workflows/publish.yaml:544). actions/setup-node writes //registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN} into .npmrc, so this resolves to an explicit-but-empty _authToken line rather than no line at all. This is usually harmless (empty token is treated as no token), but since it's the crux of whether OIDC trusted publishing actually engages instead of failing over to token auth, it's worth confirming against the actual npm publish output on the first real run rather than assuming from the diff alone.
• Behavior change, flagged for awareness: repairBranchPreviewLatestTags now unconditionally no-ops whenever NODE_AUTH_TOKEN is unset/empty (scripts/publish/src/lib/npm.ts:227). Since the workflow always clears NODE_AUTH_TOKEN for this job (both release and preview triggers share the same publish-npm step), this self-healing repair (fixing a latest dist-tag that got stuck pointing at a preview version) is now permanently disabled in CI and only reachable from a manual, token-authenticated local run. That looks intentional per the inline comment, just worth confirming the team is fine losing this automatic repair path in CI going forward.
• Nice addition: githubRepositoryUrl() validates the owner/repo shape with a strict regex before interpolating it into a git URL written to package.json (scripts/publish/src/lib/version.ts), and requirePublishRepository fails loudly instead of silently writing a missing/malformed repository field.
• The runs-on: depot-ubuntu-24.04-8 → runs-on: ubuntu-24.04 switch for the publish job is explained inline (OIDC trusted publishing needs a GitHub-hosted runner) — reasonable trade-off, worth being aware it loses whatever caching/speed benefit the Depot runner had for this one job.

Overall

The profiling feature is a solid addition. The main risks are the transaction-abandonment bug (1) and the process-vs-per-actor scoping issues (2), which should be addressed before merge. The rest are lower-severity correctness/consistency cleanups. The OIDC publishing commit looks reasonable and includes a genuine bug fix (failing the job on partial publish failure); the empty-vs-unset NODE_AUTH_TOKEN behavior is the one thing I'd actually watch in the first real publish run.