diff --git a/alkstore-postgres/src/forwarder.rs b/alkstore-postgres/src/forwarder.rs index c47c107..7e1f8d2 100644 --- a/alkstore-postgres/src/forwarder.rs +++ b/alkstore-postgres/src/forwarder.rs @@ -81,10 +81,12 @@ pub(crate) type ReconnectConfigSlot = Arc>::Stream, diff --git a/alkstore-postgres/src/opts.rs b/alkstore-postgres/src/opts.rs index b47932e..194e4a3 100644 --- a/alkstore-postgres/src/opts.rs +++ b/alkstore-postgres/src/opts.rs @@ -36,7 +36,10 @@ pub(crate) const LISTENER_APPLICATION_NAME: &str = "alkstore-pg-listener"; /// [`Config`](::tokio_postgres::Config)-parseable form, ADR-008 §6's /// `open(url, PgOpts)` pin); the engine never invents defaults for /// them. `open` fails with [`Error::Database`] if the config is -/// unparseable or the server unreachable. +/// unparseable or the server unreachable. TLS: the engine hardwires +/// `NoTls` on every connection path regardless of the config's +/// sslmode — v1 TLS is a post-v1 deployment concern +/// (deployment.md's TLS posture carries the statement). #[derive(Debug, Clone)] pub struct PgOpts { /// The engine-owned PostgreSQL schema (ADR-010 §8). Default diff --git a/alkstore/src/opts.rs b/alkstore/src/opts.rs index 0c94861..fed23be 100644 --- a/alkstore/src/opts.rs +++ b/alkstore/src/opts.rs @@ -49,6 +49,15 @@ pub struct EnqueueOpts { /// attach to every job row at enqueue — claims, heartbeats, retries, /// and sweeps read the job's own stamps, never a live registry. /// Opts changes apply to *future enqueues only*. +/// +/// The numeric fields are trusted as given, engine-side — no domain +/// validation (ADR-023 §2's domain table covers the trait-surface +/// arguments, not these consumer-constructed constants; a +/// consumer-obligation note, not a defect). A non-positive +/// `visibility_timeout_s` stamps claims instantly reclaimable, and +/// `max_attempts <= 0` stamps rows that are never claimed — see +/// deployment.md's consumer-obligation notes for the per-field +/// symptoms. #[derive(Debug, Clone, PartialEq, Eq)] pub struct QueueOpts { /// Claim visibility timeout in seconds — the default claim diff --git a/docs/architecture/deployment.md b/docs/architecture/deployment.md index ae4bf27..bc851d9 100644 --- a/docs/architecture/deployment.md +++ b/docs/architecture/deployment.md @@ -1,6 +1,6 @@ --- status: draft -last_updated: 2026-10-07 (ADR-021 — third review round: drop=rollback keeps pg pool accounting exact under error paths) +last_updated: 2026-10-10 (docs alignment — v1 TLS posture stated; QueueOpts numeric consumer-obligation notes) --- # Deployment @@ -82,6 +82,60 @@ engine-owned PostgreSQL schema (default `alkstore`, per-engine option) — layout per [ADR-010](decisions/010-queue-semantics-depth.md) §8 (resolved from [queues.md](queues.md)'s namespace bullet). +### TLS posture (v1) + +The pg engine hardwires `NoTls` on **every connection path** in v1 — +the pooled connections, the dedicated listener connection, and every +reconnect attempt alike; the engine does not ride the consumer's +`Config` sslmode setting, and a DSN carrying `sslmode=require` (or any +TLS demand) fails at connect. The listener's `NoTls` posture is +test-pinned in the type it carries +([ADR-004](decisions/004-postgres-driver.md)); v1 TLS should therefore +be treated as **effectively unavailable** — network confidentiality +between the process and the Postgres server must come from the +deployment topology itself (private network, egress rules, a local +unix socket or sidecar), not from driver TLS. Encrypting traffic +between engine and server is a **post-v1 deployment concern** (wire a +TLS connector through the pool and listener construction); until then +the boundary above is the honest statement, per +[ADR-016](decisions/016-deployment-honesty.md)'s spirit — the matrix +states the limitation rather than having code pretend otherwise. + +## Consumer-obligation notes on engine options + +Consumer-*constructed* numeric option values are **trusted as given — +the engine does not validate them against domain extents**. The +engine would be a second normative home for semantics the contract +deliberately leaves to the caller's constants; the +[ADR-023](decisions/023-fourth-review-round.md) §2 domain table +covers the trait-surface arguments, not these. This applies to the +queue stamps (`QueueOpts`: `visibility_timeout_s`, +`dead_letter_retention_s`, `max_attempts`) on both engines: + +- Negative or zero `visibility_timeout_s` stamps claims whose deadline + is already past — every claim is instantly reclaimable (the + dual-execution window is the consumer's documented budget + [ADR-010](decisions/010-queue-semantics-depth.md) §2). +- Zero or negative `max_attempts` stamps rows the claim statement + never hands out — each is dead-lettered with reason "max attempts + exceeded" at the next claim call on that queue (the pre-claim + sweep's `attempts >= max_attempts` arm), i.e. the queue silently + discards its work. +- Negative `dead_letter_retention_s` deletes every dead row at the + next `sweep_expired` call (retention is driver-free — nothing runs + without a caller). +- The stamps ride *future enqueues only* (`QueueOpts`'s documented + shape) — repair the opts before enqueueing rather than repairing + rows afterwards. + +The same trusted-as-given shape applies to `PgOpts::max_size`'s +*positive-integer* contract — 0 is typed-failed at open, the one knob +with an engine-side guard; everything above has none. The +consumer-obligation shape here is the visibility-budgeting precedent +([ADR-010](decisions/010-queue-semantics-depth.md) §2): the constant is +the consumer's, the behavior of a mis-set one is documented, nothing +runtime is fabricated. + ## Durability knobs | Engine | Knob | Shape | diff --git a/tasks/pg-fix-docs-alignment.md b/tasks/pg-fix-docs-alignment.md index bc3732a..9601a54 100644 --- a/tasks/pg-fix-docs-alignment.md +++ b/tasks/pg-fix-docs-alignment.md @@ -1,7 +1,7 @@ --- id: pg-fix-docs-alignment name: Doc alignment — TLS posture, QueueOpts consumer obligations (review 002 Finding 6 remainder) -status: pending +status: completed depends_on: [pg-fix-forwarder-reconnect] scope: single risk: trivial @@ -51,15 +51,15 @@ it and correct against the settled code. ## Acceptance Criteria -- [ ] `forwarder.rs`'s sslmode claim matches the code (`NoTls` +- [x] `forwarder.rs`'s sslmode claim matches the code (`NoTls` hardwired); no other doc in the crate repeats the claim -- [ ] `deployment.md` states the v1 TLS-unavailable posture on all +- [x] `deployment.md` states the v1 TLS-unavailable posture on all connection paths -- [ ] `deployment.md` (plus the opts' doc home) carries the +- [x] `deployment.md` (plus the opts' doc home) carries the QueueOpts numeric consumer-obligation note -- [ ] Cross-file doc sweep over the fix batch's touched files: no +- [x] Cross-file doc sweep over the fix batch's touched files: no doc-behavior mismatch remains (grep-auditable claims spot-checked) -- [ ] `cargo build`, clippy `-D warnings`, fmt clean (doc-only change; +- [x] `cargo build`, clippy `-D warnings`, fmt clean (doc-only change; tests unaffected) ## References @@ -75,8 +75,74 @@ it and correct against the settled code. ## Notes -> To be filled by implementation agent +Decisions of record the implementation made that the description +didn't pin: + +- **The QueueOpts mirror landed in core's `alkstore/src/opts.rs`, + not the pg crate** — the description offered "`queue.rs`'s or + `opts.rs`'s doc where the opts are documented"; the pg crate's + `opts.rs` documents `PgOpts` (no `QueueOpts` mention), and + `QueueOpts`' doc home is core's opts module, which both engines' + consumers read. The mirror (trusted-as-given numerics, pointer to + deployment.md) went on the `QueueOpts` struct doc there. The pg + `PgOpts` doc got a one-line TLS pointer instead (v1 hardwires + `NoTls` on every path; deployment.md carries the statement) — that + is the engine-crate-docs posture ADR-016 §2 pins, and `PgOpts` is + where a consumer meets the config split. +- **Per-field behaviors verified against code before writing the + symptoms** (the review's "self-dead-letters into churn" was + imprecise): `max_attempts <= 0` rows are *never claimed* — the + claim statement's `attempts < max_attempts` conjunct excludes them + and the pre-claim sweep (`attempts >= max_attempts`) dead-letters + each at the next claim call on its queue; negative + `dead_letter_retention_s` deletes *every* dead row at the next + `sweep_expired` (`died_at <= now - retention` with retention < 0 is + always true; the sweeper is caller-driven, retained in the doc's + wording); negative/zero `visibility_timeout_s` stamps + past-deadline claims (instantly reclaimable). The SQLite engine's + `resolution.rs` stamps identically, so the "both engines" framing + is verified, not asserted. +- **deployment.md gained a `Consumer-obligation notes on engine + options` section** (after Connection budgets, before Durability + knobs) rather than extending an existing list — the + visibility-budgeting precedent lives in core-contract.md, and + this document did not yet have an obligations home; the section + closes the QueueOpts note and records the one counter-case + (`PgOpts::max_size`'s 0-guard, `pg-fix-open-path`'s) so the + trusted-as-given posture is bounded, not blanket. The TLS + statement is a `TLS posture (v1)` subsection under Connection + budgets. ## Summary -> To be filled on completion \ No newline at end of file +Doc-only alignment landed (review 002 Finding 6's remainder): + +- **`alkstore-postgres/src/forwarder.rs`** — the `ListenerConnection` + doc no longer claims the pooled path "rides the consumer's + `Config` sslmode"; it now states `NoTls` is hardwired on *every* + connection path (pooled, listener, reconnect) and deployment.md + owns the statement. Grep-audited: no other doc in the crate (crate + docs, `PgOpts`, `open`) repeats the false claim — `PgOpts` gained + the corrected TLS pointer. +- **`docs/architecture/deployment.md`** — new `TLS posture (v1)` + subsection (NoTls everywhere; `sslmode=require` DSN fails at + connect; topology-level confidentiality is the v1 substitute; + post-v1 concern; ADR-016 spirit) and new + `Consumer-obligation notes on engine options` section (the + QueueOpts numeric trusted-as-given note with the three verified + per-field symptoms + the `max_size` guard counter-case); + `last_updated` frontmatter advanced. +- **`alkstore/src/opts.rs`** — `QueueOpts` struct doc carries the + mirror sentence (trusted-as-given, the ADR-023 §2 scoping, pointer + to deployment.md for symptoms). +- **Cross-file sweep** (forwarder/tx/scheduler/store, the fix batch's + touched files): re-read the module docs against the settled + post-fix code — reconnect retry core, fanout release guard, + stale-generation reconcile (forwarder); commit-atomic wakes and + drop=rollback (tx); quarantine + retry + TTL-lapse posture (scheduler); + open-path guard and DSN-options append (store). No mismatch + beyond the TLS claim found; the review's scheduler/`sweep_expired` + doc fixes had already landed with their own fix tasks. +- **Gates**: workspace `cargo build`, `cargo clippy --all-targets -- + -D warnings`, `cargo fmt --check` all green (doc-only change; no + test touched). \ No newline at end of file