**Detail bug report:** [View on
Detail](https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_8cac9425-ce79-4db4-b569-bf59c78d8d81)
Closes#778
## Bug
In `lite/src/server.rs`, `s3_builder()` builds the `object_store` AWS S3
client backing SlateDB. It branches on whether static env credentials
(`AWS_ACCESS_KEY_ID` + `AWS_SECRET_ACCESS_KEY`) are present:
- The **static-credentials arm** set credentials but never resolved the
region — it relied solely on `AmazonS3Builder::from_env()`, which reads
only `AWS_*` env vars and silently falls back to `us-east-1` when
`AWS_REGION`/`AWS_DEFAULT_REGION` are unset.
- The **sibling `_ =>` arm** resolved region from the full AWS default
chain (env → profile → IMDS) via `aws_config::load_defaults` and called
`with_region`.
The two arms of the same `match` resolved the same field (`region`) by
different chains. A deployment that supplied credentials via env but
region via `~/.aws/config` hit the static arm, dropped the profile
region, and targeted `us-east-1`. Against real AWS S3 for a bucket
elsewhere this produces a wrong-region `301`; `object_store` does not
disable reqwest's redirect following, and `remove_sensitive_headers`
strips the `Authorization` header on the cross-host redirect, so the
followed request to the correct-region endpoint is unauthenticated and
AWS rejects it with a non-retryable `403`.
## Fix
The static arm now resolves region env-first (`AWS_REGION` then
`AWS_DEFAULT_REGION`), and only when both env knobs are absent falls
back to `aws_config::load_defaults(...).await.region()` — the same
standard chain the `_ =>` arm uses — then applies
`builder.with_region(region)`. This makes both arms resolve region by
the same chain tiers while preserving the env-first fast path for the
common static-creds + `AWS_REGION` workflow (`load_defaults` is only
reached when env region is absent, i.e. the trigger intersection). The
credential wiring (`StaticCredentialProvider` + `AWS_SESSION_TOKEN`) is
unchanged; the change is purely additive region resolution.
## Testing
Added three hermetic `#[tokio::test]` regression tests in
`lite/src/server.rs` (`mod tests`) covering the static arm's new
contract:
- profile region is applied when env region is absent (the regression —
confirmed to fail on the pre-fix code with `left: None, right:
Some("eu-west-1")` and pass after the fix)
- env region takes precedence over profile (preserves the documented
fast path)
- no bogus region is synthesized when no region is available anywhere
Routine checks all pass: `cargo check --locked -p s2-lite` (default and
`--all-features`), `cargo +nightly fmt --all --check`, `cargo clippy
--locked -p s2-lite --all-targets -- -D warnings --allow deprecated`,
and the full s2-lite nextest suite (331/331).
End-to-end smoke (not versioned — ran against a containerized
S3-compatible backend during development): built the release `server`
binary and ran it against `adobe/s3mock` over HTTP with static env creds
and the region supplied only via `AWS_CONFIG_FILE` (the bug-trigger
intersection, `AWS_REGION`/`AWS_DEFAULT_REGION` unset). The server
logged the resolved `region=us-east-1` from the profile tier (previously
silently dropped), SlateDB wrote manifest/WAL/compaction objects to S3,
and the full basin/stream/append/tail API lifecycle succeeded (HTTP
201/200), with a clean SIGTERM shutdown and no `403`/`301`/redirect
errors. (LocalStack's latest image is license-gated; S3Mock was used as
the equivalent HTTP S3-compatible backend.) This confirms the
custom-endpoint static-creds path still works and the profile region is
now honored; it does not reproduce the live-AWS redirect/auth-strip
chain because S3Mock does not issue wrong-region 301s or validate SigV4
region.
Not verified: live AWS S3 end-to-end (a real bucket in a non-`us-east-1`
region with static keys). The environment has no AWS credentials (`aws
sts get-caller-identity` returns `Unable to locate credentials`, no
`~/.aws`, no IMDS), so the live-AWS 301 → auth-stripped 403 path could
not be captured directly. The hermetic regression test covers the
trigger (region misresolution), and the S3Mock smoke covers
no-regression plus profile-region resolution; the live-AWS HTTP-chain
leg is the only uncovered item.
---
_Automatic Fixes PRs can be [configured
here](https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/settings/repos/repo_c4bd6a47-9b7d-4b62-9c18-8cf0ac18a8f9/bugs)._
---------
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Expose available storage classes and the configured default in location
responses. Use `CompactString` for class names throughout the SDK, API,
common types, CLI, and resource specs, allowing future names to pass
through without a client release. Docs and the locations API describe
the available choices. The CLI displays available classes and the
configured default in `list-locations`, `get-default-location`, and
`set-default-location`; responses from older servers retain the previous
output when those fields are absent.
Remove OSS storage-class enums and the shared Express fallback. Omitted
classes stay unspecified until the server resolves them: cloud uses the
location default for basins and basin defaults for streams. CLI
apply/diff preserves unknown names. Dry-run reads the location default
once when needed and compares streams using the desired basin defaults.
If discovery is unavailable, it marks storage-class resolution as
uncertain instead of reporting a false change or no-op. Actual apply
leaves omitted classes for the server to resolve. Lite retains Express
as its own fallback when both the stream and basin omit a class,
including when reading older metadata. Explicit class names and basin
inheritance are preserved.
BREAKING CHANGE: remove `StorageClass` from `s2_sdk::types`,
`s2_api::v1::config`, `s2_common::config`, and `s2_resource_spec`.
Replace `.with_storage_class(StorageClass::Express)` with
`.with_storage_class("express")`, and enum matches with string
comparisons. Fields use `CompactString` with their existing
optional/patch wrappers; `s2_common::config::StreamConfig.storage_class`
is now `Option<CompactString>`. Omitted/null patch semantics are
preserved.
Validation: formatting, workspace Clippy with all features/targets, and
903 existing workspace tests passed. All 146 applicable integration
tests from unchanged Python SDK main passed against the local Lite
build. Temporary API checks verified the Express fallback, basin
inheritance, unknown class names, and null resets. Temporary CLI checks
verified all three location commands with future class names and
populated, empty, missing, and null discovery fields. Temporary dry-run
checks covered unchanged and changed defaults, future names, desired
basin inheritance, explicit overrides, unavailable discovery, error
propagation, and one lookup across multiple basins. A local Lite
create/preview/reapply check confirmed an uncertain preview and an
unchanged reapply. The generated CLI schema still matches. Existing Rust
tests were adapted; no new tests were added.
Related: [cloud](https://github.com/s2-streamstore/s2-cloud/pull/1836),
[specs](https://github.com/s2-streamstore/s2-specs/pull/24). Deploy [the
docs redirect](https://github.com/s2-streamstore/docs/pull/361) before
publishing the `/docs/storage-classes` links.
`tick_basin_deletion` reported more work whenever its page of pending
basins was full, even if every basin on the page was blocked waiting for
`stream_trim` to purge tombstoned streams. With 32 or more basins
pending and the first page blocked, `run_tick` re-ran the tick back to
back without sleeping, rescanning the same basins until `stream_trim`
caught up.
`PageProgress` now records per-item outcomes through a shared
`ItemProgress`: an item can advance with more work ready, complete, or
be blocked. Basin deletion reports one outcome per basin, so the backlog
keeps draining while some basin makes progress and otherwise waits for
the next tick. Resetting a basin's cursor to the start counts as
blocked, since treating it as progress would rescan a multi-page basin
in a tight loop. Stream trim and delete-on-empty record `Ok` as
completed and transaction conflicts as blocked, as before.
Closes#777.
---------
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Shikhar Bhushan <shikhar@schmizz.net>
## Summary
`CompressionAlgorithm::from_accept_encoding` dropped everything after
`;` in each `Accept-Encoding` entry without reading it. As a result, a
coding the client explicitly refused with a zero weight was still
selected:
| `Accept-Encoding` | before | after |
| --- | --- | --- |
| `zstd;q=0, gzip` | `Zstd` | `Gzip` |
| `gzip;q=0` | `Gzip` | `None` |
| `zstd;q=0, gzip;q=0` | `Zstd` | `None` |
RFC 9110 §12.4.2 defines `q=0` as "not acceptable". Because of this bug,
s2s read and append session frames of 1 KiB or more were compressed with
the exact coding the client had refused. The Rust SDK decodes either
codec, so it isn't affected in practice. A client that leaves out a
decompressor and says so with `q=0` gets frames it cannot decode.
With this change, codings whose `q` parameter is zero are skipped.
Non-zero weights keep the existing zstd-over-gzip preference, and
`gzip;q=0.8, deflate` still selects gzip.
## Tests
- `api`: `from_accept_encoding_skips_refused_codings` (rstest, 5 cases).
Before the fix, 4 cases fail (for example `left: Zstd, right: Gzip`).
After the fix, all pass.
- `lite`: `s2s_read_does_not_compress_with_refused_encodings` appends a
4 KiB record, reads it over `s2s/proto` with `Accept-Encoding: zstd;q=0,
gzip;q=0`, and checks the compression bits of the frame flag byte.
Before the fix: `left: 1 (zstd), right: 0`. After the fix it passes, and
the record round-trips.
- `cargo test --locked -p s2-api -p s2-lite`: all pass.
- `cargo +nightly fmt --all` is clean. `cargo clippy -p s2-api -p
s2-lite --all-targets -- -D warnings --allow deprecated` is clean (run
without the `codegen` feature because `protoc` isn't installed locally).
Note: #550 also touches this function (feature-gating the codecs). The
two changes are independent, but one of them will need a trivial rebase.
This fix and its tests were prepared with AI assistance (Claude). I
reviewed and ran them locally.
---------
Co-authored-by: breken-ai <312387581+breken-ai@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
## Summary
The API defines a scope resource set of `{"exact": ""}` as "match no
resources". The SDK decodes it into `BasinMatcher::None`,
`StreamMatcher::None`, and `AccessTokenMatcher::None`. The CLI's SDK→CLI
conversions turned each of those into an empty **prefix**, which means
the opposite: "match everything". So for a token that grants no basin,
stream, or token access:
- `s2 list-access-tokens` printed `basins=* streams=* tokens=*`.
- The JSON output reported `{"prefix": ""}`.
This makes a no-access token look like a full-access one. `s2 apply`
already renders the same scope correctly as `none` (`cli/src/diff.rs`).
This change maps a match-none matcher to an unset matcher
(`Option::None`). The CLI already renders an unset matcher as `∅` in the
summary and serializes it as `null`. That is the same shape s2-lite
returns, since it omits a match-none resource set.
## Tests
- `types::tests::match_none_scope_matchers_render_as_no_access`
deserializes the wire scope
`{"basins":{"exact":""},"streams":{"exact":""},"access_tokens":{"exact":""}}`
through the SDK into the CLI's `AccessTokenInfo`. It then checks the
list summary line and the JSON.
- Before the fix: `basins=* streams=* tokens=* perms=none ops=0`.
- After the fix: `basins=∅ streams=∅ tokens=∅ perms=none ops=0`, with
the three fields `null`.
- `cargo test --locked -p s2-cli --bin s2 --test cli`: all pass.
- `cargo +nightly fmt --all` is clean. `cargo clippy --locked -p s2-cli
--all-features --all-targets -- -D warnings --allow deprecated` is
clean.
This fix and its test were prepared with AI assistance (Claude). I
reviewed and ran them locally.
Co-authored-by: breken-ai <312387581+breken-ai@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Delete-on-empty could delete a stream before an increased `min_age`
elapsed, or discard its last deadline while records were still alive
after a retention change. Retain one authoritative DOE schedule per
stream so deferred deletion always preserves future work.
- Add separate key spaces for per-stream `Scheduled`/`Parked` state and
the time-ordered check queue. Ordinary appends no longer read or write
DOE schedules, and the refresh bookkeeping and historical `min_age`
cutoffs are removed.
- Keep deletion serialized through the streamer, checking the current
minimum age, stream incarnation, configuration revision, and stable
tail. A nonempty observation remains useful despite pending or
concurrent appends: defer to a record's stored expiration or park behind
a record without a TTL. Empty results still require stable-tail checks
before deletion. Configuration-revision mismatches retry in ten minutes
without relying on a possibly stale minimum age.
- Observe state, mapping, and metadata through a read-only snapshot. A
normal deferred check uses one serializable completion transaction to
validate the revision, consume its ticket, and install its successor.
Successful deletion completion only needs to revalidate the scheduler
state. Obsolete-work cleanup also revalidates its snapshot before
removing state.
- DOE configuration changes and completed regular trims coalesce with an
earlier check while advancing the state's revision, so an in-flight
worker cannot consume or postpone a concurrent wake. Partial trims wake
parked streams too. Trim scheduling normally reads only DOE state; if a
full trim finds no state, it checks current metadata and initializes
scheduling for enabled streams. This preserves the upgrade path for
infinite-retention streams whose last legacy deadline was already
consumed.
- Process due checks and legacy migration in pages of up to 10,000 keys,
with four concurrent workers. Use legacy deadline keys only for bounded,
idempotent migration, including future deadlines. A consistent snapshot
identifies cleanup-only streams, whose legacy keys share one write batch
per page. Streams needing initialization revalidate eligibility
transactionally and install state atomically with legacy-key removal.
Existing new state and its revision are untouched.
- Expected transaction conflicts leave affected work pending while the
rest of the page finishes. Backlog processing continues when at least
one item succeeds; a fully conflicted page waits for the next tick
rather than retrying in a tight loop. Other storage errors still fail
the tick. Configuration APIs retain retryable transaction-conflict
responses.
Enabling DOE or changing `min_age` on an existing stream requests
evaluation at the next tick. An empty stream whose last write is already
old enough can therefore be deleted then, without the previous
retention-plus-age scheduling delay. With unchanged finite retention and
no trims, deletion eligibility is based on approximately `max(retention,
min_age)` since the last write, subject to the ten-minute minimum retry
interval and background-tick granularity.
Migration removes legacy deadlines, and older binaries do not consume
the new scheduler state: downgrading does not preserve migrated
schedules. There is no metadata-wide backfill. An upgraded stream
without any legacy deadline gains state through a full trim or a DOE
configuration change; otherwise it remains unscheduled.
Regression coverage includes both configuration APIs and maximum-age
boundaries, stored expirations across retention changes, pending and
concurrent appends after the minimum age has elapsed,
trim/configuration/recreation races, full-trim recovery without legacy
deadlines, transactional wake preservation, draining migration pages
through the background loop, batched cleanup and snapshot revalidation,
durable terminal trim, and encoding/queue-boundary properties.
Validation:
- `just fmt`
- `just test --status-level fail --final-status-level fail
--no-fail-fast` — 886 tests passed across 12 binaries; the recipe
excludes Docker-backed and live integration suites.
- `just clippy`
Fixes#639.
Supersedes #769.
## Summary
Consider:
- user is appending with retry policy that retries all failures (even
indefinite)
- attempt 1 fails, with indefinite error (e.g. unavailable)
- attempt 2 fails, this time with definite error (e.g. aborted)
Right now, we'd return the final error, which is definite. This gives
the impression that no side effect was possible from the entire op,
across all attempts, but actually is only about the final attempt.
The fix tracks uncertainty across attempts using shared internal
helpers. If the final error is definite but an earlier attempt may have
had side effects, return `IndefiniteFailure { final_attempt_error }`
holding the final definite error. The entire append operation remains
indeterminate. This preserves the final attempt's diagnostic and
retryability while keeping `has_no_side_effects()` false for the logical
append. An already indefinite final error is returned directly, and
successful retries still succeed. Sessions track uncertainty per
unresolved batch.
Helper unit tests cover uncertainty wrapping, error classification, and
access to the final attempt's error.
Link to Devin session:
https://app.devin.ai/sessions/cbd9b244893a445ea0a9a49dc0cd0fe9
Open in Devin Desktop:
https://app.devin.ai/desktop/session/cbd9b244893a445ea0a9a49dc0cd0fe9?variant=devin
Requested by: @sgbalogh
---------
Co-authored-by: Stephen Balogh <stephen@s2.dev>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The `SL8_FLUSH_INTERVAL` comment in README.md stated the default was
"50ms
for remote bucket / 5ms in-memory", but after the separate WAL object
store
landed, the default follows the WAL store type when one is configured. A
reader using `--bucket` (S3) with `--wal-local-root` (local) would
wrongly
assume a 50ms default when it is actually 5ms.
Introduced by commit 1e954f014a
(@infiniteregrets, #766)
---
_Doc Drift PRs can be [configured
here](https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/settings/repos/repo_c4bd6a47-9b7d-4b62-9c18-8cf0ac18a8f9/doc-drift)._
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Bounded SSE reads could exceed their original count or byte limit after
repeated reconnects: the resume path subtracted cumulative progress from
the original budget, but emitted IDs reset progress on each connection.
Seed the emitted counters from `Last-Event-Id` when adjusting the read
bounds, and use saturating addition for client-supplied counters. This
matches the fix already on cloud main in
[s2-cloud#1778](https://github.com/s2-streamstore/s2-cloud/pull/1778).
Document the cumulative meaning of both counters on the shared API type.
Handler tests reconnect with unchanged bounds and actual emitted IDs
through three deliveries, compare against an uninterrupted read, and
verify that another resume delivers nothing after exhaustion. Coverage
includes count-only, bytes-only, both limits with either one reached
first, and near-maximum counters on bounded and unbounded reads.
Validation:
- `just fmt`
- `just test`: 843 passed
- `just clippy`
- Restoring the old counter reset makes all four reconnect regression
cases fail.
Closes#745.
---------
Co-authored-by: Stephen Balogh <stephen@s2.dev>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: shikhar <shikhar@s2.dev>
Removes the WAL storage warning block from the README.
Made with [Cursor](https://cursor.com)
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Drops the leading sentence of the WAL storage warning in the README and
rewords the remainder to stand alone.
Made with [Cursor](https://cursor.com)
Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up to #766. Exposes the `s2 lite` storage flags the chart was
missing, and folds the README feedback from that review into a combined
storage section.
**Chart.** The main store is now `objectStorage` (`--bucket`) or
`persistentVolume` (`--local-root` on a PVC), and the WAL is
`walStorage.bucket` (`--wal-bucket`) or `walStorage.persistentVolume`
(`--wal-local-root` on a PVC). The two volume blocks share one shape
(`enabled`, `mountPath`, `size`, `storageClass`, `existingClaim`) and
one `pvc.yaml` template; claims are named `<fullname>-data` and
`<fullname>-wal` and default to `/data` and `/wal`.
`walStorage.endpoint`/`region` map to
`S2LITE_WAL_AWS_ENDPOINT_URL_S3`/`S2LITE_WAL_AWS_REGION`; separate
credentials go through `env`, as for the main store. Templates fail on
`objectStorage` + `persistentVolume` both enabled, on `walStorage`
without a persistent main store, and on both WAL modes at once.
`NOTES.txt` reports the storage and WAL locations. The deployment
already uses `strategy: Recreate`, so `ReadWriteOnce` claims do not
block upgrades.
**README.** Replaces the "Separate WAL storage" section with a "Storage"
section presenting main-store and WAL settings side by side in one table
(bucket/directory, endpoint, region, credentials, session token). The
chart README links there instead of repeating it.
Both READMEs and `values.yaml` note that the WAL location must stay the
same across restarts, since changing it does not migrate data and the
server does not error.
**Merge after the next lite release.** `walStorage` passes
`--wal-bucket`/`--wal-local-root`, which the current default image
(`appVersion` 0.42.12) does not have; #766 ships in the next release.
Wait for its `appVersion` bump commit on `main` before merging.
`persistentVolume` alone works on 0.42.12.
Chart-created PVCs are deleted by `helm uninstall`; both volume blocks
expose `annotations` for `helm.sh/resource-policy: keep`, documented in
the chart README.
Chart `version` is not bumped; the `Bump Chart Version` workflow handles
that before release.
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Deleting and recreating a stream under the same name could let a delayed
deletion request, background trim job, or delete-on-empty (DOE) deadline
modify the recreated stream. A crash between terminal trim and metadata
marking also allowed provisioning in ensure mode to acknowledge an
update that pending cleanup would erase, while independent
initialization reads could revive a deleted stream.
- Read initialization state from one durable snapshot and reject
deletion-pending metadata. Finish shared initialization even if every
caller cancels, so snapshots are released and failed initialization
slots are removed. Report a missing live-stream ID mapping as a storage
invariant error instead of aborting the process.
- Reject provisioning and configuration updates once terminal trim has
begun. Stream deletion uses the append path to persist the terminal trim
command and marker together. Repeated deletion requests, late DOE
checks, and append rejections reporting deletion pending wait for the
original terminal append to become durable through the same
acknowledgement queue. Dropped deletion replies report an indeterminate
outcome because the write may have completed. Recheck the terminal trim
marker transactionally before marking metadata, and wait for the
transaction's read sequence to become durable if the trim worker has
already removed it.
- Bound each trim job by the commit sequence of the marker it scanned.
Record deletion stops before records newer than that marker, and the
existing finalization transaction clears only that marker version. Stale
work cannot delete records from a recreated stream or finalize its
deletion. Bulk record deletes stay in ordinary write batches, with no
additional reads; finite trims that empty a stream arm DOE in the
finalization transaction.
- Use the ID mapping's creation sequence to reject earlier streams' DOE
deadlines, including recreation between eligibility lookup and streamer
delivery. Give every new DOE schedule a random 128-bit key suffix.
Cleanup deletes the exact scanned keys with an ordinary write batch,
bounded by the 10,000-row scan limit, without per-deadline rereads or
cleanup transactions. A later schedule survives even if it has the same
stream and deadline.
Existing deadline keys remain readable and need no rewrite; new
schedules never overwrite them. New deadline keys are 16 bytes longer
and cannot be decoded by older binaries. Other persisted formats are
unchanged. DOE changes only address lifecycle races; retention
scheduling and min_age policy are unchanged.
Regression tests cover the split deletion state, durability of deletion
replies and append rejections, delayed deletion completion, stale trim
work across recreation, and initializer cancellation. The lifecycle test
checks both the recreated stream's records and its terminal trim marker,
which has the same trim value as the original stream's marker. It fails
without the record sequence bound and without the marker version guard.
DOE tests cover stale work with both key formats and re-arming during
cleanup; codec tests cover both formats and expired-range boundaries.
The repeated deletion request regression fails before its fix. Mutation
checks also confirm the DOE tests catch stale-generation work and reused
scheduling keys.
Closes#627Closes#628Closes#734
Validation: `just fmt`, `just test` (833 passed), `just clippy`, and
`RUST_LOG=trace just sim meta smoke --seed 1` with DOE temporarily
enabled in the smoke fixture (both runs succeeded with identical traces;
fixture restored afterward).
Lite stores WAL writes and LSM data in the same object store, so WAL
latency can be affected by memtable flushes and compaction output. Add
`--wal-bucket` and `--wal-local-root` to configure a dedicated WAL store
through SlateDB's existing builder API. The README leads with a local
filesystem WAL and a remote S3 bucket for the main database.
`--wal-bucket` reuses the main S3 connection configuration by default,
including its endpoint, region and credentials. Optional
`S2LITE_WAL_AWS_*` overrides select a different server or credentials;
the WAL endpoint takes precedence over an inherited
`AWS_ENDPOINT_URL_S3`. Both buckets use the same S3 builder; a
WAL-specific key pair replaces the complete credential set, including
its optional session token. Omitting both WAL selectors preserves the
shared-store default.
Filesystem WAL storage retains fsync. The default flush interval follows
the WAL store type, and `SL8_FLUSH_INTERVAL` still takes precedence.
Separate WAL storage requires an explicitly configured persistent main
store. Both stores use `--path` and must be reopened at the same
locations; these options do not migrate existing WAL data.
Validation: `just fmt`, `just test` (840 passed), and `just clippy`,
using locked dependencies. Tests cover CLI constraints, inherited S3
connection settings, generic and S3-specific endpoint inheritance,
endpoint-only and credential overrides, session-token isolation,
physical file routing, and recovery of acknowledged records after
killing and restarting S2. No dependency changes. No performance
improvement is claimed.
Fixes#673.
## Problem
`read_start_seq_num` rejects a read that starts at the tail and cannot
follow, returning `UnwrittenError` (HTTP 416). The check matched only
`ReadPosition::SeqNum`, and it ran *before* the
`ReadPosition::Timestamp` arm resolved its start through
`resolve_timestamp(..).unwrap_or(tail)`. A timestamp start that resolves
to the tail therefore slipped past it.
On an empty stream, the two spellings of the same read disagree:
```
GET /v1/streams/{stream}/records?seq_num=0 -> 416, tail {0,0}
GET /v1/streams/{stream}/records?timestamp=0 -> 200, {"records":[]}
```
The `200` also contradicts the documented meaning of an empty unary
batch, which is that an explicit `count`, `bytes`, or `until` bound
could not be satisfied.
The same disagreement appears whenever a timestamp start resolves to the
tail, not only on an empty stream.
## Fix
Resolve the start position to a sequence number first, then apply one
at-tail check to the resolved value.
Clamped starts are unaffected: `clamp` already rewrites the position to
`tail.seq_num` before this point, so it reaches the check exactly as it
did before. Timestamp starts that resolve behind the tail are unaffected
too.
## Tests
`test_read_at_tail_of_empty_stream_returns_unwritten` covers both
spellings on an empty stream. With the fix reverted, the `seq_num` case
passes and the `timestamp` case fails, so it pins the behaviour rather
than restating it.
`cargo nextest run -p s2-lite` passes 255/255, and `cargo fmt --check`
plus `cargo clippy --all-targets -- -D warnings` are clean.
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: shikhar <shikhar@s2.dev>
`PendingAppends::on_stable` shrank the acknowledgement queue to zero
whenever it drained. A single append therefore freed a four-slot buffer
and forced the next append to allocate again. The streamer also advances
durability one append at a time, so a 16-append burst began shrinking
before it finished draining.
Keep small queue allocations for reuse, with a minimum shrink target of
16 senders. Queues still start unallocated, and larger backlogs retain
the existing policy of shrinking at one-quarter occupancy to twice the
remaining length. Once drained, they retain space for at most 16
senders.
Regression tests exercise repeated single-append and 16-append bursts,
reclamation after a 128-append burst, and acknowledgement delivery only
after durability advances. Against the original code, the reuse tests
fail with capacities `0` versus `4` and `8` versus `16`; all four tests
pass with the fix. This establishes allocation reuse; end-to-end
throughput impact has not been measured.
Validation:
- `just fmt`
- `RUSTUP_TOOLCHAIN=stable cargo test --locked -p s2-lite --lib
--all-features backend::append::tests` — 4 passed.
- `RUSTUP_TOOLCHAIN=stable just test` — 813 passed.
- `RUSTUP_TOOLCHAIN=stable just clippy` — passed with warnings denied.
Append persistence serializes record data, timestamp indexes, and
metadata into owned `Bytes`, then passes them through SlateDB's
`put`/`put_with_options` APIs, which copy both keys and values again.
Use `put_bytes`/`put_bytes_with_options` to transfer those buffers
directly into the write batch while preserving the existing TTL options
and durable acknowledgement path.
Validation:
- `just fmt` passed.
- `RUSTUP_TOOLCHAIN=stable just test` passed: 809 tests.
The redundant allocations and copies are removed; the end-to-end
throughput impact has not been measured.
Active streamers could apply configuration notifications out of order or
miss a committed update when its request was cancelled or the streamer
was still initializing. This could leave them enforcing outdated
retention or timestamping settings indefinitely.
Use durable metadata commit sequences to apply only newer
configurations, starting with the sequence read alongside the initial
config. An owned task completes each stream-config commit and its
notification even if the caller cancels. Initializing streamers retain
the newest pending configuration and enqueue it when publishing the
ready client, under the same slot lock. Metadata is durable before
acknowledgment, while streamer initialization and configuration
application remain asynchronous.
Shared read helpers centralize value, sequence, and timestamp decoding
while preserving durable reads and transaction snapshots. The existing
Ensure/PATCH integration tests now cover cancellation before flush, and
a focused initialization test verifies delivery of the newest queued
configuration. Removing the fixes makes all three regression cases fail;
the two normal update cases still pass. The retention regression also
checks that delayed notifications cannot restore an older TTL policy.
Validation: `just fmt`, `just test` (809 passed), `just clippy`, and
`RUST_LOG=trace just sim meta smoke --seed 1` (two successful runs with
identical traces).
A delete retry could observe an unflushed deletion marker and return
success while durable metadata still described the basin or stream as
active. Wait for the existing marker's commit sequence on the idempotent
path, and retrigger basin cleanup after that wait so a cancelled first
request does not suppress the notification.
For streams, the terminal trim was already durable; this also makes the
metadata marker durable before the delete response.
Validation: `just fmt`, `just test` (804 passed), and `just clippy`. Two
focused control-plane tests retry a cancelled basin or stream deletion
while its metadata marker remains unflushed, using disabled automatic
flushing and paused time. Both fail on the base commit and pass with
this fix.
Related independent durability fixes:
[#757](https://github.com/s2-streamstore/s2/pull/757),
[#759](https://github.com/s2-streamstore/s2/pull/759). All three combine
without conflicts; combined validation passes `just fmt`, `just test`
(810 passed), `just clippy`, and simulator smoke/trace determinism with
seed 1.
Cancelling an append after its write reaches memory can drop the
streamer's last client lease while that write is still awaiting
durability. After the idle timeout, a replacement streamer would recover
the older durable tail and reuse the cancelled append's sequence number.
Keep the streamer alive while writes are queued or in flight. Once those
writes settle, normal dormancy can resume and any replacement can
recover their positions from durable storage.
Validation: `just fmt`, `just test` (803 passed), and `just clippy`. A
focused streamer lifecycle test disables automatic flushing, cancels an
accepted append, and advances past the actual dormancy timeout with no
client leases. It verifies that the streamer stays alive until the write
is durable and exits normally after flushing. Restoring the original
idle-exit condition makes the test fail.
Related independent durability fixes:
[#756](https://github.com/s2-streamstore/s2/pull/756),
[#759](https://github.com/s2-streamstore/s2/pull/759). All three combine
without conflicts; combined validation passes `just fmt`, `just test`
(810 passed), `just clippy`, and simulator smoke/trace determinism with
seed 1.
The publication cooldown can reject the first release fixing a security
advisory while `cargo deny` rejects the older vulnerable release. This
adds exceptions for reviewed security updates, matched to exact
crate/version pairs, and updates S2 to the approved Rustls release.
`security-exceptions.toml` records the crate, exact version, advisory,
and reason. The gate validates entries, checks that publication metadata
exists, and prints the justification when it waives publication age.
Other versions retain the normal cooldown. The initial entry approves
`rustls 0.23.45` for
[RUSTSEC-2026-0285](https://rustsec.org/advisories/RUSTSEC-2026-0285.html).
Exceptions ship with the shared action; consumers pick them up by
updating their pinned action or reusable-workflow commit. The README
also documents the command-scoped Cargo resolver override needed to
select an approved fresh release. Existing first-party exemptions and
other dependency checks continue to apply.
### Dependency changes
The targeted nightly Cargo update changes four transitive package
versions, with no manifest changes:
| Crate | Before → after | Upstream changes and risk |
| --- | --- | --- |
| rustls | 0.23.43 → 0.23.45 | Fixes accepting TLS 1.3 handshake
messages at the wrong encryption level; raises AWS-LC and webpki
dependency floors. [Release
notes](https://github.com/rustls/rustls/releases/tag/v/0.23.45). |
| aws-lc-rs | 1.17.3 → 1.18.1 | Stabilizes ML-DSA APIs and tightens
validation of invalid crypto inputs.
[1.18.0](https://github.com/aws/aws-lc-rs/releases/tag/v1.18.0),
[1.18.1](https://github.com/aws/aws-lc-rs/releases/tag/v1.18.1). |
| aws-lc-sys | 0.43.0 → 0.45.0 | Updates bundled AWS-LC from 5.2 to 5.7,
including native crypto/build changes and padded-decryption output
handling. Largest runtime/build change in this update. [Wrapper
notes](https://github.com/aws/aws-lc-rs/releases/tag/v1.18.1), [AWS-LC
5.7](https://github.com/aws/aws-lc/releases/tag/v5.7.0). |
| rustls-webpki | 0.103.13 → 0.103.15 | Uses stabilized ML-DSA APIs; the
final patch fixes documentation builds.
[.14](https://github.com/rustls/webpki/releases/tag/v/0.103.14),
[.15](https://github.com/rustls/webpki/releases/tag/v/0.103.15). |
Rustls requires the newer AWS-LC/webpki dependency lines. The selected
transitive versions have already completed the cooldown; only Rustls
needs the exception. Cargo also re-resolves some Windows and tempfile
dependency edges to versions already present in the lockfile.
### Validation
- 15 deterministic Python tests pass, covering exact matching, other
fresh dependencies, malformed and duplicate entries, publication
metadata, and existing cooldown behavior. A dedicated workflow runs
them.
- The gate passes against S2's four new package versions and against
cachey PR #147; in both cases only `rustls 0.23.45` uses the exception,
with the advisory and reason printed.
- After the dependency update: `just fmt`, `just test` (796 passed),
locked workspace Clippy with all features/targets and warnings denied,
`cargo deny check`, and `git diff --check` pass.
Related: https://github.com/s2-streamstore/cachey/pull/147
## Summary
Reject `Content-Type` in `S2Config::with_default_headers`, alongside the
existing encoding/framing restrictions. The SDK chooses the protocol for
each operation: a default `Content-Type: s2s/proto` can make a unary
read receive streaming frames, causing a decode error or timeout.
This addresses #737 at configuration validation, as an alternative to
the per-read override in #738. All `Content-Type` values are rejected
because request format belongs to the SDK.
Extend the existing rejection tests to cover S2S, protobuf, JSON,
mixed-case header names, and empty values. Remove the now-invalid
Content-Type default from the header propagation fixture while
preserving its assertions.
Validation: the five new cases failed before the fix. `just test` passes
all 796 workspace tests; SDK clippy with all features/targets and `just
fmt` also pass.
Closes#737
Link to Devin session:
https://app.devin.ai/sessions/c3dea6e292ce4071a41baf4943f1557c
Open in Devin Desktop:
https://app.devin.ai/desktop/session/c3dea6e292ce4071a41baf4943f1557c?variant=devin
Requested by: @sgbalogh
Co-authored-by: Stephen Balogh <stephen@s2.dev>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
## Summary
When a basin has `create_stream_on_append` enabled, an append can carry
an `s2-stream-config` header whose value is a compact JSON
`StreamConfig`. If that request is the one that creates the stream, the
config is layered over the basin's `default_stream_config`; unset fields
inherit the defaults. It is ignored once the stream exists, so clients
can attach it to every append without tracking whether the stream has
been created.
This gives per-stream config (e.g. retention, delete-on-empty) on
auto-created streams without a control-plane round trip.
Spec half: s2-streamstore/s2-specs#21 (this PR bumps the `api/specs`
submodule to that branch's commit; re-bump to the merge commit once it
lands).
## API
One header, same for JSON, proto and S2S (an append session is a single
request, so the header covers the whole session):
```sh
curl -X POST "https://$BASIN.b.s2.dev/v1/streams/tenant-42%2Fevents/records" \
-H "Authorization: Bearer $S2_ACCESS_TOKEN" \
-H "Content-Type: application/json" \
-H 's2-stream-config: {"retention_policy":{"age":3600},"delete_on_empty":{"min_age_secs":300}}' \
-d '{"records": [{"body": "hello"}]}'
```
The value is validated exactly like a `CreateStream` config; an invalid
value is rejected with `400 bad_header` before any lookup and no stream
is created:
```json
{"code":"bad_header","message":"Invalid header `s2-stream-config`: age must be greater than 0 seconds"}
```
SDK: the option lives on the stream handle, like the encryption key, and
applies to unary appends, append sessions and producers:
```rust
let stream = basin
.stream(name)
.with_stream_config(
StreamConfig::new()
.with_retention_policy(RetentionPolicy::Age(3600))
.with_delete_on_empty(DeleteOnEmptyConfig::new().with_min_age(Duration::from_secs(300))),
);
stream.append(input).await?; // or
stream.producer(ProducerConfig::default()); // or stream.append_session(..)
```
CLI:
```sh
echo hello | s2 append s2://my-basin/tenant-42/events --format text \
--retention-policy 1h --delete-on-empty-min-age 5m
```
## Why a header
- No proto change: the same header carries the config for unary appends
and S2S sessions, so `AppendInput` (which flows into storage) stays
untouched.
- Known before the server responds. With a body/frame field, S2S
sessions needed the server to wait for the first frame before creating
the stream, while clients wait for response headers before sending it;
the header removes that ordering problem entirely.
- Reusable for read paths (`create_stream_on_read`) later, since `GET`
has no body.
## Changes
- **api**
- `v1::config::STREAM_CONFIG_HEADER` (`s2-stream-config`) and
`StreamConfigHeader`, a `ParseableHeader` that deserializes the JSON
`StreamConfig` and reuses `TryFrom<StreamConfig> for
OptionalStreamConfig` so validation lives in one place.
`to_header_value` for clients.
- `data::S2StreamConfigHeader` documents the header in OpenAPI (string
schema, with an example value; utoipa cannot express `content` on a
parameter).
- `AppendRequest::Unary` / `S2s` gain `stream_config:
OptionalStreamConfig`, parsed once in the extractor.
- **lite**
- `stream_handle_with_auto_create` takes an `AutoCreateOn` (`Append` /
`Read`) and the `OptionalStreamConfig` to layer over the basin defaults
when creating.
- `Backend::open_for_append(.., stream_config)` serves both unary
appends and sessions; the stream is created (or the request fails)
before the response, as before this feature.
- **sdk**: `S2Stream::with_stream_config`, mirroring
`with_encryption_key`. Internally, `AppendHeaders { encryption,
stream_config }` is threaded through sessions/producers and set on every
(re)connect.
- **cli**: `s2 append` accepts the same stream config flags as
`create-stream` (`--retention-policy`, `--storage-class`,
`--timestamping-*`, `--delete-on-empty-min-age`), listed under their own
help heading; set on the stream handle.
## Compatibility
- Old clients never send the header; old servers ignore unknown headers.
- `s2-api` public API change: `AppendRequest` variants gain a field.
## Testing
- `s2-api` unit: header parse/validate (valid, `{}`, invalid JSON, `age:
0`) and `to_header_value` roundtrip.
- Backend-level: applies + merges with basin defaults; existing stream
ignores config.
- HTTP-level: JSON unary with header; invalid header -> `400 bad_header`
with no stream created (both invalid config and non-JSON); S2S session
with header.
- SDK integration against `s2 lite`: unary + producer create with
config, existing stream unchanged.
- CLI integration against `s2 lite`: 47/47 pass.
- `clippy -D warnings` clean; workspace unit suites pass.
Made with [Cursor](https://cursor.com)
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Allow a set of additional `default_headers` to be specified. This will
be present on all requests, unless replaced by the SDK.
Setting content-encoding headers is not supported, SDK needs full
control of that.
This is motivated by an internal (s2 cloud) usecase, hence the gate
under `_hidden`.
## Summary
Reverts the `s2_api::data::Json` / `JsonOpt` axum extractor from
`sonic_rs::from_slice` (#383) back to `serde_json::from_slice`, and
drops the `sonic-rs` dependency.
**Why:** sonic-rs has no recursion bound on its *skip* path. When the
deserializer meets a value it doesn't want — an unknown field, or a
wrong-typed value such as `"body": [[[[…` where a `String` is expected —
it walks to the end of that value via `Parser::skip_one(true)` →
`skip_array`/`skip_object` → `skip_one` …, recursing once per nesting
level with no counter. (Its `MAX_ALLOWED_DEPTH = 255` guard from
cloudwego/sonic-rs#213 only covers the visitor path.) A request body
nested ~20k+ levels deep therefore overflows the tokio worker stack and
aborts the whole frontend process:
```
thread 'tokio-rt-worker' has overflowed its stack
fatal runtime error: stack overflow, aborting
```
Reported upstream as cloudwego/sonic-rs#232 (open, reproduces on
0.5.9/main).
serde_json is safe on every such path: type mismatches error out before
descending (`invalid type: sequence, expected a string` → 422),
`deserialize_ignored_any` skips iteratively with a heap stack, and
values that are actually deserialized hit the default 128-level
recursion limit (`recursion limit exceeded` → 400).
**Error classification** keeps the same 400/422/500 split:
```rust
serde_json::error::Category::Data => DataError (422)
serde_json::error::Category::Io => Other (500)
serde_json::error::Category::Syntax | Eof => SyntaxError (400)
```
**Performance:** measured on `AppendInput` in release builds (x86-64,
sonic-rs built with `-C target-cpu=native` so it gets AVX2), serde_json
was on par or faster than sonic-rs for realistic append bodies — 371 vs
459 µs for 1000×100 B records with headers, 1.66 vs 2.09 ms for 1000×1
KiB records with escapes, 132 vs 105 µs for a single 1 MiB string.
**Tests:** `deeply_nested_json_does_not_overflow_stack` parses 50k-deep
nesting in `body`, in an unknown field, and at top level on a 2
MiB-stack thread; the old `serde_json_sonic_rs_roundtrip` differential
test is kept as a plain `serde_json_roundtrip`.
Companion change for s2-cloud: s2-streamstore/s2-cloud#1777 (will be
reduced to bumping `s2-api` and switching the OTLP extractor once this
is released).
Link to Devin session:
https://app.devin.ai/sessions/7c3250684bc84290abeeb13826313c4b
Open in Devin Desktop:
https://app.devin.ai/desktop/session/7c3250684bc84290abeeb13826313c4b?variant=devin
Requested by: @sgbalogh
---------
Co-authored-by: Stephen Balogh <stephen@s2.dev>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
## Summary
Black-box fuzzing of an S2 sandbox found that a NUL byte (0x00) in a
stream name — or in the list `prefix` / `start_after` query params —
surfaces as a generic `500 {"code":"other","message":"Internal Server
Error"}` instead of a structured 400:
```
POST /v1/streams {"stream":"a\u0000b"} -> 500 other
GET /v1/streams?prefix=a%00b -> 500 other
GET /v1/streams?start_after=a%00b -> 500 other
```
Every other control char (`\t`, `\n`, `\r`, `\x01`), arbitrary Unicode,
emoji, `/`, `?`, `#`, `%20`, etc. is accepted and round-trips fine; only
NUL breaks.
**Root cause:** `StreamNameStr::<T>::validate_str` in
`common/src/stream.rs` only checks non-empty, not `.`/`..`, and `len <=
MAX_STREAM_NAME_LEN`. Nothing rejects an interior NUL, so it passes
validation and reaches the metastore, where Postgres/DSQL rejects NUL in
a `TEXT` column and the error isn't mapped to a client error. Because
`NameProps`, `PrefixProps`, and `StartAfterProps` all share
`validate_str`, the name, `prefix`, and `start_after` paths are all
affected.
**Fix** (minimal; no other character is newly rejected):
```rust
// StreamNameStr::<T>::validate_str
if name.contains('\0') {
return Err(format!("stream {} must not contain NUL bytes", T::FIELD_NAME).into());
}
```
Applied identically to `AccessTokenIdStr::validate_str`
(`common/src/access.rs`), which mirrors the stream validator
rule-for-rule and had the same gap. `BasinNameStr` and `LocationName`
already enforce strict ASCII charsets that exclude NUL — no code change,
just added `nul` regression cases to lock it in.
Rejecting NUL narrows the documented contract ("between 1 and 512
bytes"), so the doc comments that feed the OpenAPI spec
(`CreateStreamRequest::stream`,
`ListStreamsRequest::{prefix,start_after}`,
`IssueAccessTokenRequest::id`,
`ListAccessTokensRequest::{prefix,start_after}`) and the SDK rustdoc on
the re-exported name/prefix/start-after types now say "must not contain
NUL bytes". The `api/specs` submodule is synced separately by the specs
workflow.
Since the fix is in the shared `s2-common` crate it covers both s2-cloud
and s2-lite. Verified against a locally built lite, which now returns
structured 400s:
```
create stream a\0b -> 400 bad_json "stream name must not contain NUL bytes ..."
?prefix=a%00b -> 400 bad_query "prefix: stream prefix must not contain NUL bytes"
?start_after=a%00b -> 400 bad_query "start_after: stream start-after must not contain NUL bytes"
create "a\tb/名前 😀?#" -> 201 (unchanged)
```
**s2-lite behaviour before this fix:** it did *not* 500. Running `main`
lite, all three requests succeeded (`201`/`200`) and the NUL name
round-tripped through list. SlateDB keys are raw bytes, and although
lite uses `\0` as the basin/stream separator in the `StreamMeta` key and
`StreamIdMapping` value, `deser_key` splits on the *first* `\0` and
basin names can never contain NUL, so the encoding stayed unambiguous.
The 500 is specific to s2-cloud's SQL-backed metastore; lite just
becomes consistent with cloud in rejecting NUL up-front.
**Tests:**
- Unit: `rstest` `nul` cases added to `validate_name_err` /
`validate_prefix_err` / `validate_start_after_err` (stream, basin,
access token) and `LocationName::validate_name_err`, plus
`control_chars` / `unicode` `validate_name_ok` cases guarding the
"nothing else newly rejected" intent.
- HTTP: new `handlers::v1::streams::test` module in lite (in-process
`Router::oneshot`, same pattern as the records handler tests) asserting
NUL in the create body → 400 `bad_json`, NUL in `prefix`/`start_after` →
400 `bad_query`, and control-char/Unicode names → 201. These fail if the
`validate_str` guard is removed.
`just clippy` and `just test` pass.
No version bumps; release is left to the normal release flow.
Link to Devin session:
https://app.devin.ai/sessions/503132b92ae34635b700d43f406c0ccd
Open in Devin Desktop:
https://app.devin.ai/desktop/session/503132b92ae34635b700d43f406c0ccd?variant=devin
Requested by: @sgbalogh
---------
Co-authored-by: Stephen Balogh <stephen@s2.dev>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Lite previously returned after draining HTTP requests without closing SlateDB, leaving memtables to be recovered from the WAL on the next startup.
Call `Backend::close()` after HTTP shutdown, delegating to SlateDB 0.15's `Db::close()` to flush memtables to L0 and shut down the database. Log the elapsed close time and propagate close errors. This reduces WAL replay on restart; Lite's existing `manifest_poll_interval` startup wait remains.
Shutdown awaits the database close without an application timeout. Deployments should allow enough termination grace for the final flush; an interrupted close may still leave WAL data to replay on the next startup.
Validation:
- `just fmt` and `just test` (744 tests passed).
- Local filesystem SIGTERM/reopen checks over HTTP and self-signed HTTPS preserved all 512 acknowledged records per scenario and accepted subsequent appends.
- The contributor reported restart-to-ready times of 2–6 seconds on Fly.io/Tigris, down from 106 seconds when quiet and 251–332 seconds after a burst, with a 1–2 second close and all 352 acknowledged records plus the seed record present after reopening. These remote-store timings were not independently reproduced during review.
**Detail bug report:** [View on
Detail](https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_1ccfeb5d-95f9-46a0-badf-3561947108b4)
Closes#715
## Bug
HTTP/2 `RST_STREAM` frames with `REFUSED_STREAM` were classified as
non-retryable `ClientError::Other` instead of retryable
`ClientError::ConnectionClosedEarly`. The h2 classification in
`classify_hyper_source` only handled `is_io() || is_go_away()`, missing
`RST_STREAM` resets entirely. Since RFC 9113 §8.7 guarantees a
`REFUSED_STREAM` is sent before any processing occurred (so the request,
including non-idempotent methods, can be safely retried), the client was
failing instead of retrying.
## Fix
Extracted the nested `h2::Error` handling in `sdk/src/error.rs` into a
`classify_h2_error` helper and added a branch that classifies any h2
error with `reason() == Some(h2::Reason::REFUSED_STREAM)` as
`ConnectionClosedEarly` (retryable). I/O and GOAWAY h2 errors keep their
existing classification; all other reasons (and unknown/future codes)
still fall through to non-retryable `Other`.
The check is **reason-based, not `is_reset()`-based**. h2 surfaces a
received `RST_STREAM(REFUSED_STREAM)` in two shapes: a `Reset`-kind
error while reading the response body (`is_reset()==true`), and a
`Reason`-kind error while sending the request body — hyper wraps the
reason from `SendStream::poll_reset` via `h2::Error::from(reason)`,
which has `is_reset()==false`. An `is_reset()`-only check would miss the
request-body path; keying on `reason()` covers both. Other `RST_STREAM`
reasons (`INTERNAL_ERROR`, `CANCEL`, `FLOW_CONTROL_ERROR`,
`STREAM_CLOSED`, …) may indicate partial processing and are
intentionally left non-retryable.
## Testing
- **Unit tests** (`sdk/src/error.rs`): 4 new tests covering
`classify_h2_error` — REFUSED_STREAM classifies as retryable
`ConnectionClosedEarly`; it does so even when `!is_io() && !is_go_away()
&& !is_reset()` (the request-body path); all other named reasons and
unknown codes stay non-retryable; and the `ConnectionClosedEarly` vs
`Other` retryability contract holds. The full SDK unit suite passes
(110/110).
- **End-to-end test** (`sdk/tests/refused_stream_retry.rs`): a
self-contained integration test that spins up an h2 prior-knowledge
server which `RST_STREAM(REFUSED_STREAM)`s the first request and serves
a valid `list_basins` response on the retry. The SDK retries and
succeeds, and the server is observed to receive ≥2 requests. This
exercises the full `HttpError → classify_hyper_source →
classify_h2_error → ClientError → retry loop` path that can't be
unit-tested (hyper/h2 errors have no public constructors). Reverting the
fix makes this test fail with the exact bug symptom `Client(Other("send
error: client error (SendRequest)"))`, confirming it guards against
regression.
- **Live server integration**: ran `stream_ops` + `basin_ops` +
`account_ops` (non-token) against a local `s2 lite` server (HTTP/2
prior-knowledge over cleartext) — all pass, including the
producer/append-session happy-path tests. `metrics_ops` and the
`account_ops` access-token management tests return `501 not_implemented`
from `s2 lite` (a known limitation the CI `sdk-tests.yml` skips via
`--skip access_token --skip metrics`); these run against a full S2
server in CI and are unrelated to this change.
- **Could not verify**: the bug report's Evidence 3 claims oversized
request headers trigger `REFUSED_STREAM`. In h2 0.4.16, oversized
headers actually yield an HTTP `431` response
(`proto/streams/recv.rs:207-234`), not a per-stream `REFUSED_STREAM`
(the cited `recv.rs:1041` is the `max_concurrent_streams` `refused`
path). The fix is reason-based and trigger-agnostic, so any genuine
`REFUSED_STREAM` source is covered identically by the unit and
end-to-end tests.
- Routine checks (typecheck, clippy with `-D warnings` across all
targets, `cargo +nightly fmt --all --check`, and `cargo doc` with `-D
warnings`) all pass.
---
_Automatic Fixes PRs can be [configured
here](https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/settings/repos/repo_c4bd6a47-9b7d-4b62-9c18-8cf0ac18a8f9/bugs)._
---------
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: Mehul Arora <mehul@s2.dev>
Adds a seven-day publication cooldown for newly locked crates.io
versions. Runs it as a reusable, credential-free PR gate and documents
the dependency update workflow.
---------
Co-authored-by: Codex GPT-5.6 Sol <noreply@openai.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-24 21:23:18 +03:00
121 changed files with 9774 additions and 3948 deletions
