From 4d5bb494f762bf25b5185d5dd668fd02b7503fa3 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 08:09:49 -0700 Subject: [PATCH 1/7] Close CI doctest gap, add aggregate status gate Continuous integration never ran doctests for two of the workspace's crates: the concurrent test runner skips doctests and the workspace doctest step excludes them, so a dedicated step now covers both. A new aggregate job gives branch protection a single required status check that fails when any job fails or is cancelled, so a cancelled job can no longer read as green. An unused development dependency is removed from the speech-to-text crate. - `.github/workflows/ci.yml`: places the doctest step in the check-workshop job rather than the Linux test job, with a comment recording that nextest does not run doctests. - `ci-green`: runs under if: always() and exits nonzero unless every job in its needs list reports success; the list is static, so each future job must be added to it or it bypasses the gate. - `crates/gateway-stt/Cargo.toml`: drops the unreferenced tracing-subscriber dev-dependency, with the lockfile updated to match. Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- .github/workflows/ci.yml | 26 ++ Cargo.lock | 1 - crates/gateway-stt/Cargo.toml | 1 - vibe-ledger.md | 2 + vibe/2026-09-11-1-rulebook-debt-tiers.md | 313 +++++++++++++++++++++++ vibe/ACTIVE | 1 + 6 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 vibe/2026-09-11-1-rulebook-debt-tiers.md create mode 100644 vibe/ACTIVE diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2603f73e2..648307bd0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -203,6 +203,11 @@ jobs: - name: Test (workshop, concurrent via nextest) run: cargo nextest run --locked -p workshop -p workshop-server + # nextest does not run doctests; the workspace doctest step in the + # `test` job excludes both workshop crates, so cover them here. + - name: Doctests (workshop) + run: cargo test --doc -p workshop -p workshop-server + - name: Test Gateway process ownership races run: | cargo test --locked -p shared-sidecar a_process_lifetime_lease_recovers_after_its_owner_is_terminated @@ -379,3 +384,24 @@ jobs: - name: cargo audit run: cargo audit + + # Single required status check for branch protection: it fails when any + # job fails or is cancelled, so a cancelled job cannot read as green. + # needs: is a static list, so every newly added job must be added to it + # or that job bypasses this gate. + ci-green: + runs-on: ubuntu-latest + needs: [fmt, clippy, test, docs, check-workshop, check-workshop-linux, ui, msrv, supply-chain] + if: always() + steps: + - name: Verify every job succeeded + shell: bash + run: | + status=0 + for result in ${{ join(needs.*.result, ' ') }}; do + if [ "$result" != "success" ]; then + echo "a required job did not succeed: $result" + status=1 + fi + done + exit $status diff --git a/Cargo.lock b/Cargo.lock index e9065d8fc..032ddf48e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2152,7 +2152,6 @@ dependencies = [ "tokio-util", "tower", "tracing", - "tracing-subscriber", ] [[package]] diff --git a/crates/gateway-stt/Cargo.toml b/crates/gateway-stt/Cargo.toml index fe158144c..2f6a8dcd0 100644 --- a/crates/gateway-stt/Cargo.toml +++ b/crates/gateway-stt/Cargo.toml @@ -32,7 +32,6 @@ sha2.workspace = true tempfile.workspace = true tokio-tungstenite.workspace = true tower.workspace = true -tracing-subscriber.workspace = true [features] test-fixtures = [ diff --git a/vibe-ledger.md b/vibe-ledger.md index 259272eb5..34facbc7c 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -56,3 +56,5 @@ - Async STT boot reset Step 5: Notify the browser when STT needs restart - Config UI typecheck, build, and complete suite (142 passed). Decision: toast kind `info`; catalog order ignored; membership filtered through each document's own catalog; combined process-owned plus STT Apply shows both the existing banner and one STT toast. Falsifier: reordering `[[stt_model]]` changes what boot loads, or a profile naming a non-STT model is a speech change. - Async STT boot reset Step 6: Reconcile documentation and qualify - Full verification passed: build, fmt, warnings-denied clippy (portable and staged Workshop), portable workspace and doc tests, feature-disabled Gateway, both UI suites, staged Workshop tests with guaranteed cleanup, both Miri lanes, all five native Whisper lanes with hash-pinned fixtures, guide assembler with zero drift, mdBook build, and `cargo workshop` package construction. Operator confirmed physical microphone hypothesis revision, authoritative completion, and second take on the release binaries. + +- Rulebook debt tiers Step 1: CI doctest coverage and dependency hygiene - component-scope verify pass: build, `cargo fmt --all --check`, warnings-denied clippy, and `cargo test --locked -p gateway-stt` (121 unit + 52 integration) green; nextest unavailable locally, cargo test fallback recorded in verify-step-1-round-1.log. Decision: `ci-green` treats cancelled as failure via an `if: always()` loop over `needs.*.result` | Falsifier: a cancelled or newly added job shows green as a required status check. Decision: doctest step placed in `check-workshop` (Windows) rather than the Linux `test` job | Falsifier: workshop doctests run twice or not at all in CI. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md new file mode 100644 index 000000000..c5e443a17 --- /dev/null +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -0,0 +1,313 @@ +--- +name: Fix tier 1 and 2 rulebook deviations +overview: Fix all Tier 1 (correctness/operational) and Tier 2 (hygiene) rust-rulebook deviations verified at promptforge HEAD da1456b, as a series of focused commits following the repo's vibe/ plan conventions. +todos: + - id: ws-b + content: "B: CI doctest step + aggregate job + drop unused tracing-subscriber dev-dep" + status: pending + - id: ws-a1 + content: "A: fix blocking/fallible Drop impls (RecoveryCandidate, GatewaySupervisor, SttEngine/Transcriber)" + status: pending + - id: ws-a2 + content: "A: restore discarded error causes (SessionError::Inference, AudioError::InvalidBase64)" + status: pending + - id: ws-a3 + content: "A: bound the session_agents supervisor event channel" + status: pending + - id: ws-d1 + content: "D: add #[non_exhaustive] to 7 public items/variants" + status: pending + - id: ws-d2 + content: "D: convert 43 #[allow] sites to #[expect] with reasons" + status: pending + - id: ws-e + content: "E: convert ~5 in-process async tests to paused time" + status: pending + - id: ws-c1 + content: "C1: FixtureError for the fixture API + typed sources in the take/finalization pipeline (~26 sites)" + status: pending + - id: ws-c2 + content: "C2: anyhow for test-only code and build tools + folded typed errors in remaining production (~32 sites)" + status: pending +isProject: false +--- + +# Fix Tier 1 + Tier 2 rust-rulebook deviations in promptforge + + + +## Product Requirements + +A 150-commit audit of the promptforge repository against the workspace Rust rulebook found 78 commits introducing deviations. The maintainers who run this repository need the correctness-relevant subset of that debt eliminated without a style-churn campaign, because the audit also showed that the repository holds several deliberate counter-conventions where the rulebook, not the code, is what should change. + +- Goals: fix every verified Tier 1 finding (blocking or fallible destructors, discarded error causes, a CI doctest coverage gap, an unused dev-dependency, an unbounded channel) and every verified Tier 2 finding (stringly `Result<_, String>` errors, missing `#[non_exhaustive]` on public items, `#[allow]` suppressions that should be `#[expect]`, real-time sleeps in in-process async tests) at HEAD `da1456b`. +- Non-goals: the entire Deferred Workstream cataloged under Execution Instructions - documentation-example convergence, oversized-file splits, import regrouping, bool-flag elimination, test-layout consolidation, the toolchain-pin policy question, and the real-network test sleeps. +- Success criteria: every finding verified at HEAD is either fixed with the full verification gate green or explicitly recorded as deferred. +- Constraints, from the repository's own policy: + - Behavior changes ship with tests in the same change. + - No structural enforcement may be introduced. + - The four cross-product dependency rules hold. + - The local toolchain is pinned to MSRV 1.89 while CI lints on a newer stable, so import and doc-comment changes must be checked with the newer toolchain before pushing. + - Work lands as focused commits following the repository's vibe/ plan and ledger conventions. +- Open questions: the bounded capacity for the supervisor event channel in workstream A, delegated to the executor with a justification requirement. + +## Functional Specification + +The actor is a maintainer (or agent) executing the workstreams as a series of commits; CI is the enforcing counterparty. The inputs are the HEAD-verified inventories recorded under Project Survey, and the outputs are six commits, one per step, plus this plan committed to the repository. Each commit is a state transition that must leave the full verification gate green; a commit that cannot pass the gate does not land. + +Two recovery rules govern the work itself: + +- Where a fix changes runtime behavior (the destructor and channel work), tests proving the new behavior ship in the same commit. +- Where an `#[allow]` suppression turns out to be stale during conversion, it is deleted rather than converted. + +On security and privacy, restoring error sources changes what printed error chains contain; the added sources are internal typed errors carrying no credential material, and the existing log redaction in gateway-logging must remain intact and is verified by its existing tests. + +Acceptance is observable: + +- No `Drop` impl in the touched types performs blocking joins or network I/O. +- The two discarded error causes are carried as sources. +- No `unbounded_channel` remains in the session-agents supervisor pipe. +- Workshop doctests run in CI under an aggregate required-status job. +- The unused dev-dependency is gone. +- No `Result<_, String>` remains in the surveyed sites. +- The seven public items carry `#[non_exhaustive]`. +- No surveyed `#[allow]` remains without conversion or deletion. +- The in-process async tests run on paused time. + + + + + +## Technical Design + +The design is a set of independent local repairs against one workspace; there is no cross-module architecture change. + +Every edit conforms to these rules, stated here in full so execution needs no external reference: library errors are concrete thiserror types while test, build-script, and build-tool code uses anyhow; error messages are lowercase noun phrases with no trailing period and no `failed to` prefix; every wrapped error carries its cause in a `#[source]` field; public error enums and their data-carrying variants are `#[non_exhaustive]`; error types are `Send + Sync + 'static`; lint suppressions use `#[expect(lint, reason = "...")]`, and a suppression found stale is deleted rather than converted; new public items are documented, with `# Errors` where they return `Result`; destructors stay infallible and non-blocking, with fallible or blocking work exposed through explicit `shutdown()` methods; and every behavior change ships its tests in the same commit. + +**Destructors.** Blocking `Drop` bodies are replaced with explicit fallible shutdown methods plus non-blocking drops. `RecoveryCandidate` in [crates/workshop/src/gateway/supervisor.rs:93](promptforge/crates/workshop/src/gateway/supervisor.rs) gains an explicit `shutdown(self) -> Result<...>` owning the `request_shutdown_before` call; its `Drop` becomes a non-blocking best-effort signal rather than a no-op, so a missed explicit `shutdown()` still signals the unpublished gateway process. `GatewaySupervisor` (same file, line 381) and `SttEngine` ([crates/gateway-stt-engine/src/engine.rs:187](promptforge/crates/gateway-stt-engine/src/engine.rs), with the same pattern in `Transcriber` at worker.rs:138) already expose `shutdown()` and need only their `Drop` bodies reduced to signal-and-detach, with doc comments stating that the explicit method is the blocking, error-reporting path. Two checks gate all of this: before any detach, verify the workers hold only owned or `Arc` state and borrow nothing from the dropped object, and audit every drop site of all three types so no caller silently relies on the old blocking drop. + +**Error causes.** Two unit variants gain source fields: `SessionError::Inference` in [crates/gateway-stt/src/realtime/session/state.rs:44](promptforge/crates/gateway-stt/src/realtime/session/state.rs) and `AudioError::InvalidBase64` in [crates/gateway-stt/src/audio.rs:171](promptforge/crates/gateway-stt/src/audio.rs), eliminating the `map_err(|_| ...)` discards at their call sites. Both variants become data-carrying, so each also gains variant-level `#[non_exhaustive]`, and their messages stay lowercase noun phrases. + +**Supervisor channel.** The event pipe in [crates/workshop-server/src/session_agents.rs:251](promptforge/crates/workshop-server/src/session_agents.rs) becomes a bounded `mpsc::channel(N)`. This is not semantics-preserving: today's unbounded sends fail only when the receiver is gone, while a bounded queue can also drop events under load, and these are lifecycle events whose loss can hang a state transition. Before choosing N, the executor must classify which `SupervisorEvent` kinds are loss-tolerant, route loss-intolerant events over a guaranteed path (a blocking send where the call site allows it, or a separate unbounded side channel), and justify N with headroom evidence for the remainder. + +**Stringly errors.** The elimination introduces one new public error type, a thiserror `FixtureError` for the feature-gated fixture API in [crates/gateway-stt/src/test_fixtures.rs](promptforge/crates/gateway-stt/src/test_fixtures.rs), with `#[non_exhaustive]` on the enum and separately on every data-carrying variant, operation-named variants, lowercase noun-phrase messages with no trailing period, and `Send + Sync + 'static`. The work reuses `SessionError`/`TranscribeError` where they are the genuine source in the take/finalization pipeline, folds private production helpers into their crates' existing error enums, and uses `anyhow::Result` in test-only code, the build scripts, and the build-tool binaries. New public items introduced anywhere in this work carry full documentation with `# Errors` sections where they return `Result`. + +**Attributes.** Mechanical work: `#[non_exhaustive]` on seven public items and `#[allow]` to `#[expect(..., reason = "...")]` across 43 surveyed sites. + +**Paused time.** Roughly five in-process async tests convert to `#[tokio::test(start_paused = true)]` with explicit clock advance, contingent on the `test-util` feature being enabled for those crates. + +The public API surface changes are additive attributes and new error types only; all affected crates are `publish = false`, so no semver or deprecation staging applies. + +### Verified inventories at HEAD + +The repository is a multi-crate Rust workspace at HEAD `da1456b` with a clean tree, edition 2024, resolver 3, MSRV 1.89 pinned in `rust-toolchain.toml`, and all product crates `publish = false`. The verified inventories that bound the work: + +- Four blocking or fallible destructor sites: supervisor.rs:93 and :381 in workshop, engine.rs:187 and worker.rs:138 in gateway-stt-engine. +- Two discarded error causes: route.rs:109 and audio.rs:171 in gateway-stt. +- One unbounded channel in the session-agents supervisor pipe (session_agents.rs:251, with senders in lifecycle.rs and receivers in supervisor/events.rs); the other two audited unbounded channels were already removed. +- A CI workflow at [.github/workflows/ci.yml](promptforge/.github/workflows/ci.yml) whose `check-workshop` job runs nextest for workshop and workshop-server while the only doctest step explicitly excludes both crates, and which has no aggregate `needs:` job. +- An unused `tracing-subscriber` dev-dependency at [crates/gateway-stt/Cargo.toml:35](promptforge/crates/gateway-stt/Cargo.toml), verified unreferenced in the crate. +- 58 stringly `Result<_, String>` sites: ~20 in the feature-gated fixture API of gateway-stt `test_fixtures.rs`, ~6 in the production take/finalization pipeline, ~12 in test-only code, and ~24 across remaining production code and build tools. +- 6 public items missing `#[non_exhaustive]` - `GatewayStartup` and its `OwnerTimeout` variant in gateway `relaunch.rs`, `DecodeMode` and `DecodeRequest` in gateway-stt-engine `decoder.rs`, `EnginePolicy` in `policy.rs`, `ValidatedConnection` in shared-sidecar `validated.rs` - plus the `GatewayPublicationError::Build` variant in workshop-server `gateway_binding.rs` to align. +- 43 `#[allow]` suppressions, of which 26 already carry reasons and 17 need reasons written. +- 32 `tokio::time::sleep` calls in test code, of which exactly one already uses `start_paused` and roughly six are in-process and pause-friendly: promptforge-core `execute/tests/input.rs:315`, promptforge-lua `dispatch.rs:282`, promptforge-core `execute/tests/scheduler.rs:3089` and `:3121`, gateway-stt-engine `test_fixtures/tests/scenario_cleanup/decode.rs:89`. + +Whether `anyhow` is already in `[workspace.dependencies]` and whether the affected crates enable tokio's `test-util` feature are both to be confirmed at execution time. + + + + + +## Testing Plan + +Unit coverage comes from the rule that every behavior change ships its tests in the same commit: the destructor changes gain tests proving drop no longer blocks and explicit shutdown still reports failures, the channel change gains tests proving loss-intolerant events are guaranteed delivery and a full queue never grows without bound, and the error-source changes gain assertions on the restored chains. Integration coverage is the existing suite, which must pass unmodified except where a surveyed signature changed; the paused-time conversions are themselves test changes and must be proven deterministic by running the affected tests repeatedly. + +Verification is deliberately light per step and heavy once at the end. During coding, only the step's focused tests run. Each step ends with a component-scope check: the build, `cargo fmt --all --check`, `cargo clippy --all-targets --all-features -- -D warnings`, and the touched crates' tests. The full gate runs once, on the final step: + +- `cargo fmt --all --check` +- `cargo clippy --all-targets --all-features -- -D warnings` (additionally with the newer CI stable whenever imports or doc comments were touched) +- `cargo test --locked --workspace --all-features` plus `cargo nextest run -p workshop -p workshop-server` +- `cargo test --doc` +- `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features` + +The exit criterion is that gate green on the final commit with every acceptance behavior under Functional Specification observable. + + + + + +## Decision Record + +The governing decisions were made in conversation and are recorded with the user's words. + +- Scope: fix the correctness and hygiene tiers only - "lets fix everything in tiers 1 and 2." Rejects a fix-everything campaign, because 78 of 150 commits deviating from a rule indicates the rule rather than the code is wrong for this repository, and because style churn costs blame history and review load for no behavioral gain. Revisit when a maintainer decides to enforce the rulebook strictly, which should happen through a rulebook addendum first. +- Async tests: convert only the in-process tests to paused time - the user selected "Convert only the ~6 in-process tests to paused time; leave real-network tests as-is." The remaining ~26 tests drive real websocket, TCP, or HTTP peers whose wall-clock behavior paused time cannot model. Revisit if a fake-transport seam appears for those suites. +- Stringly errors: the full cleanup - the user selected "Fix all 58: anyhow in test code, concrete thiserror types in production and the fixture API." Rejects the production-only and tests-only subsets because partial conversions leave the fixture API, the largest cluster, stringly. +- Deferred workstream: the user asked to "add as Deferred Workstream everything else," keeping every remaining finding cataloged rather than silently dropped. +- Plan form: the user asked for "the mandated sections but ... expository paragraphs instead of rigid bullets," softened to prose-plus-bullets after review, and because "I will want this plan committed to the repo," the plan is written to stand alone and lands in the repository under the vibe/ plan convention. +- Execution style: strictly serial, one commit at a time - "I do not want parallel execution." Rejects concurrent subagent workstreams despite their disjoint file sets; no revisit condition was requested. +- Step shape: six steps - the five consolidated workstreams with C split at its design seam - after the user asked "Would you prefer 6, or even 7 steps? I'm open to it - use your judgement." C1 (fixture API plus the take/finalization pipeline) carries the design content, the new `FixtureError` type; C2 (test-only anyhow conversions and remaining production helpers) is the mechanical sweep. D was not split because D1 is seven one-line attributes whose review overhead would exceed its risk. Rejects finer decomposition into per-crate or per-cluster commits to keep per-step review and verification overhead proportionate. +- Verification cadence: light per step, full gate once at the end - "I don't want to have to do the full verify at every step." Rejects the plan's earlier gate-per-commit wording; the accepted risk is that a cross-crate regression surfaces at the final step rather than at its originating commit. + +Assumptions, risks, and notes: + +- The audit judged commits historically, so the HEAD-verified inventory under Project Survey governs and any site that drifted since verification is re-checked before editing. +- The destructor changes alter shutdown timing and carry the highest behavioral risk in the plan. +- Adding `#[source]` changes printed error chains, which is the intent. +- Several audit findings self-healed before this plan (the `SpeechReplacement` rollback drop, the unbounded relay and decode channels, `SpeechError::Rollback`, and the source-compiled CI tool installs), which is why historical findings alone never justify an edit. +- A pre-execution review corrected the plan itself: the channel work originally claimed bounded `try_send` preserves today's semantics exactly (false - a full queue drops lifecycle events that today are only lost when the receiver is gone), the detach work lacked a check that workers hold no borrows from the dropped object, two either/or instructions were unresolved, and the C/D ordering contradicted itself. All were repaired before handoff. +- A final conformance pass inlined the governing rules into Technical Design so execution needs no external reference: it added variant-level `#[non_exhaustive]` for the error variants that become data-carrying, the style requirements for the new `FixtureError` (operation-named variants, lowercase noun-phrase messages, `Send + Sync + 'static`), and the documentation requirement for new public items. + + + + + +## Project Survey + +- Status: complete +- Build: `cargo build --locked -p gateway` (gateway is the sole default member; the desktop app is an explicit `cargo build --locked -p workshop`). UI bundles require `npm ci --prefix crates/workshop-server/ui` and `npm ci --prefix crates/gateway-config-ui/ui` first; crate build scripts bundle the UIs into `OUT_DIR`. +- Focused test command pattern: `cargo test -p ` (used by CI for process-ownership races, e.g. `cargo test --locked -p shared-sidecar a_process_lifetime_lease_recovers_after_its_owner_is_terminated`). +- Component test command pattern: `cargo nextest run --locked -p ` (workshop job runs `cargo nextest run --locked -p workshop -p workshop-server`); gateway integration suites run via `cargo test -p --test it [filter]` (e.g. `cargo test -p gateway-stt --test it architecture`). +- Full-suite test command: `cargo nextest run --locked --workspace --exclude workshop --exclude workshop-server --all-features`, plus doctests via `cargo test --workspace --exclude workshop --exclude workshop-server --all-features --doc`. +- Linter command: `cargo clippy --workspace --exclude workshop --exclude workshop-server --all-targets --all-features -- -D warnings`; workshop crates lint separately with `cargo clippy -p workshop -p workshop-server --all-targets -- -D warnings`. CI lints on stable, newer than the pinned MSRV. +- Formatter check command: `cargo fmt --all --check`. +- Docs command: `cargo doc --workspace --no-deps --all-features --exclude workshop --exclude workshop-server` with `RUSTDOCFLAGS: -D warnings`. The user guide builds with `mdbook build guide`; its generated indexes regenerate via `cargo run -p build-user-guide`. +- Test placement and naming conventions: unit tests live in `src` modules; integration tests live under `crates//tests/` as a multi-file suite behind a `main.rs` harness with one sibling module per area (promptforge-core: `tests/suite/{main,execution,fanout,parsing,shipped,support}.rs`; gateway: `tests/it/{main,boot,cache,chat,cuda,local,profiles,queue,sidecar,...}.rs`). Fixtures sit beside suites (e.g. `tests/prompts/`). Criterion benches live in `crates/promptforge-core/benches/`. Test names are descriptive snake_case sentences (`simultaneous_direct_launches_leave_one_owner_and_one_clean_handoff`). +- Directory map: `crates/` holds all 36 Rust workspace members plus `crates/shared-ui` (TypeScript+CSS package, excluded from the Cargo workspace); `guide/` is the mdBook user guide with four audience sets (`src/workshop/`, `src/gateway/`, `src/language/`, `src/agent/`) and a `scratch/` regeneration cache; `tools/` holds harness tool files and node scripts (`stage-gateway-sidecar.mjs`); `prompts/` holds prompt files; `vibe/` holds `archdoc.md` and dated design notes; `images/` holds README assets; `.github/workflows/` holds CI (`ci.yml`, `guide.yml`, release and nightly workflows); `target/` and `target-msrv/` are build output. +- Component boundaries (per `vibe/archdoc.md`): the executor (promptforge-core, promptforge-parser, promptforge-lua, promptforge-agent) parses and runs pipelines and depends on gateway, store, Lua VM boundary, and shared substrate; the gateway (gateway plus gateway-* crates) owns model routing, provider credentials, and local inference and depends only on shared substrate; the CLI (`promptforge` crate) is a thin shell adapter over executor, gateway, and store; the workshop UI (workshop, workshop-server) hosts the executor in-process and attaches over the gateway protocol; the store (promptforge-store) is a run-scoped virtual filesystem with no dependencies; the shared substrate (shared-loopback, shared-progress, shared-protocol, shared-sidecar) has no dependencies. AGENTS.md binds four cross-product rules: gateway crates cannot depend on workshop crates, promptforge product crates cannot depend on gateway or workshop product crates, gateway product crates cannot depend on promptforge product crates, and workshop product crates cannot depend on gateway product crates. +- Conventions summary: Rust edition 2024, MSRV 1.89 pinned in `rust-toolchain.toml` while CI lints on newer stable; workspace lints forbid `unsafe_code`, deny clippy `all` plus `unwrap_used`/`expect_used`, and warn on `missing_docs` and pedantic; behavior changes ship with tests in the same change and structural enforcement checks require explicit user approval; library and serve paths return failures instead of exiting or installing process-global state; long-running work reports through `shared-progress`; comments explain non-obvious constraints and cite upstream issue URLs for platform workarounds; Cargo features gate real constraints (toolchain, native builds), not product shape; build steps must never dirty the git tree (CI enforces a clean-tree check); docs prose bans em-dashes and opens code fences with four backticks. + + + + + +## Execution Instructions + +The work decomposes into five components landing as six steps, each step one commit, executed strictly serially in the order below per the recorded decision against parallel execution. Component placement rationale: `ci-gate-hygiene` first so the stricter CI gate guards every later commit; `runtime-hazard-repairs` next because it carries the highest behavioral risk and benefits from the tightened gate; `api-attributes` and `paused-time-tests` follow as independent mechanical components with no dependencies on each other or on the error work; `stringly-error-elimination` last because it is largest. That component splits into two sequential pieces: the `FixtureError` design content (Step 5) before the mechanical sweep (Step 6), because Step 5 introduces the `FixtureError` type and typed pipeline sources the remaining work folds into. Where a file is in both the attribute component's and the error component's scope, the attribute step skips it and that file's conversions fold into the corresponding error commit after the semantic edits. + +Per-step verification is deliberately light: the step's focused tests, then a component-scope check of the build, `cargo fmt --all --check`, `cargo clippy --all-targets --all-features -- -D warnings`, and the touched crates' tests. The full verification gate from the Testing Plan runs once, on the final step. Every behavior change ships its tests in the same commit. + +The run is seeded by copying this plan verbatim to `vibe/YYYY-MM-DD-N-words.md`, pointing `vibe/ACTIVE` at that path, and committing both with subject `[WIP] Plan: `; progress is marked in the repository copy as steps complete. + + + +### Step 1: CI doctest coverage and dependency hygiene [completed] + +- Component: ci-gate-hygiene + +Add a `cargo test --doc -p workshop -p workshop-server` step to the `check-workshop` job in `.github/workflows/ci.yml`, closing the gap where the only doctest step explicitly excludes both crates. Add an aggregate required-status job with `needs:` on all existing jobs and an `if: always()` failure check. Delete the unused `tracing-subscriber` dev-dependency at `crates/gateway-stt/Cargo.toml:35`, verified unreferenced in the crate. + +Verification: the workflow lints clean, `gateway-stt` builds and its tests pass without the dev-dependency, and the component-scope check is green. + + + + + +### Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel + +- Component: runtime-hazard-repairs + +Destructors: give `RecoveryCandidate` (`crates/workshop/src/gateway/supervisor.rs:93`) an explicit fallible `shutdown(self)` owning the `request_shutdown_before` call, and reduce its `Drop` to a non-blocking best-effort signal so a missed explicit `shutdown()` still signals the unpublished gateway process. Reduce the `Drop` bodies of `GatewaySupervisor` (same file, line 381), `SttEngine` (`crates/gateway-stt-engine/src/engine.rs:187`), and `Transcriber` (`crates/gateway-stt-engine/src/worker.rs:138`) to signal-and-detach, with doc comments stating that the explicit `shutdown()` is the blocking, error-reporting path. Before any detach, verify the workers hold only owned or `Arc` state and borrow nothing from the dropped object, and audit every drop site of all three types so no caller silently relies on the old blocking drop. + +Error causes: add `#[source]` fields to `SessionError::Inference` (`crates/gateway-stt/src/realtime/session/state.rs:44`) and `AudioError::InvalidBase64` (`crates/gateway-stt/src/audio.rs:171`), making both variants data-carrying with variant-level `#[non_exhaustive]` and lowercase noun-phrase messages, and delete the two `map_err(|_| ...)` discards at their call sites (`route.rs:109` and `audio.rs:171`). + +Channel: convert the event pipe at `crates/workshop-server/src/session_agents.rs:251` (senders in `lifecycle.rs`, receivers in `supervisor/events.rs`) to a bounded `mpsc::channel(N)` after classifying which `SupervisorEvent` kinds are loss-tolerant, routing loss-intolerant events over a guaranteed path (a blocking send where the call site allows it, or a separate unbounded side channel), and justifying N with headroom evidence for the remainder. + +Tests in the same commit prove: drop no longer blocks and explicit shutdown still reports failures; the restored error chains carry their sources; loss-intolerant events get guaranteed delivery and a full queue never grows without bound. The component-scope check follows. + + + + + +### Step 3: `#[non_exhaustive]` attributes and `#[expect]` conversions + +- Component: api-attributes + +Apply `#[non_exhaustive]` to the seven surveyed public items: `GatewayStartup` and its `OwnerTimeout` variant (gateway `relaunch.rs`), `DecodeMode` and `DecodeRequest` (gateway-stt-engine `decoder.rs`), `EnginePolicy` (gateway-stt-engine `policy.rs`), `ValidatedConnection` (shared-sidecar `validated.rs`), and the `GatewayPublicationError::Build` variant (workshop-server `gateway_binding.rs`). Convert the 43 surveyed `#[allow]` suppressions to `#[expect(lint, reason = "...")]`, writing reasons for the 17 that lack them and deleting any suppression found stale rather than converting it. Skip files in the stringly-error component's scope; their attribute conversions fold into Steps 5 and 6 after the semantic edits. + +This is a mechanical change: the existing suite unmodified plus the component-scope check is the verification. + + + + + +### Step 4: Paused-time conversion for in-process async tests + +- Component: paused-time-tests + +After confirming tokio's `test-util` feature is enabled for the affected crates, convert the roughly five in-process async tests to `#[tokio::test(start_paused = true)]` with explicit clock advance: promptforge-core `execute/tests/input.rs:315`, promptforge-lua `dispatch.rs:282`, promptforge-core `execute/tests/scheduler.rs:3089` and `:3121`, and gateway-stt-engine `test_fixtures/tests/scenario_cleanup/decode.rs:89`. Leave the ~26 real-network test sleeps as-is per the recorded decision. + +Prove the conversions deterministic by running the affected tests repeatedly; the component-scope check follows. + + + + + +### Step 5: `FixtureError` and typed sources in the take/finalization pipeline + +- Component: stringly-error-elimination + +Eliminate the ~26 stringly `Result<_, String>` sites in the fixture API and the production take/finalization pipeline. Introduce a public thiserror `FixtureError` for the feature-gated fixture API in `crates/gateway-stt/src/test_fixtures.rs`: `#[non_exhaustive]` on the enum and separately on every data-carrying variant, operation-named variants, lowercase noun-phrase messages with no trailing period and no `failed to` prefix, `#[source]` where wrapping, `Send + Sync + 'static`, and full documentation with `# Errors` on new public items returning `Result`. Include the future bound in `realtime/session/items.rs`. Give the pipeline sites in `realtime/item.rs`, `take/state.rs`, `take/finalization.rs`, and `realtime/session/state.rs` typed sources, reusing `SessionError`/`TranscribeError` where they are the genuine source. Attribute conversions for touched files fold into this commit after the semantic edits. + +Tests asserting the new error chains ship in the same commit; the component-scope check follows. + + + + + +### Step 6: anyhow for test and build code, typed errors in remaining production + +- Component: stringly-error-elimination + +Eliminate the remaining ~32 stringly sites. Use `anyhow::Result` in test-only code (the `realtime/wire/server.rs` validators, `take/state/alignment_tests/adversaries.rs`, `tests/it/realtime_session.rs`, gateway `test_support.rs`, and the test-fixture-gated `main.rs` rendezvous), the build scripts, and the build-tool binaries, confirming first whether `anyhow` is already in `[workspace.dependencies]`. Fold typed errors into the crates' existing error enums for the remaining private production helpers: gateway `config_write.rs` and `dialect.rs`, gateway-local `confine.rs`, workshop-server `session/menu.rs`, promptforge-tool-picker `rank.rs`, build-ui `lib.rs`, and the two promptforge-lua LazyLock statics. Attribute conversions for touched files fold into this commit after the semantic edits. + +This is the final step, so the full verification gate from the Testing Plan runs here: `cargo fmt --all --check`; `cargo clippy --all-targets --all-features -- -D warnings`, additionally on the newer CI stable because imports and doc comments were touched; `cargo test --locked --workspace --all-features` plus `cargo nextest run -p workshop -p workshop-server`; `cargo test --doc`; and `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features`. The commit lands only with the gate green and every acceptance behavior under the Functional Specification observable. + + + +### Deferred Workstream + +Five catalogs of findings deliberately not executed in this pass; each must be re-verified at the then-current HEAD before scheduling, because the audit judged commits historically and some entries have already self-healed. + +#### F1. Documentation gaps + +- Missing `# Examples` doctests on public items across promptforge-core `execute/config.rs`, promptforge-lua `vm.rs`, promptforge-tool-picker `model.rs`, gateway-stt `service.rs` and the test-fixture modules, gateway-stt-engine `decoder.rs` and `test_fixtures/native.rs`, shared-sidecar `shutdown.rs`/`stale.rs`/`health.rs`/`lock.rs`/`validated.rs`, workshop-server `gateway_binding.rs`/`serve.rs`/`menu.rs`/`session_agents.rs`, gateway `diagnostics.rs`, shared-loopback `lib.rs`, and gateway-config `config/stt.rs` with `accessors.rs`. +- Missing or variant-less `# Errors` on workshop-server `fixtures.rs` and `lib.rs` and gateway-stt `service.rs`. +- Third-person summary style in shared-sidecar `cancellation.rs` among others. + +#### F2. Module and file structure + +- Missing `//!` module docs across the gateway-stt `take/` and `realtime/` subtrees, gateway-stt `audio.rs`, gateway-stt-engine `test_fixtures/scenarios.rs`, and several workshop-server and gateway test modules. +- Fifteen files past the 500-line split rule, largest being gateway `src/lib.rs` at ~4858 lines, `profile_switch.rs` at 1997, gateway-logging `worker.rs` at ~1455, and build-workshop `main.rs` at 1319. +- Logic in the gateway-logging, gateway, and shared-loopback crate roots. +- `include!`-assembled test splits in workshop-server `tests/it/chat_gate.rs` and `realtime_relay.rs` and gateway `tests/it/realtime_stt.rs`. +- Mixed `mod.rs` versus `foo.rs` module style within gateway-stt. + +#### F3. API design + +- Bool flag parameters in gateway `commands.rs`, gateway-stt `segment.rs` and `segment/boundary.rs`, workshop `menu.rs`, shared-sidecar `health.rs`, gateway-logging `queue.rs`, gateway-stt `model.rs` and `realtime/wire/client.rs`, and the engine worker/model loaders. +- Clone-returning getters in workshop-server `app.rs` and `serve.rs`. +- Missing compile-time Send/Sync assertions in promptforge-core `input.rs` and gateway-logging `runtime.rs`/`writer.rs`. +- Missing `#[must_use]` on shared-sidecar's `GatewayInstanceLease`. +- Over-long combinator chains in promptforge-lua `protocol.rs` and gateway-logging `redact.rs`; the bool-producing match in workshop-server `catalog/chat.rs`. +- The `Cow` candidate in gateway-logging `redact.rs`; `{}` printing of anyhow errors in workshop `gateway.rs`. + +#### F4. Test layout and hygiene + +- Integration tests outside the single `tests/it/main.rs` binary in product-integration-tests, build-workshop, workshop, gateway-stt-engine, gateway-stt-backend-whisper, and gateway-transcribe. +- Unit tests in sibling `tests.rs` files across promptforge-lua, gateway-stt, and workshop-server. +- Bare `#[test]` functions at module scope in workshop-server `test_gateway/process.rs` and gateway-stt `test_fixtures/native.rs`. +- Hand-rolled temp dirs in gateway-logging `runtime.rs` and `worker.rs`; blocking `std::fs` and thread joins inside async tests in gateway-stt `tests/common/mod.rs`. +- Operation-style `expect` messages in gateway-transcribe `tests/native_whisper.rs`. +- The ~26 real-network async-test sleeps deferred by the recorded decision. +- Duplicated fixture helpers left behind when the `test-fixtures` feature was deleted. + +#### F5. Tooling, CI, and policy + +- The `rust-toolchain.toml` MSRV pin codified in AGENTS.md - a policy decision to resolve by unpinning or by amending the rulebook for application repos. +- `tokio::spawn` inside gateway-stt library code at `realtime/registry.rs` and `realtime/session/items.rs`. +- Import grouping outside the three-block rule across promptforge-lua, gateway, gateway-stt, gateway-logging, and workshop-server files; the free-function import in gateway-stt `realtime/wire/server.rs`. +- The `failed to` Display prefix in gateway `config_write.rs`. +- Build-script hygiene in workshop `build.rs` (boxed error instead of anyhow) and gateway `build.rs` (untested manifest-generation logic). + + diff --git a/vibe/ACTIVE b/vibe/ACTIVE new file mode 100644 index 000000000..9b34a710b --- /dev/null +++ b/vibe/ACTIVE @@ -0,0 +1 @@ +vibe/2026-09-11-1-rulebook-debt-tiers.md \ No newline at end of file From 6aa3409c5f7d03a6c3ba9e630a05bc409b8c7f38 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 08:41:59 -0700 Subject: [PATCH 2/7] Detach in Drop, restore error sources, bound cancellations Four destructors that used to block on thread joins or a bounded network shutdown now only signal and detach, with explicit shutdown methods remaining the blocking, error-reporting path. Two error variants that discarded their underlying causes now carry them as sources, so printed error chains name the real failure. The session-agent supervisor event pipe is split by loss-tolerance: events the reducer awaits keep guaranteed delivery on the unbounded queue, while operator cancellations ride a bounded queue that drops redundant duplicates. - `Transcriber::signal_and_detach` - Drop and `abandon_startup` share one signal-and-detach path; the worker captures only owned and Arc state, so a detached thread finishes a running decode on its own. - `RecoveryCandidate::shutdown` - the new explicit fallible path owns the bounded shutdown request and disarms the drop signal even on failure, because the caller owns the outcome. - `SupervisedGatewayIdentity` - the default no-op `shutdown_unpublished` hook lets the failed-replacement-publish arm shut down an unpublished recovered child through the explicit path. - `RunLifecycle` - events split by loss-tolerance: `OperatorCancellation` alone rides a bounded queue of `CANCELLATION_CAPACITY` 1, since a full queue already holds a pending cancellation that retires the current run; settlements and close keep the unbounded guaranteed queue. - `Drop for SttEngine` - signals both workers and detaches their threads instead of running the blocking shutdown join. - `Drop for GatewaySupervisor` - revokes publication, signals the stop, and detaches both threads instead of joining within the shutdown budget. - `Drop for RecoveryCandidate` - runs the late-child shutdown request on a detached named thread instead of blocking the dropping thread. - `SessionError::Inference` - now carries the engine error as its source, and SessionError drops Eq and PartialEq because TranscribeError is not PartialEq. - `AudioError::InvalidBase64` - now carries the base64 decode failure as its source; both restored variants are `#[non_exhaustive]`. - `next_lifecycle_event` - a biased select drains the guaranteed queue before the bounded cancellation queue; cross-channel ordering is not load-bearing. - `InterimTaskOutput::Decode` - the interim transcript no longer stringifies the engine error; the result stays typed from decode through `finish_interim`. Design: extends message-passing @ crates/workshop-server/src/session_agents/lifecycle.rs::RunLifecycle Repairs: non-blocking Drop @ crates/gateway-stt-engine/src/engine.rs::SttEngine - dropping the engine during a running decode blocked until the decode finished Repairs: non-blocking Drop @ crates/gateway-stt-engine/src/worker.rs::Transcriber - dropping the transcriber joined the worker thread mid-decode Repairs: non-blocking Drop @ crates/workshop/src/gateway/supervisor.rs::RecoveryCandidate - dropping an unpublished candidate blocked on the late-child shutdown budget Repairs: non-blocking Drop @ crates/workshop/src/gateway/supervisor.rs::GatewaySupervisor - dropping the supervisor waited out the shutdown budget Repairs: bounded supervisor event queue @ crates/workshop-server/src/session_agents/lifecycle.rs::RunLifecycle - repeated operator cancellations grew the event queue without bound Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- crates/gateway-stt-engine/src/engine.rs | 144 +++++++++++++++++- crates/gateway-stt-engine/src/worker.rs | 61 +++++++- crates/gateway-stt/src/audio.rs | 37 ++++- crates/gateway-stt/src/realtime/route.rs | 4 +- crates/gateway-stt/src/realtime/session.rs | 9 +- .../src/realtime/session/items/tests.rs | 15 +- .../gateway-stt/src/realtime/session/route.rs | 7 +- .../gateway-stt/src/realtime/session/state.rs | 8 +- crates/workshop-server/src/session_agents.rs | 7 +- .../src/session_agents/lifecycle.rs | 88 ++++++++++- .../src/session_agents/supervisor.rs | 3 +- .../src/session_agents/supervisor/events.rs | 71 ++++++++- crates/workshop/src/gateway/supervisor.rs | 78 ++++++++-- crates/workshop/src/gateway/tests/recovery.rs | 104 +++++++++++++ crates/workshop/src/gateway/tests/shutdown.rs | 6 +- vibe-ledger.md | 2 + vibe/2026-09-11-1-rulebook-debt-tiers.md | 2 +- 17 files changed, 586 insertions(+), 60 deletions(-) diff --git a/crates/gateway-stt-engine/src/engine.rs b/crates/gateway-stt-engine/src/engine.rs index 802393d2d..e38bf5607 100644 --- a/crates/gateway-stt-engine/src/engine.rs +++ b/crates/gateway-stt-engine/src/engine.rs @@ -158,7 +158,8 @@ impl SttEngine { /// /// Calling this method more than once has no additional effect. Native /// decoding is non-preemptible, so shutdown waits for a running decode - /// rather than detaching its worker. + /// rather than detaching its worker. This is the blocking, + /// error-reporting path; `Drop` only signals and detaches. /// # Errors /// Returns [`TranscribeError::ShutdownPanicked`] for one panicked worker or /// [`TranscribeError::ShutdownFailures`] for multiple panicked workers. @@ -186,14 +187,20 @@ impl SttEngine { impl Drop for SttEngine { fn drop(&mut self) { - // Explicit shutdown surfaces join panics. Drop cannot return one. - drop(self.shutdown()); + // `shutdown` is the blocking, error-reporting path; Drop signals + // both workers and detaches their threads without joining. + self.transcriber.signal_and_detach(); + if let Some(final_pass) = &self.final_pass { + final_pass.signal_and_detach(); + } } } #[cfg(test)] mod tests { - use std::sync::{Arc, Barrier}; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::{Arc, Barrier, Condvar, Mutex, PoisonError}; + use std::time::Duration; use crate::Decoder; @@ -248,4 +255,133 @@ mod tests { if source.to_string() == "final spawn sentinel" )); } + + /// Shared observation of one decoder parked inside `decode`. + #[derive(Clone, Debug, Default)] + struct ParkControl { + state: Arc, + } + + #[derive(Debug, Default)] + struct ParkState { + phase: Mutex, + changed: Condvar, + dropped: AtomicBool, + } + + #[derive(Debug, Default, Eq, PartialEq)] + enum ParkPhase { + #[default] + Armed, + Entered, + Released, + } + + impl ParkControl { + fn wait_until_entered(&self) { + let phase = self + .state + .phase + .lock() + .unwrap_or_else(PoisonError::into_inner); + let (_phase, timeout) = self + .state + .changed + .wait_timeout_while(phase, Duration::from_secs(1), |phase| { + *phase != ParkPhase::Entered + }) + .unwrap_or_else(PoisonError::into_inner); + assert!(!timeout.timed_out(), "the job parks inside the decoder"); + } + + fn release(&self) { + let mut phase = self + .state + .phase + .lock() + .unwrap_or_else(PoisonError::into_inner); + *phase = ParkPhase::Released; + self.state.changed.notify_all(); + } + + fn wait_until_dropped(&self) { + let deadline = std::time::Instant::now() + Duration::from_secs(1); + while !self.state.dropped.load(Ordering::Acquire) { + assert!( + std::time::Instant::now() < deadline, + "the detached worker drops the decoder after the release" + ); + std::thread::yield_now(); + } + } + } + + #[derive(Debug)] + struct ParkFactory(ParkControl); + + impl ModelFactory for ParkFactory { + fn create(&self, _mode: DecodeMode) -> Result>, TranscribeError> { + Ok(Some(Box::new(ParkDecoder(Arc::clone(&self.0.state))))) + } + } + + struct ParkDecoder(Arc); + + impl Decoder for ParkDecoder { + fn decode(&mut self, _request: DecodeRequest) -> Result { + let mut phase = self.0.phase.lock().unwrap_or_else(PoisonError::into_inner); + *phase = ParkPhase::Entered; + self.0.changed.notify_all(); + drop( + self.0 + .changed + .wait_while(phase, |phase| *phase != ParkPhase::Released) + .unwrap_or_else(PoisonError::into_inner), + ); + Ok("parked".to_owned()) + } + } + + impl Drop for ParkDecoder { + fn drop(&mut self) { + self.0.dropped.store(true, Ordering::Release); + } + } + + #[test] + fn drop_signals_and_detaches_instead_of_joining_a_running_decode() { + let control = ParkControl::default(); + let engine = + SttEngine::new(ParkFactory(control.clone()), policy()).expect("the engine builds"); + let request = + DecodeRequest::new(DecodeMode::Interim, Vec::new(), Vec::new(), String::new()); + let mut decode = Box::pin(engine.decode(request)); + let waker = std::task::Waker::noop(); + let mut context = std::task::Context::from_waker(waker); + assert!( + decode.as_mut().poll(&mut context).is_pending(), + "the submitted decode pends on the parked worker" + ); + control.wait_until_entered(); + drop(decode); + // The delayed releaser turns a blocking join into a failed timing + // assertion instead of a deadlocked test. + let releaser = std::thread::spawn({ + let control = control.clone(); + move || { + std::thread::sleep(Duration::from_millis(500)); + control.release(); + } + }); + + let started = std::time::Instant::now(); + drop(engine); + assert!( + started.elapsed() < Duration::from_millis(250), + "drop signals and detaches instead of joining the running decode" + ); + + releaser.join().expect("the releaser thread joins"); + control.wait_until_dropped(); + } } diff --git a/crates/gateway-stt-engine/src/worker.rs b/crates/gateway-stt-engine/src/worker.rs index d068178b7..db91b78ae 100644 --- a/crates/gateway-stt-engine/src/worker.rs +++ b/crates/gateway-stt-engine/src/worker.rs @@ -92,6 +92,10 @@ impl Transcriber { reply_rx.await.map_err(|_| TranscribeError::WorkerGone)? } + /// Closes the job queue and joins the worker thread. + /// + /// This is the blocking, error-reporting path; `Drop` only signals + /// and detaches through [`Transcriber::signal_and_detach`]. pub(super) fn shutdown(&self) -> Result<(), TranscribeError> { self.stopping.store(true, Ordering::Release); let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); @@ -107,12 +111,21 @@ impl Transcriber { } pub(super) fn abandon_startup(&self) { - self.stopping.store(true, Ordering::Release); - let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); - drop(state.job_tx.take()); // Construction is non-preemptible. Dropping this handle explicitly // abandons only a timed-out startup worker so the host can classify // the fatal outcome without claiming the thread was stopped. + self.signal_and_detach(); + } + + /// Signals the worker and detaches its thread without joining. + /// + /// The worker captures only owned or `Arc` state (the factory, the job + /// receiver, the stop flag) and borrows nothing from this handle, so a + /// detached thread finishes any running decode on its own. + pub(super) fn signal_and_detach(&self) { + self.stopping.store(true, Ordering::Release); + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + drop(state.job_tx.take()); drop(state.worker.take()); } @@ -137,7 +150,9 @@ impl Transcriber { impl Drop for Transcriber { fn drop(&mut self) { - drop(self.shutdown()); + // `shutdown` is the blocking, error-reporting path; Drop can + // neither wait nor report, so it signals and detaches. + self.signal_and_detach(); } } @@ -416,6 +431,44 @@ mod tests { assert_eq!(control.calls(), 2); } + #[test] + fn drop_signals_and_detaches_instead_of_joining_a_running_decode() { + let (worker, control) = parked_worker(DecodeMode::Interim, INTERIM_JOB_CAPACITY); + let reply = worker + .submit(request(DecodeMode::Interim)) + .expect("running job is admitted"); + control.wait_for( + |state| state.phase == ParkPhase::Entered, + "job enters the decoder", + ); + // The delayed releaser turns a blocking join into a failed timing + // assertion instead of a deadlocked test. + let releaser = std::thread::spawn({ + let control = control.clone(); + move || { + std::thread::sleep(Duration::from_millis(500)); + control.release(); + } + }); + + let started = std::time::Instant::now(); + drop(worker); + assert!( + started.elapsed() < Duration::from_millis(250), + "drop signals and detaches instead of joining the running decode" + ); + + releaser.join().expect("the releaser thread joins"); + control.wait_for( + |state| state.dropped, + "the detached worker finishes the decode and drops the decoder", + ); + assert!( + reply.blocking_recv().is_err(), + "a stopped worker discards the in-flight reply" + ); + } + #[test] fn shutdown_joins_the_worker_and_is_idempotent() { let (worker, control) = parked_worker(DecodeMode::Interim, INTERIM_JOB_CAPACITY); diff --git a/crates/gateway-stt/src/audio.rs b/crates/gateway-stt/src/audio.rs index 9de7ab46c..4cef526a3 100644 --- a/crates/gateway-stt/src/audio.rs +++ b/crates/gateway-stt/src/audio.rs @@ -15,7 +15,8 @@ pub(super) const MIN_COMMIT_SAMPLES: usize = #[derive(Debug, Eq, PartialEq, thiserror::Error)] pub(super) enum AudioError { #[error("audio must be canonical padded Base64")] - InvalidBase64, + #[non_exhaustive] + InvalidBase64(#[source] base64::DecodeError), #[error("decoded audio exceeds the {max_bytes} byte append limit")] AppendTooLarge { max_bytes: usize }, #[error("PCM16 audio ended with an incomplete sample")] @@ -170,7 +171,7 @@ pub(super) fn decode_base64(payload: &str) -> Result, AudioError> { } let decoded = base64::engine::general_purpose::STANDARD .decode(payload) - .map_err(|_| AudioError::InvalidBase64)?; + .map_err(AudioError::InvalidBase64)?; if decoded.len() > MAX_APPEND_AUDIO_BYTES { return Err(AudioError::AppendTooLarge { max_bytes: MAX_APPEND_AUDIO_BYTES, @@ -262,15 +263,20 @@ mod tests { #[test] fn base64_rejects_invalid_and_noncanonical_encodings_and_decoded_oversize() { - assert_eq!(decode_base64("%%%"), Err(AudioError::InvalidBase64)); - assert_eq!(decode_base64("YQ"), Err(AudioError::InvalidBase64)); + assert!(matches!( + decode_base64("%%%"), + Err(AudioError::InvalidBase64(_)) + )); + assert!(matches!( + decode_base64("YQ"), + Err(AudioError::InvalidBase64(_)) + )); for alias in [ "YR==", "YS==", "YT==", "YU==", "YV==", "YW==", "YX==", "YY==", "YZ==", "Ya==", "Yb==", "Yc==", "Yd==", "Ye==", "Yf==", "YWJ=", "YWK=", "YWL=", "YQ===", "YWI==", "YWJj=", ] { - assert_eq!( - decode_base64(alias), - Err(AudioError::InvalidBase64), + assert!( + matches!(decode_base64(alias), Err(AudioError::InvalidBase64(_))), "{alias}" ); } @@ -290,6 +296,23 @@ mod tests { ); } + #[test] + fn invalid_base64_carries_the_decode_failure_as_its_source() { + use std::error::Error as _; + + let error = decode_base64("%%%").expect_err("invalid Base64 is rejected"); + let AudioError::InvalidBase64(source) = &error else { + panic!("invalid Base64 names its cause: {error}"); + }; + assert_eq!(*source, base64::DecodeError::InvalidByte(0, b'%')); + assert!( + error + .source() + .is_some_and(<(dyn std::error::Error + 'static)>::is::), + "the error chain carries the decoder failure" + ); + } + #[test] fn odd_byte_carry_and_resampling_match_unsplit_input() { let input = (0..MIN_COMMIT_SAMPLES + 5) diff --git a/crates/gateway-stt/src/realtime/route.rs b/crates/gateway-stt/src/realtime/route.rs index 6a1ae7f49..672fae12a 100644 --- a/crates/gateway-stt/src/realtime/route.rs +++ b/crates/gateway-stt/src/realtime/route.rs @@ -300,7 +300,7 @@ fn event_id(event: &ClientEvent) -> Option { } fn session_error(error: &SessionError, client_event_id: Option) -> ClientError { match error { - SessionError::Audio(AudioError::InvalidBase64) => ClientError::request( + SessionError::Audio(AudioError::InvalidBase64(_)) => ClientError::request( "invalid_base64_audio", "Audio must be valid Base64", Some("audio"), @@ -367,7 +367,7 @@ fn session_error(error: &SessionError, client_event_id: Option) -> Clien SessionError::EpochExhausted | SessionError::CanceledTaskFailed | SessionError::GenerationUnavailable - | SessionError::Inference + | SessionError::Inference(_) | SessionError::Finalization(_) | SessionError::Mailbox(MailboxError::TerminalAlreadySet | MailboxError::UnknownItem) => { ClientError::server( diff --git a/crates/gateway-stt/src/realtime/session.rs b/crates/gateway-stt/src/realtime/session.rs index 7a3dbfd56..c4abb94a2 100644 --- a/crates/gateway-stt/src/realtime/session.rs +++ b/crates/gateway-stt/src/realtime/session.rs @@ -411,10 +411,10 @@ mod tests { .expect("retry input exists") .item_id() .to_owned(); - assert_eq!( + assert!(matches!( session.commit(), Err(SessionError::CommittedItemsAtCapacity) - ); + )); assert_eq!(session.input().expect("input remains").item_id(), retry_id); session @@ -445,7 +445,10 @@ mod tests { session .spawn_interim(pending()) .expect("capacity-plus-one task starts"); - assert_eq!(session.clear(), Err(SessionError::CancelJoinAtCapacity)); + assert!(matches!( + session.clear(), + Err(SessionError::CancelJoinAtCapacity) + )); assert!( session.input().is_some(), "recoverable error preserves input" diff --git a/crates/gateway-stt/src/realtime/session/items/tests.rs b/crates/gateway-stt/src/realtime/session/items/tests.rs index 076c383c3..0972e6805 100644 --- a/crates/gateway-stt/src/realtime/session/items/tests.rs +++ b/crates/gateway-stt/src/realtime/session/items/tests.rs @@ -113,22 +113,25 @@ async fn blocked_interim_keeps_exact_budget_until_worker_retirement_and_commit_r |session| async { assert_eq!(probe.retained_samples(), 16_002); cancel_generation_epoch(&service, session); - assert!(matches!( - session.finish_interim().await, - Err(SessionError::Inference) - )); + let Err(SessionError::Inference(source)) = session.finish_interim().await else { + panic!("a cancelled generation fails the interim with its engine cause"); + }; + assert!( + !source.to_string().is_empty(), + "the restored chain carries the engine failure" + ); assert_eq!( probe.retained_samples(), 16_002, "epoch cancellation cannot release worker-owned PCM" ); - assert_eq!( + assert!(matches!( session.commit(), Err(SessionError::Audio(AudioError::BufferTooLong { maximum_seconds: 30, })) - ); + )); assert_eq!(session.results.reserved_items(), 0); assert_eq!(session.committed.len(), 0); assert_eq!( diff --git a/crates/gateway-stt/src/realtime/session/route.rs b/crates/gateway-stt/src/realtime/session/route.rs index ee9e7aed3..f49382535 100644 --- a/crates/gateway-stt/src/realtime/session/route.rs +++ b/crates/gateway-stt/src/realtime/session/route.rs @@ -59,10 +59,7 @@ impl Session { self.interim_task = Some(tokio::spawn(async move { let request = DecodeRequest::new(DecodeMode::Interim, samples, guidance, finalized) .with_lifetime_guard(samples_owner); - let transcript = engine - .decode(request) - .await - .map_err(|error| error.to_string()); + let transcript = engine.decode(request).await; InterimTaskOutput::Decode { epoch, item_id, @@ -106,7 +103,7 @@ impl Session { if input.item_id() != item_id { return Ok(None); } - let transcript = transcript.map_err(|_| SessionError::Inference)?; + let transcript = transcript.map_err(SessionError::Inference)?; if transcript.is_empty() { return Ok(None); } diff --git a/crates/gateway-stt/src/realtime/session/state.rs b/crates/gateway-stt/src/realtime/session/state.rs index a890457e8..49606c6fb 100644 --- a/crates/gateway-stt/src/realtime/session/state.rs +++ b/crates/gateway-stt/src/realtime/session/state.rs @@ -5,6 +5,7 @@ use crate::realtime::item::CommittedItem; use crate::realtime::registry::SessionRegistration; use crate::realtime::result_mailbox::{MailboxError, ResultMailbox}; use crate::realtime::wire::{EffectiveSession, IdGenerator}; +use gateway_stt_engine::TranscribeError; use std::collections::HashMap; use tokio::task::JoinHandle; pub(super) const SESSION_CANCEL_JOIN_CAPACITY: usize = 8; @@ -22,10 +23,10 @@ pub(super) enum InterimTaskOutput { segment_start: u64, audio_start: u64, audio_end: u64, - transcript: Result, + transcript: Result, }, } -#[derive(Debug, Eq, PartialEq, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub(crate) enum SessionError { #[error(transparent)] Audio(#[from] AudioError), @@ -42,7 +43,8 @@ pub(crate) enum SessionError { #[error("speech generation is unavailable")] GenerationUnavailable, #[error("transcription failed")] - Inference, + #[non_exhaustive] + Inference(#[source] TranscribeError), #[error("the realtime session result capacity is reached")] InterimAtCapacity, #[error("{0}")] diff --git a/crates/workshop-server/src/session_agents.rs b/crates/workshop-server/src/session_agents.rs index d6972323f..91e0539b3 100644 --- a/crates/workshop-server/src/session_agents.rs +++ b/crates/workshop-server/src/session_agents.rs @@ -249,7 +249,8 @@ impl AgentSessions { .map_err(|source| LaunchRefusal::SessionState { source })?, ); let (supervisor_events, events) = mpsc::unbounded_channel(); - let lifecycle = Arc::new(RunLifecycle::new(supervisor_events)); + let (cancellations, cancellation_events) = mpsc::channel(lifecycle::CANCELLATION_CAPACITY); + let lifecycle = Arc::new(RunLifecycle::new(supervisor_events, cancellations)); let waits = Arc::new(WaitRegistry::new()); let (input_frames, _) = broadcast::channel(INPUT_CAPACITY); let (deltas, _) = broadcast::channel(DELTA_CAPACITY); @@ -273,6 +274,7 @@ impl AgentSessions { self.inner.host.clone(), self.inner.gateway.clone(), events, + cancellation_events, ); Ok(session) } @@ -1011,13 +1013,14 @@ mod tests { let menu = MenuBus::new(catalog.clone(), None); let (errors, mut errors_rx) = broadcast::channel(ERROR_CAPACITY); let (supervisor_events, _events) = mpsc::unbounded_channel(); + let (cancellations, _cancellation_events) = mpsc::channel(lifecycle::CANCELLATION_CAPACITY); let observer = SessionObserver { log: Arc::new(WorkshopObserver::new(None).expect("a memory log")), rounds: Arc::new(AtomicU64::new(0)), push: Push::new(status, catalog, menu), backoff: ReconnectBackoff::new(), errors, - lifecycle: Arc::new(RunLifecycle::new(supervisor_events)), + lifecycle: Arc::new(RunLifecycle::new(supervisor_events, cancellations)), }; observer.observe("run", "chat", Observation::ModelTurnFailed); diff --git a/crates/workshop-server/src/session_agents/lifecycle.rs b/crates/workshop-server/src/session_agents/lifecycle.rs index 7d72882c8..f20b83224 100644 --- a/crates/workshop-server/src/session_agents/lifecycle.rs +++ b/crates/workshop-server/src/session_agents/lifecycle.rs @@ -7,10 +7,21 @@ use tokio::sync::mpsc; use super::supervisor::transition::{RunId, SupervisorEvent}; +/// Capacity of the bounded operator-cancellation queue. +/// +/// `OperatorCancellation` is the one loss-tolerant supervisor event: no +/// reducer wait is ever conditioned on it, and a full queue already holds +/// a pending cancellation that retires the current run, so dropping a +/// concurrent duplicate preserves semantics. Producers are operator +/// gestures, so one pending cancellation covers the entire in-flight set +/// with headroom. +pub(super) const CANCELLATION_CAPACITY: usize = 1; + /// Synchronous producers for one supervisor's typed event stream. pub(super) struct RunLifecycle { state: Mutex, events: mpsc::UnboundedSender, + cancellations: mpsc::Sender, } /// The current run identity and cancellation handle. @@ -20,14 +31,20 @@ struct RunState { } impl RunLifecycle { - /// Creates the lifecycle over the supervisor's event sender. - pub(super) fn new(events: mpsc::UnboundedSender) -> Self { + /// Creates the lifecycle over the supervisor's event senders: the + /// unbounded queue carries the loss-intolerant events the reducer + /// waits on, the bounded queue carries operator cancellations. + pub(super) fn new( + events: mpsc::UnboundedSender, + cancellations: mpsc::Sender, + ) -> Self { Self { state: Mutex::new(RunState { cancel: CancelHandle::new(), run: None, }), events, + cancellations, } } @@ -46,8 +63,14 @@ impl RunLifecycle { } /// Publishes an operator cancellation for reducer ownership. + /// + /// Loss-tolerant by design: when the bounded queue is full it already + /// holds a pending cancellation that retires the current run, so a + /// concurrent duplicate is dropped rather than queued. pub(super) fn operator_cancel(&self) { - self.send(SupervisorEvent::OperatorCancellation); + let _ = self + .cancellations + .try_send(SupervisorEvent::OperatorCancellation); } /// Publishes that input resumed the currently armed run. @@ -87,8 +110,65 @@ impl RunLifecycle { self.send(SupervisorEvent::Close); } - /// Sends one event; a gone receiver means supervision already ended. + /// Sends one loss-intolerant event; a gone receiver means supervision + /// already ended. These events ride the unbounded queue because the + /// reducer awaits settlements and close, so their loss could hang a + /// state transition, and their volume is bounded by armed runs and + /// durable turns rather than by caller repetition. fn send(&self, event: SupervisorEvent) { let _ = self.events.send(event); } } + +#[cfg(test)] +mod tests { + use super::*; + + fn lifecycle() -> ( + RunLifecycle, + mpsc::UnboundedReceiver, + mpsc::Receiver, + ) { + let (events, guaranteed) = mpsc::unbounded_channel(); + let (cancellations, bounded) = mpsc::channel(CANCELLATION_CAPACITY); + ( + RunLifecycle::new(events, cancellations), + guaranteed, + bounded, + ) + } + + #[test] + fn operator_cancellations_never_grow_the_bounded_queue_past_capacity() { + let (lifecycle, _guaranteed, mut bounded) = lifecycle(); + + for _ in 0..8 { + lifecycle.operator_cancel(); + } + + let mut received = 0; + while bounded.try_recv().is_ok() { + received += 1; + } + assert_eq!( + received, CANCELLATION_CAPACITY, + "a full cancellation queue drops redundant duplicates instead of growing" + ); + } + + #[test] + fn guaranteed_events_flow_past_a_full_cancellation_queue() { + let (lifecycle, mut guaranteed, _bounded) = lifecycle(); + for _ in 0..4 { + lifecycle.operator_cancel(); + } + + lifecycle.close(); + + assert_eq!( + guaranteed.try_recv().expect("close is delivered"), + SupervisorEvent::Close, + "loss-intolerant events keep guaranteed delivery when cancellations overflow" + ); + } +} diff --git a/crates/workshop-server/src/session_agents/supervisor.rs b/crates/workshop-server/src/session_agents/supervisor.rs index eb187a74c..1ed0a6b73 100644 --- a/crates/workshop-server/src/session_agents/supervisor.rs +++ b/crates/workshop-server/src/session_agents/supervisor.rs @@ -26,6 +26,7 @@ pub(super) fn spawn( host: SessionHost, gateway: GatewayBinding, lifecycle: mpsc::UnboundedReceiver, + cancellations: mpsc::Receiver, ) { tokio::spawn(async move { let tool: Arc = Arc::new(UserInputTool::new( @@ -41,7 +42,7 @@ pub(super) fn spawn( } }; let (mut collector, initial_catalog, initial_gateway) = - EventCollector::new(lifecycle, host.catalog.clone(), gateway); + EventCollector::new(lifecycle, cancellations, host.catalog.clone(), gateway); let mut executor = EffectExecutor::new( Arc::clone(&session), host, diff --git a/crates/workshop-server/src/session_agents/supervisor/events.rs b/crates/workshop-server/src/session_agents/supervisor/events.rs index e5d1261ce..8aa6df026 100644 --- a/crates/workshop-server/src/session_agents/supervisor/events.rs +++ b/crates/workshop-server/src/session_agents/supervisor/events.rs @@ -33,6 +33,7 @@ pub(super) enum CollectedEvent { /// External event sources owned by one supervisor. pub(super) struct EventCollector { lifecycle: mpsc::UnboundedReceiver, + cancellations: mpsc::Receiver, catalog: CatalogBus, catalog_generation: watch::Receiver, gateway: GatewayBinding, @@ -44,6 +45,7 @@ impl EventCollector { /// disappear between those operations. pub(super) fn new( lifecycle: mpsc::UnboundedReceiver, + cancellations: mpsc::Receiver, catalog: CatalogBus, gateway: GatewayBinding, ) -> (Self, CatalogEvent, Arc) { @@ -54,6 +56,7 @@ impl EventCollector { ( Self { lifecycle, + cancellations, catalog, catalog_generation, gateway, @@ -74,7 +77,10 @@ impl EventCollector { if let Some(run) = active_run { tokio::select! { biased; - event = next_lifecycle_event(&mut self.lifecycle) => { + event = next_lifecycle_event( + &mut self.lifecycle, + &mut self.cancellations, + ) => { CollectedEvent::Supervisor(event) } catalog = next_catalog_event( @@ -94,7 +100,10 @@ impl EventCollector { } else { tokio::select! { biased; - event = next_lifecycle_event(&mut self.lifecycle) => { + event = next_lifecycle_event( + &mut self.lifecycle, + &mut self.cancellations, + ) => { CollectedEvent::Supervisor(event) } catalog = next_catalog_event( @@ -126,12 +135,62 @@ async fn next_gateway_event( } } -/// Waits for the next synchronous lifecycle event. +/// Waits for the next synchronous lifecycle event, polling the guaranteed +/// queue before the bounded cancellation queue. Cross-channel ordering is +/// not load-bearing: a cancellation is valid in any reducer phase, and a +/// close or settlement processed late lands on a phase that ignores it. async fn next_lifecycle_event( lifecycle: &mut mpsc::UnboundedReceiver, + cancellations: &mut mpsc::Receiver, ) -> SupervisorEvent { - match lifecycle.recv().await { - Some(event) => event, - None => std::future::pending().await, + tokio::select! { + biased; + event = lifecycle.recv() => match event { + Some(event) => event, + None => std::future::pending().await, + }, + event = cancellations.recv() => match event { + Some(event) => event, + None => std::future::pending().await, + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn the_collector_drains_the_guaranteed_and_bounded_lifecycle_queues() { + let (events, guaranteed) = mpsc::unbounded_channel(); + let (cancellations, bounded) = mpsc::channel(1); + let (mut collector, _initial_catalog, _initial_gateway) = EventCollector::new( + guaranteed, + bounded, + CatalogBus::default(), + GatewayBinding::new("http://127.0.0.1:1", "").expect("the test binding builds"), + ); + + events + .send(SupervisorEvent::Close) + .expect("guaranteed send"); + cancellations + .try_send(SupervisorEvent::OperatorCancellation) + .expect("bounded send"); + + assert!( + matches!( + collector.next(None, None).await, + CollectedEvent::Supervisor(SupervisorEvent::Close) + ), + "the guaranteed queue is polled first" + ); + assert!( + matches!( + collector.next(None, None).await, + CollectedEvent::Supervisor(SupervisorEvent::OperatorCancellation) + ), + "the bounded cancellation queue drains through the same collector" + ); } } diff --git a/crates/workshop/src/gateway/supervisor.rs b/crates/workshop/src/gateway/supervisor.rs index 35ad95256..21ece70b4 100644 --- a/crates/workshop/src/gateway/supervisor.rs +++ b/crates/workshop/src/gateway/supervisor.rs @@ -7,8 +7,8 @@ use std::time::{Duration, Instant}; use anyhow::Context as _; use shared_sidecar::{ - CancellationToken, GatewayDiscoveryFile, LaunchDecision, Resolution, SidecarError, - ValidatedConnection, + CancellationToken, GatewayDiscoveryFile, LaunchDecision, Resolution, ShutdownError, + SidecarError, ValidatedConnection, }; use super::boot; @@ -50,6 +50,14 @@ pub(super) trait SupervisedGatewayIdentity { /// Disarms cleanup after this identity becomes authoritative. fn publication_succeeded(&mut self) {} + + /// Shuts down an unpublished owned child through the explicit, + /// error-reporting path. Identities that own no child do nothing. + fn shutdown_unpublished(self) + where + Self: Sized, + { + } } /// A validated recovery process whose pid proves it is the child we spawned. @@ -88,6 +96,23 @@ impl RecoveryCandidate { pub(super) fn published(&mut self) { self.published = true; } + + /// Shuts down the unpublished recovered child within the late-child + /// budget. + /// + /// This is the blocking, error-reporting path; `Drop` only signals on + /// a detached thread. The drop signal is disarmed either way: the + /// caller receives the outcome, so a failed delivery is reported here + /// rather than retried silently. + pub(super) fn shutdown(mut self) -> Result<(), ShutdownError> { + if self.published { + return Ok(()); + } + debug_assert_eq!(self.child_pid, self.validated.pid()); + self.published = true; + let deadline = Instant::now() + LATE_CHILD_SHUTDOWN_BUDGET; + shared_sidecar::request_shutdown_before(&self.validated, deadline) + } } impl Drop for RecoveryCandidate { @@ -96,12 +121,23 @@ impl Drop for RecoveryCandidate { return; } debug_assert_eq!(self.child_pid, self.validated.pid()); - let deadline = Instant::now() + LATE_CHILD_SHUTDOWN_BUDGET; - if let Err(error) = shared_sidecar::request_shutdown_before(&self.validated, deadline) { - // Drop has no error return channel. The bounded authenticated - // request is best effort, so diagnostics are the only place this - // cleanup failure can be surfaced without aborting teardown. - eprintln!("could not shut down an unpublished recovered gateway: {error}"); + // Drop can neither block nor report: the bounded authenticated + // request runs on a detached thread, so a missed explicit + // `shutdown()` still signals the unpublished gateway process. + let validated = self.validated.clone(); + let signalled = std::thread::Builder::new() + .name("gateway-late-child-shutdown".to_owned()) + .spawn(move || { + let deadline = Instant::now() + LATE_CHILD_SHUTDOWN_BUDGET; + if let Err(error) = shared_sidecar::request_shutdown_before(&validated, deadline) { + // The detached signal has no error return channel, so + // diagnostics are the only place this cleanup failure + // can surface. + eprintln!("could not shut down an unpublished recovered gateway: {error}"); + } + }); + if let Err(error) = signalled { + eprintln!("could not signal an unpublished recovered gateway: {error}"); } } } @@ -137,6 +173,14 @@ impl SupervisedGatewayIdentity for RecoveryIdentity { candidate.published(); } } + + fn shutdown_unpublished(self) { + if let Self::Candidate(candidate) = self + && let Err(error) = candidate.shutdown() + { + eprintln!("could not shut down an unpublished recovered gateway: {error}"); + } + } } /// The running local-sidecar supervisor. @@ -344,6 +388,9 @@ impl GatewaySupervisor { } /// Revokes publication, requests stop, and waits at most one deadline. + /// + /// This is the blocking, outcome-reporting path; `Drop` only signals + /// and detaches. pub(crate) fn shutdown(mut self) -> SupervisorShutdown { self.stop_and_join() } @@ -380,7 +427,16 @@ impl GatewaySupervisor { impl Drop for GatewaySupervisor { fn drop(&mut self) { - let _ = self.stop_and_join(); + // `shutdown()` is the bounded, outcome-reporting path. Drop can + // neither wait nor report, so it revokes publication, signals the + // stop, and detaches both threads; the worker captures only owned + // state, so a detached thread finishes on its own. + if let Some(publication) = self.publication.as_ref() { + publication.close_publication(); + } + self.stop.signal(); + drop(self.thread.take()); + drop(self.stop_bridge.take()); } } @@ -536,6 +592,10 @@ pub(super) fn run_supervision( } Err(error) => { eprintln!("could not publish a replacement local gateway: {error}"); + // A recovered child this process launched stays + // unpublished, so it is shut down through the + // explicit, error-reporting path. + identity.shutdown_unpublished(); } }, Err(error) => { diff --git a/crates/workshop/src/gateway/tests/recovery.rs b/crates/workshop/src/gateway/tests/recovery.rs index fc91b9ed4..360671188 100644 --- a/crates/workshop/src/gateway/tests/recovery.rs +++ b/crates/workshop/src/gateway/tests/recovery.rs @@ -1,6 +1,8 @@ //! Continuous supervision, recovery, and joined-shutdown coverage. use std::cell::{Cell, RefCell}; +use std::io::{Read as _, Write as _}; +use std::net::TcpListener; use std::path::Path; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, mpsc}; @@ -206,6 +208,108 @@ fn an_exact_spawned_pid_authenticates_late_child_cleanup() { ); } +#[test] +fn explicit_candidate_shutdown_delivers_and_disarms_the_drop_signal() { + let mut gateway = validated_gateway("owned-key"); + let validated = gateway.validate("owned-key", 1_778_000_001, "2026-09-08T18:00:01Z"); + let RecoveryOwnership::Owned(candidate) = + RecoveryCandidate::authenticate(validated.pid(), validated) + else { + panic!("the validated file names the spawned child"); + }; + + candidate + .shutdown() + .expect("the fixture accepts the authenticated shutdown"); + + assert!( + gateway.received_shutdown(Duration::from_secs(1)), + "the explicit path signals the unpublished child" + ); + assert!( + !gateway.received_shutdown(Duration::from_millis(100)), + "the disarmed drop sends no second signal" + ); +} + +#[test] +fn explicit_candidate_shutdown_reports_a_delivery_failure() { + let validated = { + let gateway = validated_gateway("owned-key"); + gateway.validate("owned-key", 1_778_000_001, "2026-09-08T18:00:01Z") + }; + let RecoveryOwnership::Owned(candidate) = + RecoveryCandidate::authenticate(validated.pid(), validated) + else { + panic!("the validated file names the spawned child"); + }; + + let error = candidate + .shutdown() + .expect_err("a dead child cannot accept the shutdown"); + assert!( + matches!(error, shared_sidecar::ShutdownError::Io { .. }), + "the explicit path reports the delivery failure: {error}" + ); +} + +#[test] +fn dropping_a_candidate_signals_without_waiting_for_an_unresponsive_child() { + // The fixture child lends its validatable pid; the hanging listener + // answers the validation probe, then parks the shutdown connection. + let gateway = validated_gateway("hanging-key"); + let reference = gateway.validate("hanging-key", 1_778_000_001, "2026-09-08T18:00:01Z"); + let hang = Arc::new(AtomicBool::new(false)); + let listener = TcpListener::bind("127.0.0.1:0").expect("bind the hanging fixture"); + let port = listener.local_addr().expect("the fixture address").port(); + std::thread::spawn({ + let hang = Arc::clone(&hang); + move || { + while let Ok((mut stream, _)) = listener.accept() { + let mut buffer = [0_u8; 1024]; + if hang.load(Ordering::SeqCst) { + let _ = stream.read(&mut buffer); + std::thread::sleep(Duration::from_secs(5)); + continue; + } + while let Ok(read) = stream.read(&mut buffer) { + if read == 0 { + break; + } + if stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .is_err() + { + break; + } + } + } + } + }); + let file = GatewayDiscoveryFile { + port, + api_key: "hanging-key".to_owned(), + pid: reference.pid(), + epoch: 1_778_000_001, + version: "test".to_owned(), + started_at: "2026-09-08T18:00:01Z".to_owned(), + }; + let validated = ValidatedConnection::validate(file).expect("the hanging endpoint validates"); + hang.store(true, Ordering::SeqCst); + let RecoveryOwnership::Owned(candidate) = + RecoveryCandidate::authenticate(validated.pid(), validated) + else { + panic!("the validated file names the spawned child"); + }; + + let started = Instant::now(); + drop(candidate); + assert!( + started.elapsed() < Duration::from_millis(250), + "drop signals on a detached thread instead of waiting out the late-child budget" + ); +} + #[test] fn a_mismatched_spawned_pid_never_claims_or_cleans_the_validated_process() { let mut gateway = validated_gateway("unowned-key"); diff --git a/crates/workshop/src/gateway/tests/shutdown.rs b/crates/workshop/src/gateway/tests/shutdown.rs index bf5c78dd2..f330e8573 100644 --- a/crates/workshop/src/gateway/tests/shutdown.rs +++ b/crates/workshop/src/gateway/tests/shutdown.rs @@ -74,9 +74,9 @@ fn a_spurious_completion_wake_cannot_trigger_a_blocking_join() { } #[test] -fn dropping_a_supervisor_uses_the_same_bounded_detach_path() { +fn dropping_a_supervisor_signals_and_detaches_without_waiting() { let (release, blocked) = mpsc::channel(); - let supervisor = GatewaySupervisor::spawn_with_budget(Duration::from_millis(50), move |_| { + let supervisor = GatewaySupervisor::spawn_with_budget(Duration::from_secs(1), move |_| { let _ = blocked.recv(); }) .expect("spawn test supervisor"); @@ -86,7 +86,7 @@ fn dropping_a_supervisor_uses_the_same_bounded_detach_path() { assert!( started.elapsed() < Duration::from_millis(250), - "Drop cannot wait beyond the supervisor shutdown budget" + "Drop signals and detaches instead of waiting out the shutdown budget" ); release.send(()).expect("release the detached test worker"); } diff --git a/vibe-ledger.md b/vibe-ledger.md index 34facbc7c..1afe4b127 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -58,3 +58,5 @@ - Async STT boot reset Step 6: Reconcile documentation and qualify - Full verification passed: build, fmt, warnings-denied clippy (portable and staged Workshop), portable workspace and doc tests, feature-disabled Gateway, both UI suites, staged Workshop tests with guaranteed cleanup, both Miri lanes, all five native Whisper lanes with hash-pinned fixtures, guide assembler with zero drift, mdBook build, and `cargo workshop` package construction. Operator confirmed physical microphone hypothesis revision, authoritative completion, and second take on the release binaries. - Rulebook debt tiers Step 1: CI doctest coverage and dependency hygiene - component-scope verify pass: build, `cargo fmt --all --check`, warnings-denied clippy, and `cargo test --locked -p gateway-stt` (121 unit + 52 integration) green; nextest unavailable locally, cargo test fallback recorded in verify-step-1-round-1.log. Decision: `ci-green` treats cancelled as failure via an `if: always()` loop over `needs.*.result` | Falsifier: a cancelled or newly added job shows green as a required status check. Decision: doctest step placed in `check-workshop` (Windows) rather than the Linux `test` job | Falsifier: workshop doctests run twice or not at all in CI. + +- Rulebook debt tiers Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel - component-scope verify pass: gateway and workshop builds, `cargo fmt --all --check`, both clippy invocations, nextest across the four touched crates, and the gateway-stt integration suite green (verify-step-2-round-1.log). Decision: `OperatorCancellation` is the one loss-tolerant event and alone rides `mpsc::channel(1)`; `AcceptedInput`/`TerminalSettlement`/`Close` keep guaranteed delivery on the unbounded queue | Falsifier: a reducer transition that awaits an operator cancellation. Decision: `RecoveryCandidate::shutdown` disarms the drop signal even on failure since the caller owns the outcome | Falsifier: a failure class where a silent drop retry recovers the child. Decision: `SessionError` dropped `Eq/PartialEq` because `TranscribeError` is not `PartialEq`; test comparisons became `matches!` | Falsifier: a production `SessionError` equality comparison. Decision: reject cargo-nextest install in the fix round, accept the passing cargo test fallback | Falsifier: a later round requires nextest-specific behavior. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md index c5e443a17..d8c36fe73 100644 --- a/vibe/2026-09-11-1-rulebook-debt-tiers.md +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -203,7 +203,7 @@ Verification: the workflow lints clean, `gateway-stt` builds and its tests pass -### Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel +### Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel [completed] - Component: runtime-hazard-repairs From 53004d3f2d608463c31e371c7d0e63971f4781a4 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 09:32:06 -0700 Subject: [PATCH 3/7] Add non_exhaustive attributes and convert allows to expects Marks seven public types and error variants non-exhaustive so each can gain fields or variants without breaking downstream matches, and converts lint suppressions across the workspace from blanket allows to reasoned expects so a suppression that stops firing fails the build. Suppressions that had already gone stale were deleted outright, and match sites downstream of the newly non-exhaustive decode mode gained wildcard arms. Runtime behavior is unchanged apart from one defensive exit path. - `#[non_exhaustive]` now marks `DecodeMode`, `DecodeRequest`, `EnginePolicy`, `GatewayStartup`, `ValidatedConnection`, and the `OwnerTimeout` and `GatewayPublicationError::Build` variants, so downstream matches must carry a wildcard arm and each type can grow without a breaking change. - `Ok(_) =>` in `crates/gateway/src/main.rs` prints an error and returns `ExitCode::FAILURE` on any unrecognized future startup decision instead of falling through. - `#[cfg_attr(` gates the suppressions that only fire in non-test builds (`FaultInjector`, `ProvisionModel`, `UnloadModel`, `sys_live_handle`, `DelimiterGroup::family`), keeping test builds free of unfulfilled expectations. - `_ => return Ok(None)` in `WhisperModelFactory::load` treats an unrecognized decode mode as no model to load, while the scripted test factories `unreachable!` on one because their scripts only produce interim and final. - `#[allow(dead_code)]` on `GatewaySource::ready`, `Request::Mcp`, `BindSchema`, and `NearDuplicateTools` was stale and is deleted rather than converted. - `#[allow(clippy::expect_used)]` on `blocked_interim_keeps_exact_budget_until_worker_retirement_and_commit_retries`, `blocked_final_keeps_budget_after_epoch_cancellation_until_worker_retirement`, and `append_after_retirement` was stale and deleted. - `#[allow(unused_imports)]` on the `parse_client_event` and `shared::{ClientError, ClientEvent, IdGenerator}` re-exports was stale and deleted; the `server` re-export keeps its suppression as an expect. - `clippy::ref_as_ptr` was dropped from the `bridge.rs` test-module suppression list as no longer firing. Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- crates/gateway-logging/src/worker.rs | 25 ++++++++++++++++--- .../gateway-stt-backend-whisper/src/model.rs | 1 + crates/gateway-stt-engine/src/decoder.rs | 2 ++ crates/gateway-stt-engine/src/policy.rs | 1 + .../tests/engine_contract.rs | 1 + .../tests/startup_cleanup.rs | 1 + crates/gateway-stt/src/audio.rs | 10 ++++++-- .../src/realtime/session/items/tests.rs | 14 +++-------- crates/gateway-stt/src/realtime/wire.rs | 10 +------- crates/gateway-stt/src/test_fixtures/hour.rs | 1 + crates/gateway-stt/tests/it/initial_load.rs | 2 +- .../tests/it/realtime_forced_windows.rs | 16 +++++------- crates/gateway/src/commands.rs | 18 ++++++++----- crates/gateway/src/main.rs | 4 +++ crates/gateway/src/relaunch.rs | 2 ++ .../src/untrusted/inventory.rs | 8 +++++- .../promptforge-core/benches/models_loop.rs | 5 +++- crates/promptforge-core/src/error.rs | 2 -- .../promptforge-core/src/execute/context.rs | 5 +++- .../promptforge-core/src/execute/gateway.rs | 2 -- crates/promptforge-lua/benches/surface.rs | 5 +++- crates/promptforge-lua/src/protocol.rs | 4 --- crates/promptforge-lua/src/vm.rs | 7 ++++-- crates/shared-sidecar/src/validated.rs | 1 + crates/workshop-server/src/gateway_binding.rs | 1 + crates/workshop/src/bridge.rs | 13 ++++++---- vibe-ledger.md | 2 ++ vibe/2026-09-11-1-rulebook-debt-tiers.md | 2 +- 28 files changed, 103 insertions(+), 62 deletions(-) diff --git a/crates/gateway-logging/src/worker.rs b/crates/gateway-logging/src/worker.rs index aaa41a2ae..dc9a0cc00 100644 --- a/crates/gateway-logging/src/worker.rs +++ b/crates/gateway-logging/src/worker.rs @@ -48,7 +48,14 @@ struct FaultInjector { } impl FaultInjector { - #[cfg_attr(not(test), allow(clippy::unused_self, clippy::unnecessary_wraps))] + #[cfg_attr( + not(test), + expect( + clippy::unused_self, + clippy::unnecessary_wraps, + reason = "in non-test builds the fault injector is inert: checkpoint ignores self and never fails" + ) + )] fn checkpoint(&mut self, operation: &'static str) -> io::Result<()> { #[cfg(not(test))] let _ = operation; @@ -66,7 +73,13 @@ impl FaultInjector { Ok(()) } - #[cfg_attr(not(test), allow(clippy::unused_self))] + #[cfg_attr( + not(test), + expect( + clippy::unused_self, + reason = "in non-test builds the injector state is cfg'd out, so the method ignores self" + ) + )] fn is_simulated_crash(&self) -> bool { #[cfg(test)] { @@ -78,7 +91,13 @@ impl FaultInjector { } } - #[cfg_attr(not(test), allow(clippy::unused_self))] + #[cfg_attr( + not(test), + expect( + clippy::unused_self, + reason = "in non-test builds the injector state is cfg'd out, so the method ignores self" + ) + )] fn record_commit_marker(&mut self) { #[cfg(test)] { diff --git a/crates/gateway-stt-backend-whisper/src/model.rs b/crates/gateway-stt-backend-whisper/src/model.rs index 143831647..70a1660e0 100644 --- a/crates/gateway-stt-backend-whisper/src/model.rs +++ b/crates/gateway-stt-backend-whisper/src/model.rs @@ -69,6 +69,7 @@ impl ModelFactory for WhisperModelFactory { }; (path, "final") } + _ => return Ok(None), }; let progress = self .config diff --git a/crates/gateway-stt-engine/src/decoder.rs b/crates/gateway-stt-engine/src/decoder.rs index 1b65cabc5..6c0829782 100644 --- a/crates/gateway-stt-engine/src/decoder.rs +++ b/crates/gateway-stt-engine/src/decoder.rs @@ -7,6 +7,7 @@ use crate::TranscribeError; /// Selects the physical worker and backend decode policy for one request. #[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[non_exhaustive] pub enum DecodeMode { /// Responsive provisional transcription. Interim, @@ -16,6 +17,7 @@ pub enum DecodeMode { /// One complete stateless decode job. #[derive(Clone, Debug)] +#[non_exhaustive] pub struct DecodeRequest { mode: DecodeMode, samples: RequestSamples, diff --git a/crates/gateway-stt-engine/src/policy.rs b/crates/gateway-stt-engine/src/policy.rs index 4e8a97705..f4f60fdcf 100644 --- a/crates/gateway-stt-engine/src/policy.rs +++ b/crates/gateway-stt-engine/src/policy.rs @@ -9,6 +9,7 @@ const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(120); /// Checked capture, startup, and backend capability policy. #[derive(Clone, Copy, Debug)] +#[non_exhaustive] pub struct EnginePolicy { window_samples: usize, interval: Duration, diff --git a/crates/gateway-stt-engine/tests/engine_contract.rs b/crates/gateway-stt-engine/tests/engine_contract.rs index 72b2a71d2..02225ebc9 100644 --- a/crates/gateway-stt-engine/tests/engine_contract.rs +++ b/crates/gateway-stt-engine/tests/engine_contract.rs @@ -112,6 +112,7 @@ impl ModelFactory for FinalFailingModelFactory { PathBuf::from(FINAL_INIT_SENTINEL), std::io::Error::other(FINAL_INIT_SENTINEL), )), + _ => unreachable!("the engine contract factory scripts only interim and final modes"), } } } diff --git a/crates/gateway-stt-engine/tests/startup_cleanup.rs b/crates/gateway-stt-engine/tests/startup_cleanup.rs index 8bf89c919..734053b95 100644 --- a/crates/gateway-stt-engine/tests/startup_cleanup.rs +++ b/crates/gateway-stt-engine/tests/startup_cleanup.rs @@ -41,6 +41,7 @@ impl ModelFactory for ConcurrentStartupFailureFactory { PathBuf::from(FINAL_SENTINEL), std::io::Error::other(FINAL_SENTINEL), )), + _ => unreachable!("the startup cleanup factory scripts only interim and final modes"), } } } diff --git a/crates/gateway-stt/src/audio.rs b/crates/gateway-stt/src/audio.rs index 4cef526a3..ffc97a925 100644 --- a/crates/gateway-stt/src/audio.rs +++ b/crates/gateway-stt/src/audio.rs @@ -44,7 +44,10 @@ impl CommittedAudio { self.input_samples } - #[allow(clippy::cast_precision_loss)] + #[expect( + clippy::cast_precision_loss, + reason = "sample counts at audio-buffer scale convert to f64 without meaningful precision loss" + )] pub(super) fn duration_seconds(&self) -> f64 { self.input_samples as f64 / INPUT_SAMPLE_RATE as f64 } @@ -151,7 +154,10 @@ impl AudioBuffer { } #[cfg(test)] - #[allow(clippy::cast_precision_loss)] + #[expect( + clippy::cast_precision_loss, + reason = "sample counts at audio-buffer scale convert to f64 without meaningful precision loss" + )] pub(super) fn buffered_duration_seconds(&self) -> f64 { self.input_samples as f64 / INPUT_SAMPLE_RATE as f64 } diff --git a/crates/gateway-stt/src/realtime/session/items/tests.rs b/crates/gateway-stt/src/realtime/session/items/tests.rs index 0972e6805..3bfb59c54 100644 --- a/crates/gateway-stt/src/realtime/session/items/tests.rs +++ b/crates/gateway-stt/src/realtime/session/items/tests.rs @@ -19,7 +19,7 @@ fn encoded(samples: &[i16]) -> String { base64::engine::general_purpose::STANDARD.encode(bytes) } -#[allow( +#[expect( clippy::expect_used, reason = "the isolated fixture constructs one session and one scripted generation" )] @@ -47,7 +47,7 @@ fn session_with_audio(service: &crate::SpeechService, payload: &str, budget: usi /// Cancels the session's generation epoch the way production still can: /// closing the runtime's admission, as service shutdown does. -#[allow( +#[expect( clippy::expect_used, reason = "the session owns its generation in these fixtures" )] @@ -65,7 +65,7 @@ fn cancel_generation_epoch(service: &crate::SpeechService, session: &Session) { ); } -#[allow( +#[expect( clippy::expect_used, reason = "the bounded worker observations are deterministic fixture assertions" )] @@ -86,10 +86,6 @@ async fn wait_for_decode_retirement(decoder: ScriptedDecoder, service: &crate::S } #[tokio::test] -#[allow( - clippy::expect_used, - reason = "the scripted interim sequence must reach every asserted ownership boundary" -)] async fn blocked_interim_keeps_exact_budget_until_worker_retirement_and_commit_retries() { let interim = ScriptedDecoder::new(); let service = scripted_service(ScriptedModelFactory::new(interim.clone()), 15, 500) @@ -158,10 +154,6 @@ async fn blocked_interim_keeps_exact_budget_until_worker_retirement_and_commit_r } #[tokio::test] -#[allow( - clippy::expect_used, - reason = "the scripted final sequence must reach every asserted ownership boundary" -)] async fn blocked_final_keeps_budget_after_epoch_cancellation_until_worker_retirement() { let interim = ScriptedDecoder::new(); let final_decoder = ScriptedDecoder::new(); diff --git a/crates/gateway-stt/src/realtime/wire.rs b/crates/gateway-stt/src/realtime/wire.rs index a79810f2e..6a4cc3623 100644 --- a/crates/gateway-stt/src/realtime/wire.rs +++ b/crates/gateway-stt/src/realtime/wire.rs @@ -5,20 +5,12 @@ mod shared; #[cfg(test)] mod tests; -#[allow( - unused_imports, - reason = "private wire surface is consumed by later realtime steps" -)] pub(in crate::realtime) use client::parse_client_event; -#[allow( +#[expect( unused_imports, reason = "private wire surface is consumed by later realtime steps" )] pub(in crate::realtime) use server::{ ConversationItem, DurationUsage, EffectiveSession, ServerEvent, WireError, }; -#[allow( - unused_imports, - reason = "private wire surface is consumed by later realtime steps" -)] pub(in crate::realtime) use shared::{ClientError, ClientEvent, IdGenerator}; diff --git a/crates/gateway-stt/src/test_fixtures/hour.rs b/crates/gateway-stt/src/test_fixtures/hour.rs index b9464a4e5..168e955f8 100644 --- a/crates/gateway-stt/src/test_fixtures/hour.rs +++ b/crates/gateway-stt/src/test_fixtures/hour.rs @@ -230,6 +230,7 @@ impl Decoder for HourSimulationDecoder { .map_err(|_| marker_error("marker end does not fit fixture text"))?; Ok(timeline_text(start_second, end_second)) } + _ => unreachable!("the hour simulation scripts only interim and final decodes"), } } } diff --git a/crates/gateway-stt/tests/it/initial_load.rs b/crates/gateway-stt/tests/it/initial_load.rs index 448c6f641..6e72eec1e 100644 --- a/crates/gateway-stt/tests/it/initial_load.rs +++ b/crates/gateway-stt/tests/it/initial_load.rs @@ -183,7 +183,7 @@ impl ModelFactory for CancelDuringBuild { self.token.cancel(); Ok(match mode { DecodeMode::Interim => Some(Box::new(TrackedDecoder(Arc::clone(&self.dropped)))), - DecodeMode::Final => None, + _ => None, }) } } diff --git a/crates/gateway-stt/tests/it/realtime_forced_windows.rs b/crates/gateway-stt/tests/it/realtime_forced_windows.rs index b370cf5e9..48d435f02 100644 --- a/crates/gateway-stt/tests/it/realtime_forced_windows.rs +++ b/crates/gateway-stt/tests/it/realtime_forced_windows.rs @@ -44,7 +44,7 @@ fn timeline_text(start_second: usize, end_second: usize) -> String { .join(" ") } -#[allow( +#[expect( clippy::expect_used, reason = "the fixture creates one deterministic session and generation" )] @@ -56,7 +56,7 @@ fn scripted_session(final_decoder: &ScriptedDecoder) -> RealtimeSessionFixture { .expect("the scripted Realtime session starts") } -#[allow( +#[expect( clippy::expect_used, reason = "the bounded decoder observation is a deterministic fixture assertion" )] @@ -70,7 +70,7 @@ async fn wait_for_decodes(decoder: &ScriptedDecoder, count: usize) { ); } -#[allow( +#[expect( clippy::expect_used, reason = "the bounded hour probe is observed off the async executor" )] @@ -81,10 +81,6 @@ async fn wait_for_hour_decodes(probe: &HourSimulationProbe, count: usize) -> boo .expect("the hour decode observer joins") } -#[allow( - clippy::expect_used, - reason = "the retry waits only for worker-owned PCM retirement" -)] async fn append_after_retirement(session: &mut RealtimeSessionFixture, payload: &str) { let deadline = Instant::now() + WAIT; loop { @@ -98,7 +94,7 @@ async fn append_after_retirement(session: &mut RealtimeSessionFixture, payload: } } -#[allow( +#[expect( clippy::expect_used, reason = "the bounded fixture must observe one exact settled coverage frontier" )] @@ -134,7 +130,7 @@ struct HourPeaks { } impl HourPeaks { - #[allow( + #[expect( clippy::expect_used, reason = "the bounded hour fixture keeps one input and a fixed stride count" )] @@ -180,7 +176,7 @@ fn append_marked_rotated( } } -#[allow( +#[expect( clippy::expect_used, reason = "the complete decoded hypothesis has one deterministic fixture shape" )] diff --git a/crates/gateway/src/commands.rs b/crates/gateway/src/commands.rs index 5b0c3484e..033912295 100644 --- a/crates/gateway/src/commands.rs +++ b/crates/gateway/src/commands.rs @@ -90,9 +90,12 @@ pub(crate) enum Command { /// into the routing table needs the model's full configuration, which /// this command does not carry; that arrives with the command's first /// producer. - #[allow( - dead_code, - reason = "no producer exists yet; the config UI's model download wires it in a later step" + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "no producer exists yet; the config UI's model download wires it in a later step" + ) )] ProvisionModel { /// The model name, for status display and debounce. @@ -104,9 +107,12 @@ pub(crate) enum Command { }, /// Stop one local model's `llama-server` child and drop it from the /// routing table. Not debounced: unloads are fast and order-independent. - #[allow( - dead_code, - reason = "no producer exists yet; the admin queue routes wire it in a later step" + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "no producer exists yet; the admin queue routes wire it in a later step" + ) )] UnloadModel { /// The model to stop. diff --git a/crates/gateway/src/main.rs b/crates/gateway/src/main.rs index 21f7f155b..727d5888b 100644 --- a/crates/gateway/src/main.rs +++ b/crates/gateway/src/main.rs @@ -129,6 +129,10 @@ fn main() -> ExitCode { } return ExitCode::SUCCESS; } + Ok(_) => { + eprintln!("error: unrecognized gateway startup decision"); + return ExitCode::FAILURE; + } Err(error) => { print_error_chain(&error); return ExitCode::FAILURE; diff --git a/crates/gateway/src/relaunch.rs b/crates/gateway/src/relaunch.rs index e6ebda82e..bd9f7690a 100644 --- a/crates/gateway/src/relaunch.rs +++ b/crates/gateway/src/relaunch.rs @@ -26,6 +26,7 @@ pub(crate) enum Relaunch { /// The process ownership decision made before Gateway startup side effects. #[derive(Debug)] +#[non_exhaustive] pub enum GatewayStartup { /// This process owns the lifetime lease and may boot. Boot(shared_sidecar::GatewayInstanceLease), @@ -48,6 +49,7 @@ pub enum GatewayStartupError { Resolve(#[source] shared_sidecar::SidecarError), /// The lease owner did not publish a validated record in time. #[error("the Gateway process owner published no validated connection within {timeout:?}")] + #[non_exhaustive] OwnerTimeout { /// The bounded wait that elapsed. timeout: Duration, diff --git a/crates/promptforge-core-support/src/untrusted/inventory.rs b/crates/promptforge-core-support/src/untrusted/inventory.rs index f206f38af..25dbde646 100644 --- a/crates/promptforge-core-support/src/untrusted/inventory.rs +++ b/crates/promptforge-core-support/src/untrusted/inventory.rs @@ -36,7 +36,13 @@ pub(super) enum Shape { pub(super) struct DelimiterGroup { /// The model family or protocol whose templates emit these delimiters. // Read by the table sanity tests; live matching keys on shape and names. - #[allow(dead_code)] + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "read by the table sanity tests; live matching keys on shape and names" + ) + )] pub(super) family: &'static str, /// How each entry in `names` spells its opener. pub(super) shape: Shape, diff --git a/crates/promptforge-core/benches/models_loop.rs b/crates/promptforge-core/benches/models_loop.rs index ff03cabce..ea5ed9ab1 100644 --- a/crates/promptforge-core/benches/models_loop.rs +++ b/crates/promptforge-core/benches/models_loop.rs @@ -6,7 +6,10 @@ // The criterion_group! macro expansion generates an undocumented public // entry point; bench targets have no docs contract. -#![allow(missing_docs)] +#![expect( + missing_docs, + reason = "the criterion_group! macro expansion generates an undocumented public entry point; bench targets have no docs contract" +)] #![expect( clippy::expect_used, reason = "bench setup panics on construction failure, which is the desired behavior" diff --git a/crates/promptforge-core/src/error.rs b/crates/promptforge-core/src/error.rs index f4cddd6c4..955ffab67 100644 --- a/crates/promptforge-core/src/error.rs +++ b/crates/promptforge-core/src/error.rs @@ -245,7 +245,6 @@ pub(crate) enum Error { /// until the `models.loop` step rewires it. #[error("model-facing schema build failure for tool alias {alias:?}")] #[non_exhaustive] - #[allow(dead_code)] BindSchema { /// The prompt-local alias whose schema could not be built. alias: String, @@ -352,7 +351,6 @@ pub(crate) enum Error { similarity = diagnostic.similarity, )] #[non_exhaustive] - #[allow(dead_code)] // constructed by the scope validation, test-only until `models.loop` NearDuplicateTools { /// The complete pair diagnostic, boxed to keep every crate error small. /// The diagnostic vocabulary lives in tool-scope validation (F10). diff --git a/crates/promptforge-core/src/execute/context.rs b/crates/promptforge-core/src/execute/context.rs index ab3478b1b..4e955f824 100644 --- a/crates/promptforge-core/src/execute/context.rs +++ b/crates/promptforge-core/src/execute/context.rs @@ -186,7 +186,10 @@ impl RunContext { /// The run's tool set, read-only. /// /// Unused until the `models.loop` step reads the call-time tool scope. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "unused until the models.loop step reads the call-time tool scope" + )] pub(crate) fn tools(&self) -> &dyn ToolView { &*self.tools } diff --git a/crates/promptforge-core/src/execute/gateway.rs b/crates/promptforge-core/src/execute/gateway.rs index e21837cac..1cbb1cc3a 100644 --- a/crates/promptforge-core/src/execute/gateway.rs +++ b/crates/promptforge-core/src/execute/gateway.rs @@ -86,8 +86,6 @@ impl GatewaySource { /// The caller-supplied client when the source wraps one, so a driver can /// seed a chain's client slot without forcing the environment build. - // Consumed by the scheduler driver until the flip. - #[allow(dead_code)] pub(crate) fn ready(&self) -> Option<&GatewayClient> { match self { GatewaySource::Ready(client) => Some(client), diff --git a/crates/promptforge-lua/benches/surface.rs b/crates/promptforge-lua/benches/surface.rs index d21505de7..a41066e0e 100644 --- a/crates/promptforge-lua/benches/surface.rs +++ b/crates/promptforge-lua/benches/surface.rs @@ -5,7 +5,10 @@ // The criterion_group! macro expansion generates an undocumented public // entry point; bench targets have no docs contract. -#![allow(missing_docs)] +#![expect( + missing_docs, + reason = "the criterion_group! macro expansion generates an undocumented public entry point; bench targets have no docs contract" +)] #![expect( clippy::expect_used, reason = "bench setup panics on construction failure, which is the desired behavior" diff --git a/crates/promptforge-lua/src/protocol.rs b/crates/promptforge-lua/src/protocol.rs index a35c2838d..ce4e21e5b 100644 --- a/crates/promptforge-lua/src/protocol.rs +++ b/crates/promptforge-lua/src/protocol.rs @@ -215,10 +215,6 @@ pub enum Request { /// broker and its host policy own the whole interaction. UserInput, /// Reserved. Never dispatched: receiving one is a typed protocol error. - // The fields are read only by this module's own tests; production parses - // them for strict validation and never reads them until the variant - // gains a dispatch. - #[allow(dead_code)] Mcp { /// The reserved server name. server: String, diff --git a/crates/promptforge-lua/src/vm.rs b/crates/promptforge-lua/src/vm.rs index 2bf5ee12c..d08964391 100644 --- a/crates/promptforge-lua/src/vm.rs +++ b/crates/promptforge-lua/src/vm.rs @@ -683,7 +683,10 @@ impl SectionVm { /// Shared live `sys` JSON for finish-reason updates. #[must_use] - #[allow(dead_code)] // exercised by the lua module's poisoned-slot test + #[cfg_attr( + not(test), + expect(dead_code, reason = "exercised by the lua module's poisoned-slot test") + )] pub(crate) fn sys_live_handle(&self) -> Arc>> { Arc::clone(&self.sys_live) } @@ -907,7 +910,7 @@ impl SectionVm { /// /// # Errors /// Returns [`Error::Lua`] if the local-tools registry was poisoned. - #[allow(dead_code)] // wired up by the local-tools dispatch step + #[expect(dead_code, reason = "wired up by the local-tools dispatch step")] pub(crate) fn has_local_tool(&self, alias: &str) -> Result { self.local_tools.contains(alias) } diff --git a/crates/shared-sidecar/src/validated.rs b/crates/shared-sidecar/src/validated.rs index 6307db6cf..c627fd7ba 100644 --- a/crates/shared-sidecar/src/validated.rs +++ b/crates/shared-sidecar/src/validated.rs @@ -91,6 +91,7 @@ pub enum ValidationError { /// let _ = ValidatedConnection::validate_named(raw, "my-test-binary"); /// ``` #[derive(Clone, PartialEq, Eq)] +#[non_exhaustive] pub struct ValidatedConnection { connection: GatewayDiscoveryFile, process_identity: ProcessIdentity, diff --git a/crates/workshop-server/src/gateway_binding.rs b/crates/workshop-server/src/gateway_binding.rs index 79c44b6a8..33d00d93f 100644 --- a/crates/workshop-server/src/gateway_binding.rs +++ b/crates/workshop-server/src/gateway_binding.rs @@ -97,6 +97,7 @@ struct PublicationState { pub enum GatewayPublicationError { /// The replacement clients could not be built. #[error(transparent)] + #[non_exhaustive] Build(#[from] GatewayError), /// The binding has permanently revoked replacement publication. #[error("gateway replacement publication is permanently closed")] diff --git a/crates/workshop/src/bridge.rs b/crates/workshop/src/bridge.rs index 31201b83f..0d5469bbc 100644 --- a/crates/workshop/src/bridge.rs +++ b/crates/workshop/src/bridge.rs @@ -198,13 +198,13 @@ fn dropped_paths(args: &ICoreWebView2WebMessageReceivedEventArgs) -> Vec windows_core::Result<()> { - #[allow(clippy::cast_possible_truncation)] + #[expect( + clippy::cast_possible_truncation, + reason = "the test fake holds a handful of objects, far below u32::MAX" + )] // SAFETY: `value` is the caller's out-pointer, valid for one write. unsafe { *value = self.objects.len() as u32; diff --git a/vibe-ledger.md b/vibe-ledger.md index 1afe4b127..7512f0406 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -60,3 +60,5 @@ - Rulebook debt tiers Step 1: CI doctest coverage and dependency hygiene - component-scope verify pass: build, `cargo fmt --all --check`, warnings-denied clippy, and `cargo test --locked -p gateway-stt` (121 unit + 52 integration) green; nextest unavailable locally, cargo test fallback recorded in verify-step-1-round-1.log. Decision: `ci-green` treats cancelled as failure via an `if: always()` loop over `needs.*.result` | Falsifier: a cancelled or newly added job shows green as a required status check. Decision: doctest step placed in `check-workshop` (Windows) rather than the Linux `test` job | Falsifier: workshop doctests run twice or not at all in CI. - Rulebook debt tiers Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel - component-scope verify pass: gateway and workshop builds, `cargo fmt --all --check`, both clippy invocations, nextest across the four touched crates, and the gateway-stt integration suite green (verify-step-2-round-1.log). Decision: `OperatorCancellation` is the one loss-tolerant event and alone rides `mpsc::channel(1)`; `AcceptedInput`/`TerminalSettlement`/`Close` keep guaranteed delivery on the unbounded queue | Falsifier: a reducer transition that awaits an operator cancellation. Decision: `RecoveryCandidate::shutdown` disarms the drop signal even on failure since the caller owns the outcome | Falsifier: a failure class where a silent drop retry recovers the child. Decision: `SessionError` dropped `Eq/PartialEq` because `TranscribeError` is not `PartialEq`; test comparisons became `matches!` | Falsifier: a production `SessionError` equality comparison. Decision: reject cargo-nextest install in the fix round, accept the passing cargo test fallback | Falsifier: a later round requires nextest-specific behavior. + +- Rulebook debt tiers Step 3: non_exhaustive attributes and expect conversions - component-scope verify pass: builds, `cargo fmt --all --check`, both clippy gates, and nextest across the touched crates green, 1925 tests, zero failures (verify-step-3-round-1.log). Decision: wildcard arms on downstream `DecodeMode` matches - `unreachable!` in test factories, `_ => None` fallbacks in initial_load.rs and backend-whisper model.rs | Falsifier: a third variant is added and the chosen fallback proves wrong. Decision: gateway bin `main.rs` wildcard exits `FAILURE` on an unrecognized future `GatewayStartup` variant | Falsifier: a new variant needs serving behavior there. Decision: 8 suppressions were stale (lint never fires) and deleted rather than converted | Falsifier: the `-D warnings` gate, which re-verified each. Decision: cfg-dependent sites use `#[cfg_attr(not(test), expect(...))]` | Falsifier: the gate. Decision: `app.rs` keeps its `#[allow]` per its in-code comment (expectation unfulfilled in some cfg permutations) | Falsifier: clippy behavior changes. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md index d8c36fe73..5f45fdc1a 100644 --- a/vibe/2026-09-11-1-rulebook-debt-tiers.md +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -219,7 +219,7 @@ Tests in the same commit prove: drop no longer blocks and explicit shutdown stil -### Step 3: `#[non_exhaustive]` attributes and `#[expect]` conversions +### Step 3: `#[non_exhaustive]` attributes and `#[expect]` conversions [completed] - Component: api-attributes From 267713e24eddf758168892b8f1be282eb744c6b6 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 10:11:49 -0700 Subject: [PATCH 4/7] Convert in-process async tests to paused time Four in-process async tests now run on a paused clock, so their timeouts and cancellation races resolve deterministically instead of depending on wall-clock timing. The three affected crates enable the async runtime's test-utilities feature for their test builds, which makes the paused clock available. One rendezvous test no longer races a timer-based late arrival against a real-time wait; the late arrival now starts only after the rendezvous has timed out, staying deterministic while the paused clock auto-advances on idle. One cancellation test moves from a multi-thread runtime to a current-thread runtime, which pausing requires. No production code changes. - `crates/gateway-stt-engine/src/test_fixtures/tests/scenario_cleanup/decode.rs` spawns the late decode only after the rendezvous times out instead of racing it on a 50-millisecond timer; the oneshot channel that published the decode handle is removed. - `crates/promptforge-core/src/execute/tests/input.rs` runs `cancellation_interrupts_a_pending_input_wait` on `current_thread` instead of `multi_thread` with two worker threads, which `start_paused` requires. - `crates/gateway-stt-engine/Cargo.toml`, `crates/promptforge-core/Cargo.toml`, and `crates/promptforge-lua/Cargo.toml` add the `test-util` feature to the tokio dev-dependency. - `crates/promptforge-core/src/execute/tests/scheduler.rs` and `crates/promptforge-lua/src/dispatch.rs` gain `start_paused = true` with no body changes; their in-test sleeps now resolve on the paused clock. - `vibe-ledger.md` records the component-scope verify pass and the conversion decisions with their falsifiers. Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- crates/gateway-stt-engine/Cargo.toml | 1 + .../tests/scenario_cleanup/decode.rs | 41 ++++++++----------- crates/promptforge-core/Cargo.toml | 2 +- .../src/execute/tests/input.rs | 2 +- .../src/execute/tests/scheduler.rs | 2 +- crates/promptforge-lua/Cargo.toml | 2 +- crates/promptforge-lua/src/dispatch.rs | 2 +- vibe-ledger.md | 2 + vibe/2026-09-11-1-rulebook-debt-tiers.md | 2 +- 9 files changed, 26 insertions(+), 30 deletions(-) diff --git a/crates/gateway-stt-engine/Cargo.toml b/crates/gateway-stt-engine/Cargo.toml index 979ab2273..cefee55a5 100644 --- a/crates/gateway-stt-engine/Cargo.toml +++ b/crates/gateway-stt-engine/Cargo.toml @@ -15,6 +15,7 @@ tokio.workspace = true [dev-dependencies] tempfile.workspace = true +tokio = { workspace = true, features = ["test-util"] } [features] test-fixtures = [] diff --git a/crates/gateway-stt-engine/src/test_fixtures/tests/scenario_cleanup/decode.rs b/crates/gateway-stt-engine/src/test_fixtures/tests/scenario_cleanup/decode.rs index 265b91f9d..5fe224321 100644 --- a/crates/gateway-stt-engine/src/test_fixtures/tests/scenario_cleanup/decode.rs +++ b/crates/gateway-stt-engine/src/test_fixtures/tests/scenario_cleanup/decode.rs @@ -71,7 +71,7 @@ async fn canceled_decode_scenario_releases_and_permits_a_follow_up() { engine.shutdown().expect("worker joins"); } -#[tokio::test] +#[tokio::test(start_paused = true)] async fn decode_rendezvous_timeout_releases_a_late_arrival_and_permits_a_follow_up() { let decoder = ScriptedDecoder::new(); decoder.push_text("late arrival"); @@ -79,33 +79,26 @@ async fn decode_rendezvous_timeout_releases_a_late_arrival_and_permits_a_follow_ SttEngine::new(ScriptedModelFactory::new(decoder.clone()), policy()) .expect("scripted worker starts"), ); - let delayed_engine = Arc::clone(&engine); - let (decode_tx, decode_rx) = tokio::sync::oneshot::channel(); + // The rendezvous timeout is a real-time condvar wait while the paused + // clock auto-advances when the runtime idles, so a timer-based late + // arrival would race the rendezvous. Arriving only after the timeout + // keeps the lateness deterministic. let result = decoder - .with_next_decode_blocked( - Duration::from_millis(10), - || async move { - let decode = tokio::spawn(async move { - tokio::time::sleep(Duration::from_millis(50)).await; - start_decode(delayed_engine) - .await - .expect("nested decode task joins") - }); - drop(decode_tx.send(decode)); - }, - |()| async {}, - ) + .with_next_decode_blocked(Duration::from_millis(10), || async {}, |()| async {}) .await; assert!(result.is_none(), "the rendezvous must time out"); + let delayed_engine = Arc::clone(&engine); + let decode = tokio::spawn(async move { + start_decode(delayed_engine) + .await + .expect("nested decode task joins") + }); assert_eq!( - tokio::time::timeout( - WAIT, - decode_rx.await.expect("late decode handle is published") - ) - .await - .expect("late decode is not stranded") - .expect("late decode task joins") - .expect("late decode succeeds"), + tokio::time::timeout(WAIT, decode) + .await + .expect("late decode is not stranded") + .expect("late decode task joins") + .expect("late decode succeeds"), "late arrival" ); run_blocked_decode(&decoder, &engine, "follow-up after timeout").await; diff --git a/crates/promptforge-core/Cargo.toml b/crates/promptforge-core/Cargo.toml index 59faea03a..bd4926076 100644 --- a/crates/promptforge-core/Cargo.toml +++ b/crates/promptforge-core/Cargo.toml @@ -36,7 +36,7 @@ axum.workspace = true criterion.workspace = true promptforge-parser = { workspace = true, features = ["test-support"] } promptforge-tool-picker = { workspace = true, features = ["test-fixtures"] } -tokio.workspace = true +tokio = { workspace = true, features = ["test-util"] } [[bench]] name = "models_loop" diff --git a/crates/promptforge-core/src/execute/tests/input.rs b/crates/promptforge-core/src/execute/tests/input.rs index d782d5636..842b07649 100644 --- a/crates/promptforge-core/src/execute/tests/input.rs +++ b/crates/promptforge-core/src/execute/tests/input.rs @@ -300,7 +300,7 @@ async fn an_uncaught_broker_failure_fails_the_run_typed() { ); } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test(flavor = "current_thread", start_paused = true)] async fn cancellation_interrupts_a_pending_input_wait() { use crate::cancel::{self, CancelHandle}; use std::time::{Duration, Instant}; diff --git a/crates/promptforge-core/src/execute/tests/scheduler.rs b/crates/promptforge-core/src/execute/tests/scheduler.rs index 6a8ebb4d9..a40555589 100644 --- a/crates/promptforge-core/src/execute/tests/scheduler.rs +++ b/crates/promptforge-core/src/execute/tests/scheduler.rs @@ -3091,7 +3091,7 @@ impl Tool for SignallingSlowTool { } } -#[tokio::test(flavor = "current_thread")] +#[tokio::test(flavor = "current_thread", start_paused = true)] async fn cancellation_interrupts_a_slow_script_tools_call() { use crate::cancel::{self, CancelHandle}; diff --git a/crates/promptforge-lua/Cargo.toml b/crates/promptforge-lua/Cargo.toml index af2012e9b..d825bac18 100644 --- a/crates/promptforge-lua/Cargo.toml +++ b/crates/promptforge-lua/Cargo.toml @@ -26,7 +26,7 @@ tokio.workspace = true [dev-dependencies] async-trait.workspace = true criterion.workspace = true -tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "test-util"] } [[bench]] name = "surface" diff --git a/crates/promptforge-lua/src/dispatch.rs b/crates/promptforge-lua/src/dispatch.rs index aea27479f..f8c309e23 100644 --- a/crates/promptforge-lua/src/dispatch.rs +++ b/crates/promptforge-lua/src/dispatch.rs @@ -389,7 +389,7 @@ mod tests { ); } - #[tokio::test] + #[tokio::test(start_paused = true)] async fn cancellation_interrupts_the_dispatch() { let recorder = Recorder::default(); let slow = binding("slow", Arc::new(SlowTool)); diff --git a/vibe-ledger.md b/vibe-ledger.md index 7512f0406..3592e2cc2 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -62,3 +62,5 @@ - Rulebook debt tiers Step 2: Non-blocking destructors, restored error causes, bounded supervisor channel - component-scope verify pass: gateway and workshop builds, `cargo fmt --all --check`, both clippy invocations, nextest across the four touched crates, and the gateway-stt integration suite green (verify-step-2-round-1.log). Decision: `OperatorCancellation` is the one loss-tolerant event and alone rides `mpsc::channel(1)`; `AcceptedInput`/`TerminalSettlement`/`Close` keep guaranteed delivery on the unbounded queue | Falsifier: a reducer transition that awaits an operator cancellation. Decision: `RecoveryCandidate::shutdown` disarms the drop signal even on failure since the caller owns the outcome | Falsifier: a failure class where a silent drop retry recovers the child. Decision: `SessionError` dropped `Eq/PartialEq` because `TranscribeError` is not `PartialEq`; test comparisons became `matches!` | Falsifier: a production `SessionError` equality comparison. Decision: reject cargo-nextest install in the fix round, accept the passing cargo test fallback | Falsifier: a later round requires nextest-specific behavior. - Rulebook debt tiers Step 3: non_exhaustive attributes and expect conversions - component-scope verify pass: builds, `cargo fmt --all --check`, both clippy gates, and nextest across the touched crates green, 1925 tests, zero failures (verify-step-3-round-1.log). Decision: wildcard arms on downstream `DecodeMode` matches - `unreachable!` in test factories, `_ => None` fallbacks in initial_load.rs and backend-whisper model.rs | Falsifier: a third variant is added and the chosen fallback proves wrong. Decision: gateway bin `main.rs` wildcard exits `FAILURE` on an unrecognized future `GatewayStartup` variant | Falsifier: a new variant needs serving behavior there. Decision: 8 suppressions were stale (lint never fires) and deleted rather than converted | Falsifier: the `-D warnings` gate, which re-verified each. Decision: cfg-dependent sites use `#[cfg_attr(not(test), expect(...))]` | Falsifier: the gate. Decision: `app.rs` keeps its `#[allow]` per its in-code comment (expectation unfulfilled in some cfg permutations) | Falsifier: clippy behavior changes. + +- Rulebook debt tiers Step 4: Paused-time conversion for in-process async tests - component-scope verify pass: build, `cargo fmt --all --check`, clippy `-D warnings`, and nextest across promptforge-core, promptforge-lua, and gateway-stt-engine green (verify-step-4-round-1.log); converted tests proven deterministic over 60 repeated runs. Decision: `cancellation_interrupts_a_pending_input_wait` switched from `multi_thread` to `current_thread`, which tokio requires for `start_paused` | Falsifier: the cancel-during-pending-wait assertion is flavor-independent and passes 20/20. Decision: relied on tokio's idle auto-advance rather than explicit `advance()` calls, since the sleeps live in spawned canceller/tool tasks | Falsifier: 60/60 green repetitions. Decision: the gateway-stt-engine rendezvous test's late arrival was restructured to start after the rendezvous times out rather than relying on a timer race, because a paused clock auto-advances into the rendezvous window | Falsifier: tokio's paused runtime shown not to auto-advance while a `spawn_blocking` condvar wait is outstanding. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md index 5f45fdc1a..27c08ea26 100644 --- a/vibe/2026-09-11-1-rulebook-debt-tiers.md +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -231,7 +231,7 @@ This is a mechanical change: the existing suite unmodified plus the component-sc -### Step 4: Paused-time conversion for in-process async tests +### Step 4: Paused-time conversion for in-process async tests [completed] - Component: paused-time-tests From a29d8fae0b0d3c525a713fbed594762849b16f90 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 10:59:31 -0700 Subject: [PATCH 5/7] Replace stringly take and fixture errors with typed sources Failure channels in the take pipeline, the realtime session, and the fixture API carried bare strings, so downstream classification compared message text and wrapped causes were discarded. Typed error enums now represent take failures, item finalization failures, and fixture operations, with sources retained and shared by reference where a failure crosses from the take into session gating. Every affected signature, assertion, and test moves to the typed form while the user-visible message text stays unchanged. - `TakeFailure` is a new thiserror enum of thirteen variants, retained as `Arc` because pending-failure reads clone it out of the take mutex for session precommit gating. - `FixtureError` is a new public non-exhaustive thiserror enum with twenty-two operation-named variants; crate-internal sources are boxed, while `SpeechError` and `serde_json::Error` are carried concretely. - `FinalizationError` types the item finalization join failures, and `SessionError::Finalization` now wraps it transparently. - `ClientError` gains Display and Error trait impls that write its message field. - `from_precommit` classifies on the typed variant instead of comparing message text. - `fail_precommit` and `replace_finalization` wrap caller-supplied strings in the test-gated `TakeFailure::Recorded` variant, preserving the string-injection hook. - `source_message` helpers in both integration suites assert the operation message on the fixture error and the original message on its source. - `PendingPrecommitFailure` carries `Arc` formatted inline, with no source chain. - `ItemFailure` keeps string payloads on the wire terminal; only its classification became typed. Design: removes stringly-typed @ crates/gateway-stt/src/test_fixtures.rs boundary: pub Design: removes stringly-typed @ crates/gateway-stt/src/take/state.rs::TakeState::record_failure Design: removes stringly-typed @ crates/gateway-stt/src/realtime/route.rs::RoutePolicy::precommit_failure Design: removes stringly-typed @ crates/gateway-stt/src/realtime/result_mailbox.rs::ItemFailure::from_precommit deps: TakeFailure Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- crates/gateway-stt/src/realtime/input.rs | 8 +- crates/gateway-stt/src/realtime/item.rs | 37 +-- .../src/realtime/result_mailbox.rs | 10 +- crates/gateway-stt/src/realtime/route.rs | 11 +- crates/gateway-stt/src/realtime/session.rs | 7 +- .../gateway-stt/src/realtime/session/items.rs | 10 +- .../src/realtime/session/items/tests.rs | 26 ++ .../gateway-stt/src/realtime/session/state.rs | 10 +- .../gateway-stt/src/realtime/wire/shared.rs | 8 + crates/gateway-stt/src/take.rs | 27 ++- crates/gateway-stt/src/take/final_decode.rs | 12 +- crates/gateway-stt/src/take/finalization.rs | 56 +++-- crates/gateway-stt/src/take/state.rs | 68 ++++-- .../take/state/alignment_tests/adversaries.rs | 15 +- crates/gateway-stt/src/take/state/tests.rs | 19 +- crates/gateway-stt/src/test_fixtures.rs | 226 ++++++++++++++---- .../tests/it/realtime_forced_windows.rs | 24 +- .../gateway-stt/tests/it/realtime_session.rs | 126 +++++++--- vibe-ledger.md | 2 + vibe/2026-09-11-1-rulebook-debt-tiers.md | 2 +- 20 files changed, 515 insertions(+), 189 deletions(-) diff --git a/crates/gateway-stt/src/realtime/input.rs b/crates/gateway-stt/src/realtime/input.rs index 1e02cdfdc..0f8f71450 100644 --- a/crates/gateway-stt/src/realtime/input.rs +++ b/crates/gateway-stt/src/realtime/input.rs @@ -1,6 +1,8 @@ +use std::sync::Arc; + use crate::audio::{AudioBuffer, AudioError}; use crate::generation::GenerationLease; -use crate::take::Take; +use crate::take::{Take, TakeFailure}; const INPUT_FORMAT: &str = "audio/pcm"; const INPUT_RATE: u32 = 24_000; const INPUT_MODEL: &str = "realtime-transcribe"; @@ -168,11 +170,11 @@ impl UncommittedInput { self.audio.buffered_duration_seconds() } - pub(crate) fn pending_failure(&self) -> Option { + pub(crate) fn pending_failure(&self) -> Option> { self.take.pending_failure() } - pub(crate) fn record_pending_failure(&mut self, failure: String) { + pub(crate) fn record_pending_failure(&mut self, failure: TakeFailure) { self.take.record_failure(failure); } diff --git a/crates/gateway-stt/src/realtime/item.rs b/crates/gateway-stt/src/realtime/item.rs index a5cf912c8..0292e5a3f 100644 --- a/crates/gateway-stt/src/realtime/item.rs +++ b/crates/gateway-stt/src/realtime/item.rs @@ -6,9 +6,20 @@ use tokio::task::JoinHandle; use super::input::InputSnapshot; use super::input::SealedInput; use super::result_mailbox::{ItemFailure, ItemResult}; -use crate::take::Take; - -type FinalizationTask = JoinHandle>; +use crate::take::{Take, TakeFailure}; + +type FinalizationTask = JoinHandle>>; + +/// A committed item's finalization join failure. +#[derive(Debug, thiserror::Error)] +pub(crate) enum FinalizationError { + #[error("the committed item has no active finalization")] + NotFinalizing, + #[error("committed item finalization task failed")] + Task(#[source] tokio::task::JoinError), + #[error("the committed item already reached a terminal outcome")] + TerminalAlreadySet, +} #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct CommitReceipt { @@ -41,7 +52,7 @@ pub(crate) struct CommittedItem { snapshot: InputSnapshot, #[cfg_attr( not(any(test, feature = "test-fixtures")), - allow(dead_code, reason = "retains take ownership until item retirement") + expect(dead_code, reason = "retains take ownership until item retirement") )] take: Arc, duration_seconds: f64, @@ -53,7 +64,7 @@ impl CommittedItem { pub(crate) fn from_sealed( sealed: SealedInput, previous_item_id: Option, - ) -> (Self, Option) { + ) -> (Self, Option>) { let pending_failure = sealed.take.pending_failure(); let take = Arc::new(sealed.take); let finalization = if pending_failure.is_none() { @@ -105,21 +116,19 @@ impl CommittedItem { .is_some_and(tokio::task::JoinHandle::is_finished) } - pub(crate) async fn finish_finalization(&mut self) -> Result { + pub(crate) async fn finish_finalization(&mut self) -> Result { let Some(task) = self.finalization.as_mut() else { - return Err("the committed item has no active finalization".to_owned()); + return Err(FinalizationError::NotFinalizing); }; - let outcome = task - .await - .map_err(|error| format!("committed item finalization task failed: {error}"))?; + let outcome = task.await.map_err(FinalizationError::Task)?; self.finalization = None; match outcome { Ok(transcript) => self .completed(transcript) - .ok_or_else(|| "the committed item already reached a terminal outcome".to_owned()), - Err(message) => self - .failed(ItemFailure::TranscriptionFailed(message)) - .ok_or_else(|| "the committed item already reached a terminal outcome".to_owned()), + .ok_or(FinalizationError::TerminalAlreadySet), + Err(failure) => self + .failed(ItemFailure::TranscriptionFailed(failure.to_string())) + .ok_or(FinalizationError::TerminalAlreadySet), } } diff --git a/crates/gateway-stt/src/realtime/result_mailbox.rs b/crates/gateway-stt/src/realtime/result_mailbox.rs index ce3582166..9a49cb36d 100644 --- a/crates/gateway-stt/src/realtime/result_mailbox.rs +++ b/crates/gateway-stt/src/realtime/result_mailbox.rs @@ -1,5 +1,7 @@ use std::collections::{HashMap, VecDeque}; +use crate::take::TakeFailure; + pub(crate) const SESSION_RESULT_CAPACITY: usize = 16; #[derive(Clone, Debug, Eq, PartialEq)] @@ -10,11 +12,11 @@ pub(crate) enum ItemFailure { } impl ItemFailure { - pub(crate) fn from_precommit(message: &str) -> Self { - if message == "final segment capacity is reached" { - Self::FinalSegmentOverload(message.to_owned()) + pub(crate) fn from_precommit(failure: &TakeFailure) -> Self { + if matches!(failure, TakeFailure::SegmentCapacity) { + Self::FinalSegmentOverload(failure.to_string()) } else { - Self::PrecommitTranscriptionFailed(message.to_owned()) + Self::PrecommitTranscriptionFailed(failure.to_string()) } } diff --git a/crates/gateway-stt/src/realtime/route.rs b/crates/gateway-stt/src/realtime/route.rs index 672fae12a..d834f9d2b 100644 --- a/crates/gateway-stt/src/realtime/route.rs +++ b/crates/gateway-stt/src/realtime/route.rs @@ -21,6 +21,7 @@ use super::session::SessionError; use super::wire::{ClientError, ClientEvent, ServerEvent, parse_client_event}; use crate::audio::AudioError; use crate::generation::{GenerationLease, GenerationState}; +use crate::take::TakeFailure; const SEND_DEADLINE: Duration = Duration::from_millis(500); @@ -70,14 +71,12 @@ impl RoutePolicy { self.forced_precommit_failure = Some(failure); } - fn precommit_failure(&self) -> Option<&'static str> { + fn precommit_failure(&self) -> Option { #[cfg(feature = "test-fixtures")] if let Some(failure) = self.forced_precommit_failure { return Some(match failure { - ForcedPrecommitFailure::FinalSegmentOverload => "final segment capacity is reached", - ForcedPrecommitFailure::Transcription => { - "final transcription worker is unavailable" - } + ForcedPrecommitFailure::FinalSegmentOverload => TakeFailure::SegmentCapacity, + ForcedPrecommitFailure::Transcription => TakeFailure::WorkerUnavailable, }); } None @@ -270,7 +269,7 @@ fn append_events( session.ensure_interim_capacity()?; session.append_base64(audio)?; if let Some(failure) = policy.precommit_failure() { - session.record_pending_failure(failure.to_owned())?; + session.record_pending_failure(failure)?; } Ok(Vec::new()) } diff --git a/crates/gateway-stt/src/realtime/session.rs b/crates/gateway-stt/src/realtime/session.rs index c4abb94a2..1f0a8b9b6 100644 --- a/crates/gateway-stt/src/realtime/session.rs +++ b/crates/gateway-stt/src/realtime/session.rs @@ -3,6 +3,7 @@ use super::item::CommittedItem; use super::registry::SessionRegistration; use super::wire::{ClientError, EffectiveSession, IdGenerator, ServerEvent}; use crate::generation::GenerationLease; +use crate::take::TakeFailure; #[cfg(any(test, feature = "test-fixtures"))] use std::future::Future; mod items; @@ -150,7 +151,10 @@ impl Session { self.canceled_tasks.len() } - pub(crate) fn record_pending_failure(&mut self, failure: String) -> Result<(), SessionError> { + pub(crate) fn record_pending_failure( + &mut self, + failure: TakeFailure, + ) -> Result<(), SessionError> { let input = self.input.as_mut().ok_or(SessionError::NoInput)?; input.record_pending_failure(failure); Ok(()) @@ -161,6 +165,7 @@ impl Session { self.input .as_ref() .and_then(UncommittedInput::pending_failure) + .map(|failure| failure.to_string()) } #[cfg(feature = "test-fixtures")] diff --git a/crates/gateway-stt/src/realtime/session/items.rs b/crates/gateway-stt/src/realtime/session/items.rs index 8e815d9c2..55d7ae314 100644 --- a/crates/gateway-stt/src/realtime/session/items.rs +++ b/crates/gateway-stt/src/realtime/session/items.rs @@ -1,11 +1,15 @@ #[cfg(feature = "test-fixtures")] use std::future::Future; +#[cfg(feature = "test-fixtures")] +use std::sync::Arc; use super::state::{ MAX_COMMITTED_ITEMS_PER_SESSION, SESSION_CANCEL_JOIN_CAPACITY, Session, SessionError, }; use crate::realtime::item::{CommitReceipt, CommittedItem}; use crate::realtime::result_mailbox::{ItemFailure, ItemResult, MailboxError}; +#[cfg(feature = "test-fixtures")] +use crate::take::TakeFailure; impl Session { pub(crate) fn commit(&mut self) -> Result { @@ -72,7 +76,7 @@ impl Session { task: F, ) -> Result<(), SessionError> where - F: Future> + Send + 'static, + F: Future>> + Send + 'static, { let item = self .committed @@ -151,9 +155,7 @@ impl Session { .committed .get_mut(item_id) .ok_or(MailboxError::UnknownItem)?; - item.finish_finalization() - .await - .map_err(SessionError::Finalization)? + item.finish_finalization().await? }; self.results.set_terminal(item_id, terminal)?; Ok(()) diff --git a/crates/gateway-stt/src/realtime/session/items/tests.rs b/crates/gateway-stt/src/realtime/session/items/tests.rs index 3bfb59c54..f072d679e 100644 --- a/crates/gateway-stt/src/realtime/session/items/tests.rs +++ b/crates/gateway-stt/src/realtime/session/items/tests.rs @@ -5,6 +5,7 @@ use gateway_stt_engine::test_fixtures::{ScriptedDecoder, ScriptedModelFactory}; use crate::audio::AudioError; use crate::realtime::input::{InputSnapshot, UncommittedInput}; +use crate::realtime::item::FinalizationError; use crate::realtime::registry::SessionRegistry; use crate::realtime::session::{Session, SessionError}; use crate::test_fixtures::scripted_service; @@ -85,6 +86,31 @@ async fn wait_for_decode_retirement(decoder: ScriptedDecoder, service: &crate::S .expect("the detached generation job retires"); } +#[tokio::test] +async fn finalization_without_active_work_surfaces_a_typed_session_source() { + let registration = SessionRegistry::default() + .register() + .expect("session registers"); + let mut session = Session::new(registration, None); + session + .append_base64(&encoded(&vec![0; 2_400])) + .expect("input appends"); + let item_id = session.commit().expect("item commits").item_id().to_owned(); + + let error = session + .finish_finalization(&item_id) + .await + .expect_err("an item without finalization work is rejected"); + let SessionError::Finalization(source) = &error else { + panic!("the item failure surfaces as a finalization error"); + }; + assert!(matches!(source, FinalizationError::NotFinalizing)); + assert_eq!( + error.to_string(), + "the committed item has no active finalization" + ); +} + #[tokio::test] async fn blocked_interim_keeps_exact_budget_until_worker_retirement_and_commit_retries() { let interim = ScriptedDecoder::new(); diff --git a/crates/gateway-stt/src/realtime/session/state.rs b/crates/gateway-stt/src/realtime/session/state.rs index 49606c6fb..7eadc7215 100644 --- a/crates/gateway-stt/src/realtime/session/state.rs +++ b/crates/gateway-stt/src/realtime/session/state.rs @@ -1,12 +1,14 @@ use crate::audio::AudioError; use crate::generation::GenerationLease; use crate::realtime::input::UncommittedInput; -use crate::realtime::item::CommittedItem; +use crate::realtime::item::{CommittedItem, FinalizationError}; use crate::realtime::registry::SessionRegistration; use crate::realtime::result_mailbox::{MailboxError, ResultMailbox}; use crate::realtime::wire::{EffectiveSession, IdGenerator}; +use crate::take::TakeFailure; use gateway_stt_engine::TranscribeError; use std::collections::HashMap; +use std::sync::Arc; use tokio::task::JoinHandle; pub(super) const SESSION_CANCEL_JOIN_CAPACITY: usize = 8; pub(super) const MAX_COMMITTED_ITEMS_PER_SESSION: usize = 4; @@ -48,9 +50,9 @@ pub(crate) enum SessionError { #[error("the realtime session result capacity is reached")] InterimAtCapacity, #[error("{0}")] - PendingPrecommitFailure(String), - #[error("{0}")] - Finalization(String), + PendingPrecommitFailure(Arc), + #[error(transparent)] + Finalization(#[from] FinalizationError), #[error(transparent)] Mailbox(MailboxError), } diff --git a/crates/gateway-stt/src/realtime/wire/shared.rs b/crates/gateway-stt/src/realtime/wire/shared.rs index 0c2adbaf6..f5e6471be 100644 --- a/crates/gateway-stt/src/realtime/wire/shared.rs +++ b/crates/gateway-stt/src/realtime/wire/shared.rs @@ -123,6 +123,14 @@ impl ClientError { } } +impl std::fmt::Display for ClientError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str(&self.message) + } +} + +impl std::error::Error for ClientError {} + #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) enum RequiredNullable { Null, diff --git a/crates/gateway-stt/src/take.rs b/crates/gateway-stt/src/take.rs index f11f57b41..43aae2c6f 100644 --- a/crates/gateway-stt/src/take.rs +++ b/crates/gateway-stt/src/take.rs @@ -27,6 +27,7 @@ pub(crate) use interim::InterimSnapshot; #[cfg(test)] use pcm::PcmBudgetProbe; use pcm::RetainedPcm; +pub(crate) use state::TakeFailure; use state::TakeState; use window::WholeWindowState; @@ -166,8 +167,7 @@ impl Take { if let Ok(snapshot) = update { snapshot } else { - self.state - .record_failure("accepted hypothesis capacity is reached".to_owned()); + self.state.record_failure(TakeFailure::HypothesisCapacity); None } } @@ -177,11 +177,11 @@ impl Take { self.state.record_finalized(result, None); } - pub(crate) fn record_failure(&self, failure: impl Into) { - self.state.record_failure(failure.into()); + pub(crate) fn record_failure(&self, failure: TakeFailure) { + self.state.record_failure(failure); } - pub(crate) fn pending_failure(&self) -> Option { + pub(crate) fn pending_failure(&self) -> Option> { self.state.pending_failure() } @@ -213,7 +213,7 @@ impl Take { } #[cfg(test)] - fn take_failure(&self) -> Option { + fn take_failure(&self) -> Option> { self.state.take_failure() } @@ -233,7 +233,7 @@ mod tests { use tokio::sync::{mpsc, oneshot}; - use super::{FinalCommand, FinalSegmentOwner, Take, run_final_pipeline}; + use super::{FinalCommand, FinalSegmentOwner, Take, TakeFailure, run_final_pipeline}; #[test] fn miri_final_segment_reservation_is_exact() { @@ -278,10 +278,10 @@ mod tests { #[test] fn a_take_retains_its_first_final_failure() { let take = Take::without_final(Vec::new()); - take.record_failure("first"); - take.record_failure("second"); + take.record_failure(TakeFailure::Recorded("first".to_owned())); + take.record_failure(TakeFailure::Recorded("second".to_owned())); let failure = take.take_failure().expect("the take owns its failure"); - assert_eq!(failure, "first"); + assert_eq!(failure.to_string(), "first"); } #[tokio::test] @@ -316,8 +316,11 @@ mod tests { .expect("completion queues"); assert_eq!( - completion.await.expect("the completion pipeline replies"), - Ok(String::new()) + completion + .await + .expect("the completion pipeline replies") + .expect("completion succeeds"), + String::new() ); tokio::time::timeout(Duration::from_secs(1), task) .await diff --git a/crates/gateway-stt/src/take/final_decode.rs b/crates/gateway-stt/src/take/final_decode.rs index 6e5a361ac..f95a0e829 100644 --- a/crates/gateway-stt/src/take/final_decode.rs +++ b/crates/gateway-stt/src/take/final_decode.rs @@ -9,7 +9,7 @@ use crate::segment::{FORCED_OVERLAP_SAMPLES, ForcedBoundary}; use super::final_outcome::{FinalRangeOutcome, SkipReason}; use super::pcm::RetainedPcm; -use super::state::TakeState; +use super::state::{TakeFailure, TakeState}; use super::window::WholeWindowState; pub(super) async fn process_samples( @@ -36,7 +36,7 @@ pub(super) async fn process_samples( }; if let Some(reason) = skipped { if forced.is_some() { - state.record_failure("forced final window was not decodable".to_owned()); + state.record_failure(TakeFailure::ForcedWindowNotDecodable); } else { super::finalization::record_outcome( state, @@ -106,11 +106,11 @@ async fn process_forced( }); let outcome = decode(request).await; let Ok(restored) = receiver.await else { - state.record_failure("forced final PCM retirement failed".to_owned()); + state.record_failure(TakeFailure::RetirementFailed); return; }; if !restored { - state.record_failure("forced final PCM ownership became inconsistent".to_owned()); + state.record_failure(TakeFailure::OwnershipInconsistent); return; } record_decode(state, whole_window, outcome, |text| { @@ -147,7 +147,7 @@ fn record_decode( Some(Ok(text)) => { super::finalization::record_outcome(state, whole_window, completed(text)); } - Some(Err(error)) => state.record_failure(error.to_string()), - None => state.record_failure("final transcription worker is unavailable".to_owned()), + Some(Err(error)) => state.record_failure(TakeFailure::Transcribe(error)), + None => state.record_failure(TakeFailure::WorkerUnavailable), } } diff --git a/crates/gateway-stt/src/take/finalization.rs b/crates/gateway-stt/src/take/finalization.rs index a8328b4bc..7b82ff53e 100644 --- a/crates/gateway-stt/src/take/finalization.rs +++ b/crates/gateway-stt/src/take/finalization.rs @@ -12,10 +12,11 @@ use crate::segment::{ForcedBoundary, SegmentOutcome}; use super::final_decode::process_samples; use super::final_outcome::{FinalRangeOutcome, SkipReason}; -use super::state::TakeState; +use super::state::{TakeFailure, TakeState}; use super::window::{AcceptedHypothesis, WholeWindowState}; -pub(super) type TakeFinalization = Pin> + Send>>; +pub(super) type TakeFinalization = + Pin>> + Send>>; pub(super) const FINAL_SEGMENT_CAPACITY: usize = 4; #[derive(Debug)] @@ -35,7 +36,7 @@ pub(super) enum FinalCommand { Complete { committed_samples: u64, accepted: Vec, - reply: oneshot::Sender>, + reply: oneshot::Sender>>, }, } @@ -63,7 +64,7 @@ impl FinalPipeline { break; }; let Some(owner) = FinalSegmentOwner::reserve(&self.pending_segments) else { - state.record_failure("final segment capacity is reached".to_owned()); + state.record_failure(TakeFailure::SegmentCapacity); break; }; let range = match &outcome { @@ -96,11 +97,11 @@ impl FinalPipeline { match self.commands.try_send(command) { Ok(()) => {} Err(mpsc::error::TrySendError::Full(_)) => { - state.record_failure("final segment capacity is reached".to_owned()); + state.record_failure(TakeFailure::SegmentCapacity); break; } Err(mpsc::error::TrySendError::Closed(_)) => { - state.record_failure("final transcription pipeline exited".to_owned()); + state.record_failure(TakeFailure::PipelineExited); break; } } @@ -129,11 +130,11 @@ impl FinalPipeline { .await .is_err() { - return Err("final transcription pipeline exited".to_owned()); + return Err(Arc::new(TakeFailure::PipelineExited)); } reply_rx .await - .unwrap_or_else(|_| Err("final transcription pipeline exited".to_owned())) + .unwrap_or_else(|_| Err(Arc::new(TakeFailure::PipelineExited))) }) } } @@ -309,9 +310,9 @@ mod tests { use tokio::sync::{mpsc, oneshot}; use super::{FINAL_SEGMENT_CAPACITY, FinalCommand, FinalPipeline, run_final_pipeline}; - use crate::take::Take; use crate::take::state::TakeState; use crate::take::window::AcceptedHypothesis; + use crate::take::{Take, TakeFailure}; fn accepted_from_snapshot( take: &Take, @@ -375,10 +376,27 @@ mod tests { assert!(receiver.try_recv().is_err()); assert_eq!(TakeState::lock(&state.buffer).origin(), 0); - assert_eq!( + assert!(matches!( state.pending_failure().as_deref(), - Some("final segment capacity is reached") - ); + Some(TakeFailure::SegmentCapacity) + )); + } + + #[tokio::test] + async fn finalization_reports_a_typed_failure_after_the_pipeline_exits() { + let (commands, receiver) = mpsc::channel(FINAL_SEGMENT_CAPACITY); + drop(receiver); + let pipeline = FinalPipeline { + commands, + task: tokio::spawn(std::future::pending()), + pending_segments: Arc::new(AtomicUsize::new(0)), + }; + + let failure = pipeline + .finalization(0, Vec::new()) + .await + .expect_err("an exited pipeline fails the finalization"); + assert!(matches!(&*failure, TakeFailure::PipelineExited)); } #[tokio::test] @@ -412,8 +430,11 @@ mod tests { .expect("completion queues"); assert_eq!( - completion.await.expect("completion replies"), - Ok("last word".to_owned()) + completion + .await + .expect("completion replies") + .expect("completion succeeds"), + "last word" ); assert_eq!(calls.load(Ordering::SeqCst), 0); task.await.expect("pipeline exits"); @@ -451,8 +472,11 @@ mod tests { .expect("completion queues"); assert_eq!( - completion.await.expect("completion replies"), - Ok(String::new()) + completion + .await + .expect("completion replies") + .expect("completion succeeds"), + String::new() ); assert_eq!(calls.load(Ordering::SeqCst), 0); task.await.expect("pipeline exits"); diff --git a/crates/gateway-stt/src/take/state.rs b/crates/gateway-stt/src/take/state.rs index 94f92859c..b91b842b0 100644 --- a/crates/gateway-stt/src/take/state.rs +++ b/crates/gateway-stt/src/take/state.rs @@ -1,6 +1,5 @@ -use std::sync::{Mutex, MutexGuard, PoisonError}; +use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; -#[cfg(test)] use gateway_stt_engine::TranscribeError; use super::agreement::{ @@ -18,12 +17,47 @@ use crate::segment::{ForcedBoundary, Segmenter}; #[derive(Debug, Default)] struct FinalizedState { text: String, - failure: Option, + failure: Option>, samples: u64, outcomes: Vec, pending_forced: Option, } +/// One typed take failure retained for commit gating and finalization. +/// +/// The failure is shared between the take's slot and the session's precommit +/// gating, so it is reference-counted at the boundary. +#[derive(Debug, thiserror::Error)] +pub(crate) enum TakeFailure { + #[error("final transcript exceeds the 16 KiB window limit")] + TranscriptLimit, + #[error("forced final window was not decoded")] + ForcedWindowNotDecoded, + #[error("forced final window was not decodable")] + ForcedWindowNotDecodable, + #[error("forced final overlap metadata is inconsistent")] + ForcedOverlapInconsistent, + #[error("final outcome capacity is reached")] + OutcomeCapacity, + #[error("final segment capacity is reached")] + SegmentCapacity, + #[error("final transcription pipeline exited")] + PipelineExited, + #[error("accepted hypothesis capacity is reached")] + HypothesisCapacity, + #[error("forced final PCM retirement failed")] + RetirementFailed, + #[error("forced final PCM ownership became inconsistent")] + OwnershipInconsistent, + #[error("final transcription worker is unavailable")] + WorkerUnavailable, + #[error(transparent)] + Transcribe(#[from] TranscribeError), + #[cfg(any(test, feature = "test-fixtures"))] + #[error("{0}")] + Recorded(String), +} + #[derive(Debug)] struct PendingForced { boundary: ForcedBoundary, @@ -103,7 +137,9 @@ impl TakeState { state.samples = samples; } } - Err(error) if state.failure.is_none() => state.failure = Some(error.to_string()), + Err(error) if state.failure.is_none() => { + state.failure = Some(Arc::new(TakeFailure::from(error))); + } Ok(_) | Err(_) => {} } } @@ -121,14 +157,14 @@ impl TakeState { &outcome.result, FinalRangeResult::Decoded(text) if !final_transcript_within_limit(text) ) { - state.failure = Some("final transcript exceeds the 16 KiB window limit".to_owned()); + state.failure = Some(Arc::new(TakeFailure::TranscriptLimit)); return; } match outcome.boundary.clone() { FinalBoundary::Natural => record_natural_outcome(&mut state, outcome, accepted), FinalBoundary::Forced(boundary) => { let FinalRangeResult::Decoded(text) = outcome.result else { - state.failure = Some("forced final window was not decoded".to_owned()); + state.failure = Some(Arc::new(TakeFailure::ForcedWindowNotDecoded)); return; }; record_forced_outcome(&mut state, boundary, text, accepted); @@ -136,10 +172,10 @@ impl TakeState { } } - pub(super) fn record_failure(&self, failure: String) { + pub(super) fn record_failure(&self, failure: TakeFailure) { let mut state = Self::lock(&self.finalized); if state.failure.is_none() { - state.failure = Some(failure); + state.failure = Some(Arc::new(failure)); } } @@ -147,7 +183,7 @@ impl TakeState { Self::lock(&self.finalized).failure.is_some() } - pub(super) fn pending_failure(&self) -> Option { + pub(super) fn pending_failure(&self) -> Option> { Self::lock(&self.finalized).failure.clone() } @@ -165,7 +201,7 @@ impl TakeState { } #[cfg(test)] - pub(super) fn take_failure(&self) -> Option { + pub(super) fn take_failure(&self) -> Option> { Self::lock(&self.finalized).failure.take() } @@ -173,10 +209,10 @@ impl TakeState { &self, accepted: &[AcceptedHypothesis], committed_samples: u64, - ) -> Result { + ) -> Result> { let mut state = Self::lock(&self.finalized); if state.failure.is_none() && !flush_pending_forced(&mut state, accepted) { - state.failure = Some("final outcome capacity is reached".to_owned()); + state.failure = Some(Arc::new(TakeFailure::OutcomeCapacity)); } settle_skipped(&mut state, accepted, false); match state.failure.take() { @@ -230,21 +266,21 @@ fn record_forced_outcome( ) { let Some(overlap) = boundary.overlap() else { if state.pending_forced.is_some() { - state.failure = Some("forced final overlap metadata is inconsistent".to_owned()); + state.failure = Some(Arc::new(TakeFailure::ForcedOverlapInconsistent)); return; } state.pending_forced = Some(PendingForced { boundary, text }); return; }; let Some(previous) = state.pending_forced.take() else { - state.failure = Some("forced final overlap metadata is inconsistent".to_owned()); + state.failure = Some(Arc::new(TakeFailure::ForcedOverlapInconsistent)); return; }; if previous.boundary.decode_range().end != overlap.end || boundary.new_audio().start != overlap.end { state.pending_forced = Some(previous); - state.failure = Some("forced final overlap metadata is inconsistent".to_owned()); + state.failure = Some(Arc::new(TakeFailure::ForcedOverlapInconsistent)); return; } let previous_range = previous.boundary.decode_range(); @@ -262,7 +298,7 @@ fn record_forced_outcome( projected_prefix_end(&previous.text, previous_range.clone(), overlap.start) else { state.pending_forced = Some(previous); - state.failure = Some("forced final overlap metadata is inconsistent".to_owned()); + state.failure = Some(Arc::new(TakeFailure::ForcedOverlapInconsistent)); return; }; tracing::warn!( diff --git a/crates/gateway-stt/src/take/state/alignment_tests/adversaries.rs b/crates/gateway-stt/src/take/state/alignment_tests/adversaries.rs index fb4ce72d3..249436cae 100644 --- a/crates/gateway-stt/src/take/state/alignment_tests/adversaries.rs +++ b/crates/gateway-stt/src/take/state/alignment_tests/adversaries.rs @@ -1,4 +1,7 @@ +use std::sync::Arc; + use super::{FinalRangeOutcome, ForcedBoundary, TakeState}; +use crate::take::TakeFailure; fn completion( first_range: std::ops::Range, @@ -6,7 +9,7 @@ fn completion( new_audio: std::ops::Range, previous: &str, current: &str, -) -> Result { +) -> Result> { let state = TakeState::default(); state.record_final_outcome( FinalRangeOutcome::forced(ForcedBoundary::first(first_range), previous.to_owned()), @@ -22,7 +25,7 @@ fn completion( state.completion(&[], new_audio.end) } -fn assert_estimated(result: Result, expected: &str) { +fn assert_estimated(result: Result>, expected: &str) { assert_eq!( result.expect("weak overlap uses bounded projection"), expected @@ -93,10 +96,12 @@ fn an_over_limit_final_transcript_fails_before_it_can_become_pending() { &[], ); + let failure = state + .completion(&[], 160_000) + .expect_err("over-limit transcript fails"); + assert!(matches!(&*failure, TakeFailure::TranscriptLimit)); assert_eq!( - state - .completion(&[], 160_000) - .expect_err("over-limit transcript fails"), + failure.to_string(), "final transcript exceeds the 16 KiB window limit" ); } diff --git a/crates/gateway-stt/src/take/state/tests.rs b/crates/gateway-stt/src/take/state/tests.rs index a5de1ac21..c03e3e9e9 100644 --- a/crates/gateway-stt/src/take/state/tests.rs +++ b/crates/gateway-stt/src/take/state/tests.rs @@ -2,9 +2,9 @@ use std::sync::Arc; use std::sync::mpsc; use std::time::Duration; -use gateway_stt_engine::TranscribeError; +use gateway_stt_engine::{EnginePolicy, TranscribeError}; -use super::TakeState; +use super::{TakeFailure, TakeState}; use crate::segment::ForcedBoundary; use crate::take::final_outcome::{FinalRangeOutcome, SkipReason}; use crate::take::window::AcceptedHypothesis; @@ -46,6 +46,21 @@ fn finalized_snapshot_cannot_mix_text_and_sample_ownership() { assert_eq!(state.finalized_snapshot(), ("old new".to_owned(), 200)); } +#[test] +fn a_recorded_decode_failure_keeps_its_typed_source() { + let state = TakeState::default(); + let source = EnginePolicy::new(0, 500, false).expect_err("a zero window is rejected"); + let expected = source.to_string(); + state.record_finalized(Err(source), None); + + let failure = state.take_failure().expect("the take owns its failure"); + assert!( + matches!(&*failure, TakeFailure::Transcribe(_)), + "the decode failure stays typed" + ); + assert_eq!(failure.to_string(), expected); +} + #[test] fn natural_speech_and_pause_cycles_settle_without_history_growth() { let state = TakeState::default(); diff --git a/crates/gateway-stt/src/test_fixtures.rs b/crates/gateway-stt/src/test_fixtures.rs index 77a22887a..f33b6b759 100644 --- a/crates/gateway-stt/src/test_fixtures.rs +++ b/crates/gateway-stt/src/test_fixtures.rs @@ -2,9 +2,15 @@ #[cfg(feature = "test-fixtures")] use std::future::Future; +#[cfg(feature = "test-fixtures")] +use std::sync::Arc; +#[cfg(feature = "test-fixtures")] +use crate::SpeechError; #[cfg(feature = "test-fixtures")] use crate::realtime::{CommitReceipt, ItemResult, Session, SessionRegistry}; +#[cfg(feature = "test-fixtures")] +use crate::take::TakeFailure; #[cfg(feature = "test-fixtures")] mod generation; @@ -34,6 +40,108 @@ pub(crate) use native::{jfk_samples, require_model}; #[cfg(feature = "test-fixtures")] pub use segment::segment_ranges; +/// A boxed crate-internal source surfaced through [`FixtureError`]. +#[cfg(feature = "test-fixtures")] +type BoxedSource = Box; + +#[cfg(feature = "test-fixtures")] +fn boxed(source: impl std::error::Error + Send + Sync + 'static) -> BoxedSource { + Box::new(source) +} + +/// One deterministic fixture operation failure. +#[cfg(feature = "test-fixtures")] +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum FixtureError { + /// Session registration was rejected. + #[error("register fixture session")] + #[non_exhaustive] + Register(#[source] BoxedSource), + /// The scripted service failed to start. + #[error("start scripted service")] + #[non_exhaustive] + ScriptedService(#[source] SpeechError), + /// The scripted generation never published an engine. + #[error("scripted generation did not publish")] + ScriptedGenerationNotPublished, + /// The hour simulation service failed to start. + #[error("start hour simulation service")] + #[non_exhaustive] + HourSimulationService(#[source] SpeechError), + /// The hour simulation generation never published an engine. + #[error("hour simulation generation did not publish")] + HourSimulationNotPublished, + /// The session update was rejected. + #[error("apply session update")] + #[non_exhaustive] + SessionUpdate(#[source] BoxedSource), + /// The audio append failed. + #[error("append fixture audio")] + #[non_exhaustive] + Append(#[source] BoxedSource), + /// The input clear failed. + #[error("clear fixture input")] + #[non_exhaustive] + Clear(#[source] BoxedSource), + /// The commit failed. + #[error("commit fixture input")] + #[non_exhaustive] + Commit(#[source] BoxedSource), + /// Recording the precommit failure failed. + #[error("record precommit failure")] + #[non_exhaustive] + RecordPrecommitFailure(#[source] BoxedSource), + /// The delta push failed. + #[error("push fixture delta")] + #[non_exhaustive] + PushDelta(#[source] BoxedSource), + /// The hypothesis replacement failed. + #[error("replace fixture hypothesis")] + #[non_exhaustive] + ReplaceHypothesis(#[source] BoxedSource), + /// The completed terminal outcome was rejected. + #[error("finalize fixture item completed")] + #[non_exhaustive] + FinalizeCompleted(#[source] BoxedSource), + /// The failed terminal outcome was rejected. + #[error("finalize fixture item failed")] + #[non_exhaustive] + FinalizeFailed(#[source] BoxedSource), + /// The finalization replacement failed. + #[error("replace fixture finalization")] + #[non_exhaustive] + ReplaceFinalization(#[source] BoxedSource), + /// The finalization join failed. + #[error("finish fixture finalization")] + #[non_exhaustive] + FinishFinalization(#[source] BoxedSource), + /// The interim task spawn failed. + #[error("spawn fixture interim")] + #[non_exhaustive] + SpawnInterim(#[source] BoxedSource), + /// The interim decode scheduling failed. + #[error("schedule fixture interim")] + #[non_exhaustive] + ScheduleInterim(#[source] BoxedSource), + /// The interim accept failed. + #[error("accept fixture interim")] + #[non_exhaustive] + AcceptInterim(#[source] BoxedSource), + /// The interim join failed. + #[error("finish fixture interim")] + #[non_exhaustive] + FinishInterim(#[source] BoxedSource), + /// The canceled-task join failed. + #[error("join canceled fixture interims")] + #[non_exhaustive] + JoinCanceled(#[source] BoxedSource), + /// Result serialization failed. + #[error("serialize fixture result")] + #[non_exhaustive] + Serialize(#[source] serde_json::Error), +} + /// A deterministic registry for focused Realtime session integration tests. #[cfg(feature = "test-fixtures")] #[derive(Clone, Debug, Default)] @@ -47,8 +155,11 @@ impl RealtimeSessionRegistryFixture { /// /// # Errors /// Returns the stable capacity error when eight sessions are active or retiring. - pub fn register(&self) -> Result { - let registration = self.inner.register().map_err(|error| error.to_string())?; + pub fn register(&self) -> Result { + let registration = self + .inner + .register() + .map_err(|error| FixtureError::Register(boxed(error)))?; Ok(RealtimeSessionFixture { session: Session::new(registration, None), }) @@ -61,13 +172,16 @@ impl RealtimeSessionRegistryFixture { pub fn register_with_scripted_engine( &self, factory: ScriptedModelFactory, - ) -> Result { - let registration = self.inner.register().map_err(|error| error.to_string())?; - let service = scripted_service(factory, 15, 500).map_err(|error| error.to_string())?; + ) -> Result { + let registration = self + .inner + .register() + .map_err(|error| FixtureError::Register(boxed(error)))?; + let service = scripted_service(factory, 15, 500).map_err(FixtureError::ScriptedService)?; let engine = service .state .active() - .ok_or_else(|| "scripted generation did not publish".to_owned())?; + .ok_or(FixtureError::ScriptedGenerationNotPublished)?; Ok(RealtimeSessionFixture { session: Session::new(registration, Some(engine)), }) @@ -80,14 +194,17 @@ impl RealtimeSessionRegistryFixture { pub fn register_with_hour_simulation( &self, probe: &HourSimulationProbe, - ) -> Result { - let registration = self.inner.register().map_err(|error| error.to_string())?; - let service = - hour::hour_simulation_service(probe.clone()).map_err(|error| error.to_string())?; + ) -> Result { + let registration = self + .inner + .register() + .map_err(|error| FixtureError::Register(boxed(error)))?; + let service = hour::hour_simulation_service(probe.clone()) + .map_err(FixtureError::HourSimulationService)?; let engine = service .state .active() - .ok_or_else(|| "hour simulation generation did not publish".to_owned())?; + .ok_or(FixtureError::HourSimulationNotPublished)?; Ok(RealtimeSessionFixture { session: Session::new(registration, Some(engine)), }) @@ -180,28 +297,30 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns the wire validation error for an invalid update. - pub fn update_text(&mut self, text: &str) -> Result<(), String> { + pub fn update_text(&mut self, text: &str) -> Result<(), FixtureError> { self.session .update_text(text) - .map_err(|error| format!("{error:?}")) + .map_err(|error| FixtureError::SessionUpdate(boxed(error))) } /// Appends one Base64-encoded PCM16 chunk. /// /// # Errors /// Returns the audio or session ownership error. - pub fn append_base64(&mut self, payload: &str) -> Result<(), String> { + pub fn append_base64(&mut self, payload: &str) -> Result<(), FixtureError> { self.session .append_base64(payload) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::Append(boxed(error))) } /// Clears uncommitted input and retires its current interim task. /// /// # Errors /// Returns the bounded cleanup or epoch error. - pub fn clear(&mut self) -> Result<(), String> { - self.session.clear().map_err(|error| error.to_string()) + pub fn clear(&mut self) -> Result<(), FixtureError> { + self.session + .clear() + .map_err(|error| FixtureError::Clear(boxed(error))) } /// Returns the current immutable input snapshot. @@ -231,31 +350,31 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns validation or bounded-capacity failures without detaching input. - pub fn commit(&mut self) -> Result { + pub fn commit(&mut self) -> Result { self.session .commit() .map(RealtimeCommitFixture) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::Commit(boxed(error))) } /// Records a final-segment failure before commit. /// /// # Errors /// Returns an error when there is no uncommitted input. - pub fn fail_precommit(&mut self, failure: &str) -> Result<(), String> { + pub fn fail_precommit(&mut self, failure: &str) -> Result<(), FixtureError> { self.session - .record_pending_failure(failure.to_owned()) - .map_err(|error| error.to_string()) + .record_pending_failure(TakeFailure::Recorded(failure.to_owned())) + .map_err(|error| FixtureError::RecordPrecommitFailure(boxed(error))) } /// Adds one accepted nonterminal result to bounded session capacity. /// /// # Errors /// Returns item-state or capacity errors. - pub fn push_delta(&mut self, item_id: &str, transcript: &str) -> Result<(), String> { + pub fn push_delta(&mut self, item_id: &str, transcript: &str) -> Result<(), FixtureError> { self.session .push_delta(item_id, transcript.to_owned()) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::PushDelta(boxed(error))) } /// Replaces the item's newest-wins hypothesis slot. @@ -267,30 +386,34 @@ impl RealtimeSessionFixture { item_id: &str, revision: u64, transcript: &str, - ) -> Result<(), String> { + ) -> Result<(), FixtureError> { self.session .replace_hypothesis(item_id, revision, transcript.to_owned()) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::ReplaceHypothesis(boxed(error))) } /// Records the item's sole successful terminal outcome. /// /// # Errors /// Returns item-state errors, including duplicate terminal attempts. - pub fn finalize_completed(&mut self, item_id: &str, transcript: &str) -> Result<(), String> { + pub fn finalize_completed( + &mut self, + item_id: &str, + transcript: &str, + ) -> Result<(), FixtureError> { self.session .finalize_completed(item_id, transcript.to_owned()) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::FinalizeCompleted(boxed(error))) } /// Records the item's sole failed terminal outcome. /// /// # Errors /// Returns item-state errors, including duplicate terminal attempts. - pub fn finalize_failed(&mut self, item_id: &str, message: &str) -> Result<(), String> { + pub fn finalize_failed(&mut self, item_id: &str, message: &str) -> Result<(), FixtureError> { self.session .finalize_failed(item_id, message.to_owned()) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::FinalizeFailed(boxed(error))) } /// Drains bounded results and releases terminal item ownership. @@ -318,13 +441,16 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns an error when the committed item does not exist. - pub fn replace_finalization(&mut self, item_id: &str, task: F) -> Result<(), String> + pub fn replace_finalization(&mut self, item_id: &str, task: F) -> Result<(), FixtureError> where F: Future> + Send + 'static, { self.session - .replace_finalization(item_id, task) - .map_err(|error| error.to_string()) + .replace_finalization(item_id, async move { + task.await + .map_err(|message| Arc::new(TakeFailure::Recorded(message))) + }) + .map_err(|error| FixtureError::ReplaceFinalization(boxed(error))) } /// Returns the committed immutable prompt and take guidance. @@ -348,25 +474,25 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns item-state, task, decode, or result-mailbox errors. - pub async fn finish_finalization(&mut self, item_id: &str) -> Result<(), String> { + pub async fn finish_finalization(&mut self, item_id: &str) -> Result<(), FixtureError> { self.session .finish_finalization(item_id) .await - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::FinishFinalization(boxed(error))) } /// Spawns one session-owned interim task. /// /// # Errors /// Returns the bounded cleanup or epoch error. - pub fn spawn_interim(&mut self, task: F) -> Result<(), String> + pub fn spawn_interim(&mut self, task: F) -> Result<(), FixtureError> where F: Future + Send + 'static, { self.session .spawn_interim(task) .map(|_| ()) - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::SpawnInterim(boxed(error))) } /// Accepts one interim, clears its input, then rejects the stale epoch. @@ -377,24 +503,26 @@ impl RealtimeSessionFixture { &mut self, current: &str, stale: &str, - ) -> Result<(Option, Option), String> { + ) -> Result<(Option, Option), FixtureError> { let epoch = self .session .begin_interim() - .map_err(|error| error.to_string())?; + .map_err(|error| FixtureError::AcceptInterim(boxed(error)))?; let current = self .session .accept_interim(epoch, current.to_owned()) .map(serde_json::to_value) .transpose() - .map_err(|error| error.to_string())?; - self.session.clear().map_err(|error| error.to_string())?; + .map_err(FixtureError::Serialize)?; + self.session + .clear() + .map_err(|error| FixtureError::Clear(boxed(error)))?; let stale = self .session .accept_interim(epoch, stale.to_owned()) .map(serde_json::to_value) .transpose() - .map_err(|error| error.to_string())?; + .map_err(FixtureError::Serialize)?; Ok((current, stale)) } @@ -402,24 +530,24 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns a task, session, or serialization error. - pub async fn finish_interim(&mut self) -> Result, String> { + pub async fn finish_interim(&mut self) -> Result, FixtureError> { self.session .finish_interim() .await - .map_err(|error| error.to_string())? + .map_err(|error| FixtureError::FinishInterim(boxed(error)))? .map(serde_json::to_value) .transpose() - .map_err(|error| error.to_string()) + .map_err(FixtureError::Serialize) } /// Schedules and accepts one production interim decode. /// /// # Errors /// Returns an audio, generation, task, session, or serialization error. - pub async fn run_interim(&mut self) -> Result, String> { + pub async fn run_interim(&mut self) -> Result, FixtureError> { self.session .schedule_interim() - .map_err(|error| error.to_string())?; + .map_err(|error| FixtureError::ScheduleInterim(boxed(error)))?; self.finish_interim().await } @@ -427,11 +555,11 @@ impl RealtimeSessionFixture { /// /// # Errors /// Returns an error when a canceled task failed instead of canceling. - pub async fn join_canceled(&mut self) -> Result<(), String> { + pub async fn join_canceled(&mut self) -> Result<(), FixtureError> { self.session .join_canceled() .await - .map_err(|error| error.to_string()) + .map_err(|error| FixtureError::JoinCanceled(boxed(error))) } /// Returns the number of retained canceled-task joins. diff --git a/crates/gateway-stt/tests/it/realtime_forced_windows.rs b/crates/gateway-stt/tests/it/realtime_forced_windows.rs index 48d435f02..c0a2b0780 100644 --- a/crates/gateway-stt/tests/it/realtime_forced_windows.rs +++ b/crates/gateway-stt/tests/it/realtime_forced_windows.rs @@ -2,10 +2,14 @@ use std::time::{Duration, Instant}; use base64::Engine as _; use gateway_stt::test_fixtures::{ - HourSimulationProbe, RealtimeSessionFixture, RealtimeSessionRegistryFixture, ScriptedDecoder, - ScriptedModelFactory, hour_marker_input, + FixtureError, HourSimulationProbe, RealtimeSessionFixture, RealtimeSessionRegistryFixture, + ScriptedDecoder, ScriptedModelFactory, hour_marker_input, }; +fn source_message(error: &FixtureError) -> Option { + std::error::Error::source(error).map(ToString::to_string) +} + const WAIT: Duration = Duration::from_secs(2); const INPUT_SAMPLES_PER_STRIDE: usize = 24_000 * 10; const FIRST_FORCED_SAMPLES: usize = 16_000 * 10; @@ -86,7 +90,11 @@ async fn append_after_retirement(session: &mut RealtimeSessionFixture, payload: loop { match session.append_base64(payload) { Ok(()) => return, - Err(error) if error.contains("audio buffer exceeds") && Instant::now() < deadline => { + Err(error) + if source_message(&error) + .is_some_and(|message| message.contains("audio buffer exceeds")) + && Instant::now() < deadline => + { tokio::task::yield_now().await; } Err(error) => panic!("continuous append must succeed after retirement: {error}"), @@ -469,11 +477,13 @@ async fn blocked_forced_decode_enforces_the_thirty_second_aggregate_budget() { provisional ); assert_eq!(session.pending_final_segments(), Some(2)); + let error = session + .append_base64(&encoded_speech()) + .expect_err("capture faster than decoding reaches retained ownership"); + assert_eq!(error.to_string(), "append fixture audio"); assert_eq!( - session - .append_base64(&encoded_speech()) - .expect_err("capture faster than decoding reaches retained ownership"), - "audio buffer exceeds 30 seconds" + source_message(&error).as_deref(), + Some("audio buffer exceeds 30 seconds") ); assert_eq!( session diff --git a/crates/gateway-stt/tests/it/realtime_session.rs b/crates/gateway-stt/tests/it/realtime_session.rs index 07dcebc95..e114d9329 100644 --- a/crates/gateway-stt/tests/it/realtime_session.rs +++ b/crates/gateway-stt/tests/it/realtime_session.rs @@ -8,7 +8,8 @@ use std::time::Duration; use base64::Engine as _; use futures_util::FutureExt as _; use gateway_stt::test_fixtures::{ - RealtimeSessionFixture, RealtimeSessionRegistryFixture, ScriptedDecoder, ScriptedModelFactory, + FixtureError, RealtimeSessionFixture, RealtimeSessionRegistryFixture, ScriptedDecoder, + ScriptedModelFactory, }; const SESSION_CAPACITY: usize = 8; @@ -47,7 +48,7 @@ fn update(prompt: &str, include: bool) -> String { .to_string() } -#[allow( +#[expect( clippy::expect_used, reason = "a fixture registry has no prior session that could consume capacity" )] @@ -57,6 +58,10 @@ fn session() -> RealtimeSessionFixture { .expect("session registers") } +fn source_message(error: &FixtureError) -> Option { + std::error::Error::source(error).map(ToString::to_string) +} + struct BlockingPoll { started: Arc<(Mutex, Condvar)>, release: Arc, @@ -126,9 +131,11 @@ fn session_registration_has_no_wait_queue_at_capacity() { .map(|_| registry.register().expect("session is admitted")) .collect::>(); + let error = registry.register().expect_err("ninth session is rejected"); + assert_eq!(error.to_string(), "register fixture session"); assert_eq!( - registry.register().expect_err("ninth session is rejected"), - "the realtime transcription session limit is reached" + source_message(&error).as_deref(), + Some("the realtime transcription session limit is reached") ); drop(sessions); assert!( @@ -195,7 +202,7 @@ fn failed_first_append_does_not_capture_configuration() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[allow( +#[expect( clippy::await_holding_lock, reason = "the process-wide test lock serializes deliberately blocked runtime workers" )] @@ -234,9 +241,11 @@ async fn dropping_session_retains_admission_until_interim_cleanup_joins() { SESSION_CAPACITY, "retiring work keeps admission owned" ); + let error = registry.register().expect_err("capacity remains occupied"); + assert_eq!(error.to_string(), "register fixture session"); assert_eq!( - registry.register().expect_err("capacity remains occupied"), - "the realtime transcription session limit is reached" + source_message(&error).as_deref(), + Some("the realtime transcription session limit is reached") ); release.store(true, Ordering::Release); @@ -257,7 +266,7 @@ async fn dropping_session_retains_admission_until_interim_cleanup_joins() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[allow( +#[expect( clippy::await_holding_lock, reason = "the process-wide test lock serializes deliberately blocked runtime workers" )] @@ -306,9 +315,11 @@ async fn dropping_session_retains_admission_until_finalization_cleanup_joins() { SESSION_CAPACITY, "retiring finalization keeps admission owned" ); + let error = registry.register().expect_err("capacity remains occupied"); + assert_eq!(error.to_string(), "register fixture session"); assert_eq!( - registry.register().expect_err("capacity remains occupied"), - "the realtime transcription session limit is reached" + source_message(&error).as_deref(), + Some("the realtime transcription session limit is reached") ); release.store(true, Ordering::Release); @@ -402,7 +413,7 @@ async fn canceling_finish_keeps_current_task_owned_for_retry() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[allow( +#[expect( clippy::await_holding_lock, reason = "the process-wide test lock serializes deliberately blocked runtime workers" )] @@ -455,9 +466,11 @@ async fn canceled_join_capacity_is_exact_and_recoverable() { session .spawn_interim(pending()) .expect("current task starts"); + let error = session.clear().expect_err("next retirement is rejected"); + assert_eq!(error.to_string(), "clear fixture input"); assert_eq!( - session.clear().expect_err("next retirement is rejected"), - "the canceled interim task join capacity is reached" + source_message(&error).as_deref(), + Some("the canceled interim task join capacity is reached") ); assert!(session.input_snapshot().is_some()); session.join_canceled().await.expect("retired tasks join"); @@ -485,7 +498,7 @@ fn stale_interim_is_rejected_before_event_id_allocation() { ); } -#[allow( +#[expect( clippy::expect_used, reason = "the helper establishes valid canonical fixture audio and input" )] @@ -500,7 +513,7 @@ fn append_committable(session: &mut RealtimeSessionFixture) -> String { .to_owned() } -#[allow( +#[expect( clippy::expect_used, reason = "the helper establishes valid decodable fixture audio" )] @@ -538,9 +551,11 @@ fn committed_capacity_is_reserved_before_input_detach_and_retryable() { assert_eq!(session.committed_count(), COMMITTED_ITEM_CAPACITY); let retry_id = append_committable(&mut session); + let error = session.commit().expect_err("fifth item is rejected"); + assert_eq!(error.to_string(), "commit fixture input"); assert_eq!( - session.commit().expect_err("fifth item is rejected"), - "the committed realtime item limit is reached" + source_message(&error).as_deref(), + Some("the committed realtime item limit is reached") ); assert_eq!( session @@ -684,11 +699,13 @@ fn result_capacity_hypothesis_replacement_and_terminal_reservation_are_independe .push_delta(item.item_id(), &format!("delta-{index}")) .expect("result enters bounded capacity"); } + let error = session + .push_delta(item.item_id(), "overflow") + .expect_err("capacity-plus-one is rejected"); + assert_eq!(error.to_string(), "push fixture delta"); assert_eq!( - session - .push_delta(item.item_id(), "overflow") - .expect_err("capacity-plus-one is rejected"), - "the realtime session result capacity is reached" + source_message(&error).as_deref(), + Some("the realtime session result capacity is reached") ); session @@ -731,22 +748,26 @@ fn pending_precommit_failure_blocks_append_but_commits_one_item_failure() { session .fail_precommit("accurate segment failed") .expect("failure is retained by the input"); + let error = session + .append_base64(&encoded(&[0, 0])) + .expect_err("failed input rejects later audio"); + assert_eq!(error.to_string(), "append fixture audio"); assert_eq!( - session - .append_base64(&encoded(&[0, 0])) - .expect_err("failed input rejects later audio"), - "accurate segment failed" + source_message(&error).as_deref(), + Some("accurate segment failed") ); let committed = session .commit() .expect("failed input still establishes item"); assert_eq!(committed.item_id(), item_id); + let error = session + .finalize_failed(committed.item_id(), "duplicate") + .expect_err("a second terminal is rejected"); + assert_eq!(error.to_string(), "finalize fixture item failed"); assert_eq!( - session - .finalize_failed(committed.item_id(), "duplicate") - .expect_err("a second terminal is rejected"), - "the committed item already reached a terminal outcome" + source_message(&error).as_deref(), + Some("the committed item already reached a terminal outcome") ); let results = session.drain_results(); assert_eq!(results.len(), 1); @@ -766,6 +787,32 @@ fn clear_discards_pending_precommit_failure_without_creating_an_item() { assert!(session.drain_results().is_empty()); } +#[tokio::test] +async fn fixture_finalization_errors_carry_their_operation_and_source() { + let mut session = session(); + let error = session + .finish_finalization("missing") + .await + .expect_err("an unknown item is rejected"); + assert_eq!(error.to_string(), "finish fixture finalization"); + assert_eq!( + source_message(&error).as_deref(), + Some("the committed item is not active") + ); + + let item_id = append_committable(&mut session); + session.commit().expect("item commits"); + let error = session + .finish_finalization(&item_id) + .await + .expect_err("an item without finalization work is rejected"); + assert_eq!(error.to_string(), "finish fixture finalization"); + assert_eq!( + source_message(&error).as_deref(), + Some("the committed item has no active finalization") + ); +} + #[tokio::test] async fn asynchronous_final_failure_is_observed_before_the_next_append_and_at_commit() { let interim = ScriptedDecoder::new(); @@ -795,12 +842,11 @@ async fn asynchronous_final_failure_is_observed_before_the_next_append_and_at_co .pending_failure() .expect("the take owns the asynchronous failure"); - assert_eq!( - session - .append_base64(&encoded(&[1, 2])) - .expect_err("the next append is rejected before mutating audio"), - failure - ); + let error = session + .append_base64(&encoded(&[1, 2])) + .expect_err("the next append is rejected before mutating audio"); + assert_eq!(error.to_string(), "append fixture audio"); + assert_eq!(source_message(&error).as_deref(), Some(failure.as_str())); let item = session .commit() .expect("commit still establishes the failed item"); @@ -882,11 +928,13 @@ async fn commit_reserves_interim_join_capacity_before_detaching_input() { session .spawn_interim(pending()) .expect("current interim starts"); + let error = session + .commit() + .expect_err("commit cannot detach an unowned task"); + assert_eq!(error.to_string(), "commit fixture input"); assert_eq!( - session - .commit() - .expect_err("commit cannot detach an unowned task"), - "the canceled interim task join capacity is reached" + source_message(&error).as_deref(), + Some("the canceled interim task join capacity is reached") ); assert_eq!( session diff --git a/vibe-ledger.md b/vibe-ledger.md index 3592e2cc2..86ae4c593 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -64,3 +64,5 @@ - Rulebook debt tiers Step 3: non_exhaustive attributes and expect conversions - component-scope verify pass: builds, `cargo fmt --all --check`, both clippy gates, and nextest across the touched crates green, 1925 tests, zero failures (verify-step-3-round-1.log). Decision: wildcard arms on downstream `DecodeMode` matches - `unreachable!` in test factories, `_ => None` fallbacks in initial_load.rs and backend-whisper model.rs | Falsifier: a third variant is added and the chosen fallback proves wrong. Decision: gateway bin `main.rs` wildcard exits `FAILURE` on an unrecognized future `GatewayStartup` variant | Falsifier: a new variant needs serving behavior there. Decision: 8 suppressions were stale (lint never fires) and deleted rather than converted | Falsifier: the `-D warnings` gate, which re-verified each. Decision: cfg-dependent sites use `#[cfg_attr(not(test), expect(...))]` | Falsifier: the gate. Decision: `app.rs` keeps its `#[allow]` per its in-code comment (expectation unfulfilled in some cfg permutations) | Falsifier: clippy behavior changes. - Rulebook debt tiers Step 4: Paused-time conversion for in-process async tests - component-scope verify pass: build, `cargo fmt --all --check`, clippy `-D warnings`, and nextest across promptforge-core, promptforge-lua, and gateway-stt-engine green (verify-step-4-round-1.log); converted tests proven deterministic over 60 repeated runs. Decision: `cancellation_interrupts_a_pending_input_wait` switched from `multi_thread` to `current_thread`, which tokio requires for `start_paused` | Falsifier: the cancel-during-pending-wait assertion is flavor-independent and passes 20/20. Decision: relied on tokio's idle auto-advance rather than explicit `advance()` calls, since the sleeps live in spawned canceller/tool tasks | Falsifier: 60/60 green repetitions. Decision: the gateway-stt-engine rendezvous test's late arrival was restructured to start after the rendezvous times out rather than relying on a timer race, because a paused clock auto-advances into the rendezvous window | Falsifier: tokio's paused runtime shown not to auto-advance while a `spawn_blocking` condvar wait is outstanding. + +- Rulebook debt tiers Step 5: FixtureError and typed sources in the take/finalization pipeline - focused verification: `cargo test -p gateway-stt -F test-fixtures` (125 lib + 53 integration, 0 failed), clippy `-D warnings`, fmt, and feature-off check green; review clean; no Verify dispatch (mid-component step with no fixes). Decision: take failure stored and shared as `Arc` since `pending_failure` clones it out of the mutex for session gating | Falsifier: if the failure were only ever moved, plain `TakeFailure` channels suffice. Decision: `PendingPrecommitFailure` carries `Arc` without `#[source]`; thiserror 2.0.19 `AsDynError` has no `Arc` impl | Falsifier: a thiserror release supporting Arc sources. Decision: `FixtureError` boxes pub(crate) sources as `Box`; `SpeechError`/`serde_json::Error` carried concretely | Falsifier: making `SessionError`/`RegisterError`/`ClientError` public. Decision: `ItemFailure` stays String-carrying (Clone+Eq wire terminal) but is classified from the typed `TakeFailure` | Falsifier: the wire protocol gains typed failure codes. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md index 27c08ea26..cc669219a 100644 --- a/vibe/2026-09-11-1-rulebook-debt-tiers.md +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -243,7 +243,7 @@ Prove the conversions deterministic by running the affected tests repeatedly; th -### Step 5: `FixtureError` and typed sources in the take/finalization pipeline +### Step 5: `FixtureError` and typed sources in the take/finalization pipeline [completed] - Component: stringly-error-elimination From 4a5a532373f113081a02dcbb24cbc9fedeb35f4d Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 11:26:11 -0700 Subject: [PATCH 6/7] Replace string error channels with anyhow and typed errors Test-only validators, build scripts, and build-tool binaries now report failures through anyhow instead of owned strings, while the remaining private production helpers return their crates' existing typed error enums. Two new private error enums give the tool-call parser and the profile-switch driver one variant per rejection, and each variant renders exactly the message the bare string carried, because those texts are wire warnings and user-facing descriptions. The cached Lua shim programs now keep a shared typed cause and re-wrap it at each install rather than flattening it to a string. No error message text changes. - `ToolCallRejection` is a new private thiserror enum with one variant per tool-call rejection; its display text becomes the turn's gateway_warning verbatim, so each variant keeps the exact string the bare channel carried, including the retained serde_json cause on the arguments variant. - `SwitchFailure` is a new private enum for profile-switch failures; its Transport variant keeps the gateway client error as the source while Refused and Failed relay the gateway's own messages, and the display text is what the failure status pushes to the user. - `Error::shared` re-wraps a cached SharedSource as a typed Error::LuaRuntime by cloning the Arc, and both LazyLock program statics now store that shared cause instead of a flattened string. - `parse_whoami_user_sid` takes the cache root and returns LocalError::CacheNotPrivate directly, folding every parse rejection into the existing enum instead of returning a reason string for the caller to wrap. - `anyhow::Result` replaces the string channel in the build-ui helper, the gateway build script, both build-tool argument parsers, the cfg(test) wire validators, and the test-fixture rendezvous; gateway and gateway-stt gate the new dependency behind the test-fixtures feature. - `replace_finalization` accepts anyhow futures from tests and stringifies only at the TakeFailure::Recorded boundary. - `Index::new` returns IndexError::layout for each broken invariant, so the picker drops its map_err. - `crates/gateway-local/src/artifacts/tests.rs` changes only to pass the cache path and compare message text, asserting the same failures as before; no behavior or message text changes anywhere in the diff. Design: removes stringly-typed @ crates/gateway/src/dialect.rs::parse_openai_tool_calls deps: Value Design: removes stringly-typed @ crates/gateway/src/config_write.rs::json_to_toml deps: Value Design: removes stringly-typed @ crates/gateway-local/src/artifacts/confine.rs::parse_whoami_user_sid deps: Path,bool,u8 Design: removes stringly-typed @ crates/workshop-server/src/session/menu.rs::drive_switch deps: GatewayClient,Push,str Design: removes stringly-typed @ crates/promptforge-tool-picker/src/rank.rs::Index::new Design: removes stringly-typed @ crates/promptforge-lua/src/coro.rs::SHIM_PROGRAM Design: removes stringly-typed @ crates/promptforge-lua/src/messages/mod.rs::MESSAGES_PROGRAM Design: removes stringly-typed @ crates/build-llama-cuda/src/main.rs::parse_args deps: String Design: removes stringly-typed @ crates/build-ui/src/lib.rs boundary: pub Design: removes stringly-typed @ crates/build-workshop/src/main.rs Design: removes stringly-typed @ crates/gateway/build.rs Design: removes stringly-typed @ crates/gateway/src/main.rs::wait_for_test_start_rendezvous Design: removes stringly-typed @ crates/gateway/src/test_support.rs::app_state_with_scripted_stt deps: Config,ScriptedModelFactory Design: removes stringly-typed @ crates/gateway-stt/src/realtime/wire/server.rs Design: removes stringly-typed @ crates/gateway-stt/src/test_fixtures.rs::RealtimeSessionFixture::replace_finalization boundary: pub Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- Cargo.lock | 6 ++ crates/build-llama-cuda/src/main.rs | 16 ++-- crates/build-ui/Cargo.toml | 1 + crates/build-ui/src/lib.rs | 39 +++++----- crates/build-workshop/Cargo.toml | 1 + crates/build-workshop/src/main.rs | 44 ++++++----- crates/gateway-local/src/artifacts/confine.rs | 49 ++++++++---- crates/gateway-local/src/artifacts/tests.rs | 25 ++++-- crates/gateway-stt/Cargo.toml | 5 ++ .../gateway-stt/src/realtime/wire/server.rs | 36 ++++----- crates/gateway-stt/src/test_fixtures.rs | 4 +- .../gateway-stt/tests/it/realtime_session.rs | 2 +- crates/gateway/Cargo.toml | 9 ++- crates/gateway/build.rs | 28 ++++--- crates/gateway/src/config_write.rs | 18 +++-- crates/gateway/src/dialect.rs | 76 +++++++++++++++---- crates/gateway/src/main.rs | 8 +- crates/gateway/src/test_support.rs | 5 +- crates/promptforge-lua/src/coro.rs | 22 +++--- crates/promptforge-lua/src/error.rs | 11 +++ crates/promptforge-lua/src/messages/mod.rs | 20 ++--- crates/promptforge-tool-picker/src/picker.rs | 3 +- crates/promptforge-tool-picker/src/rank.rs | 18 +++-- crates/workshop-server/src/session/menu.rs | 52 ++++++++++--- vibe-ledger.md | 2 + vibe/2026-09-11-1-rulebook-debt-tiers.md | 2 +- 26 files changed, 334 insertions(+), 168 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 032ddf48e..b8b4eef24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -559,6 +559,9 @@ dependencies = [ [[package]] name = "build-ui" version = "0.3.0" +dependencies = [ + "anyhow", +] [[package]] name = "build-user-guide" @@ -571,6 +574,7 @@ dependencies = [ name = "build-workshop" version = "0.3.0" dependencies = [ + "anyhow", "ctrlc", "tempfile", ] @@ -2006,6 +2010,7 @@ dependencies = [ name = "gateway" version = "0.3.0" dependencies = [ + "anyhow", "axum", "base64 0.22.1", "block2", @@ -2132,6 +2137,7 @@ dependencies = [ name = "gateway-stt" version = "0.3.0" dependencies = [ + "anyhow", "axum", "base64 0.22.1", "futures-util", diff --git a/crates/build-llama-cuda/src/main.rs b/crates/build-llama-cuda/src/main.rs index 1d5624d50..171923f70 100644 --- a/crates/build-llama-cuda/src/main.rs +++ b/crates/build-llama-cuda/src/main.rs @@ -31,7 +31,7 @@ OPTIONS: /// Parses the command line into a [`BuildRequest`]. Every error exit /// prints the usage text. -fn parse_args(args: &[String]) -> Result { +fn parse_args(args: &[String]) -> anyhow::Result { let mut source: Option = None; let mut tag: Option = None; let mut out: Option = None; @@ -44,7 +44,7 @@ fn parse_args(args: &[String]) -> Result { iter.next() .filter(|value| !value.starts_with("--")) .cloned() - .ok_or_else(|| format!("{arg} needs a value\n\n{USAGE}")) + .ok_or_else(|| anyhow::anyhow!("{arg} needs a value\n\n{USAGE}")) }; match arg.as_str() { "--source" => source = Some(PathBuf::from(value(&mut iter)?)), @@ -56,7 +56,7 @@ fn parse_args(args: &[String]) -> Result { if entry.is_empty() || !entry.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { - return Err(format!( + return Err(anyhow::anyhow!( "malformed --arch entry `{entry}` (expected for example 120a-real)\n\n{USAGE}" )); } @@ -64,15 +64,15 @@ fn parse_args(args: &[String]) -> Result { } } "--no-smoke" => smoke = false, - "-h" | "--help" => return Err(USAGE.to_string()), - other => return Err(format!("unknown argument `{other}`\n\n{USAGE}")), + "-h" | "--help" => return Err(anyhow::anyhow!(USAGE.to_string())), + other => return Err(anyhow::anyhow!("unknown argument `{other}`\n\n{USAGE}")), } } Ok(BuildRequest { - source: source.ok_or_else(|| format!("missing required --source\n\n{USAGE}"))?, - tag: tag.ok_or_else(|| format!("missing required --tag\n\n{USAGE}"))?, - out: out.ok_or_else(|| format!("missing required --out\n\n{USAGE}"))?, + source: source.ok_or_else(|| anyhow::anyhow!("missing required --source\n\n{USAGE}"))?, + tag: tag.ok_or_else(|| anyhow::anyhow!("missing required --tag\n\n{USAGE}"))?, + out: out.ok_or_else(|| anyhow::anyhow!("missing required --out\n\n{USAGE}"))?, archs, smoke, }) diff --git a/crates/build-ui/Cargo.toml b/crates/build-ui/Cargo.toml index bd18cc281..630dc1448 100644 --- a/crates/build-ui/Cargo.toml +++ b/crates/build-ui/Cargo.toml @@ -10,6 +10,7 @@ publish = false description = "Build-script helper that bundles a crate's ui/ sources with esbuild into OUT_DIR" [dependencies] +anyhow.workspace = true [lints] workspace = true diff --git a/crates/build-ui/src/lib.rs b/crates/build-ui/src/lib.rs index d55a4e72c..78c87942a 100644 --- a/crates/build-ui/src/lib.rs +++ b/crates/build-ui/src/lib.rs @@ -44,16 +44,18 @@ pub struct UiBuild { /// bundle. /// /// # Errors -/// Returns an error string when not run through Cargo, when the local +/// Returns an error when not run through Cargo, when the local /// esbuild install is missing or fails, or when a static file cannot be /// copied. -pub fn build(config: UiBuild) -> Result<(), String> { +pub fn build(config: UiBuild) -> anyhow::Result<()> { let manifest_dir = PathBuf::from( std::env::var_os("CARGO_MANIFEST_DIR") - .ok_or("CARGO_MANIFEST_DIR is not set; run through cargo")?, + .ok_or_else(|| anyhow::anyhow!("CARGO_MANIFEST_DIR is not set; run through cargo"))?, + ); + let out_dir = PathBuf::from( + std::env::var_os("OUT_DIR") + .ok_or_else(|| anyhow::anyhow!("OUT_DIR is not set; run through cargo"))?, ); - let out_dir = - PathBuf::from(std::env::var_os("OUT_DIR").ok_or("OUT_DIR is not set; run through cargo")?); let ui_dir = manifest_dir.join("ui"); let dist_dir = out_dir.join("ui-dist"); @@ -63,7 +65,7 @@ pub fn build(config: UiBuild) -> Result<(), String> { // linger into what debug builds serve and release builds embed. if dist_dir.exists() { std::fs::remove_dir_all(&dist_dir) - .map_err(|error| format!("clear {}: {error}", dist_dir.display()))?; + .map_err(|error| anyhow::anyhow!("clear {}: {error}", dist_dir.display()))?; } bundle(&ui_dir, &dist_dir, config.define_app_version)?; copy_static(&ui_dir, &dist_dir, config.static_files)?; @@ -101,7 +103,7 @@ fn watch(ui_dir: &Path, config: &UiBuild) { /// Runs the esbuild bundle step from the local `ui/node_modules` install. /// There is no `npx` fallback: `npx` can download a different esbuild /// version and produce different output. -fn bundle(ui_dir: &Path, dist_dir: &Path, define_app_version: bool) -> Result<(), String> { +fn bundle(ui_dir: &Path, dist_dir: &Path, define_app_version: bool) -> anyhow::Result<()> { let mut command = esbuild_command(ui_dir)?; command.current_dir(ui_dir).args([ "src/main.ts", @@ -114,20 +116,23 @@ fn bundle(ui_dir: &Path, dist_dir: &Path, define_app_version: bool) -> Result<() command.arg("--minify"); } if define_app_version { - let version = std::env::var("CARGO_PKG_VERSION") - .map_err(|error| format!("CARGO_PKG_VERSION is not set: {error}; run through cargo"))?; + let version = std::env::var("CARGO_PKG_VERSION").map_err(|error| { + anyhow::anyhow!("CARGO_PKG_VERSION is not set: {error}; run through cargo") + })?; // Single quotes: esbuild evaluates the define value as a JS string // literal, and unlike double quotes they pass through `cmd /c` on // Windows untouched. command.arg(format!("--define:__APP_VERSION__='{version}'")); } let output = command.output().map_err(|error| { - format!("esbuild could not be started: {error}; install Node.js 22 so it is on PATH") + anyhow::anyhow!( + "esbuild could not be started: {error}; install Node.js 22 so it is on PATH" + ) })?; if output.status.success() { return Ok(()); } - Err(format!( + Err(anyhow::anyhow!( "the UI bundle failed (status {}):\n{}\n{}", output.status, String::from_utf8_lossy(&output.stdout), @@ -138,7 +143,7 @@ fn bundle(ui_dir: &Path, dist_dir: &Path, define_app_version: bool) -> Result<() /// Builds the command that invokes the local esbuild install, failing with /// the setup instructions when `ui/node_modules` is absent. On Windows the /// npm shim is a `.cmd` file, which only runs through `cmd /c`. -fn esbuild_command(ui_dir: &Path) -> Result { +fn esbuild_command(ui_dir: &Path) -> anyhow::Result { let bin_dir = ui_dir.join("node_modules").join(".bin"); #[cfg(windows)] @@ -159,7 +164,7 @@ fn esbuild_command(ui_dir: &Path) -> Result { } } - Err(format!( + Err(anyhow::anyhow!( "ui/node_modules is missing; run `npm ci` in {} first", ui_dir.display() )) @@ -167,17 +172,17 @@ fn esbuild_command(ui_dir: &Path) -> Result { /// Copies the static UI files next to the bundle, keeping the relative /// paths. -fn copy_static(ui_dir: &Path, dist_dir: &Path, static_files: &[&str]) -> Result<(), String> { +fn copy_static(ui_dir: &Path, dist_dir: &Path, static_files: &[&str]) -> anyhow::Result<()> { std::fs::create_dir_all(dist_dir) - .map_err(|error| format!("create {}: {error}", dist_dir.display()))?; + .map_err(|error| anyhow::anyhow!("create {}: {error}", dist_dir.display()))?; for file in static_files { let target = dist_dir.join(file); if let Some(parent) = target.parent() { std::fs::create_dir_all(parent) - .map_err(|error| format!("create the parent for {file}: {error}"))?; + .map_err(|error| anyhow::anyhow!("create the parent for {file}: {error}"))?; } std::fs::copy(ui_dir.join(file), &target) - .map_err(|error| format!("copy ui/{file} into the bundle output: {error}"))?; + .map_err(|error| anyhow::anyhow!("copy ui/{file} into the bundle output: {error}"))?; } Ok(()) } diff --git a/crates/build-workshop/Cargo.toml b/crates/build-workshop/Cargo.toml index 9b80a98de..94154afc6 100644 --- a/crates/build-workshop/Cargo.toml +++ b/crates/build-workshop/Cargo.toml @@ -14,6 +14,7 @@ name = "build-workshop" path = "src/main.rs" [dependencies] +anyhow.workspace = true ctrlc.workspace = true [dev-dependencies] diff --git a/crates/build-workshop/src/main.rs b/crates/build-workshop/src/main.rs index 64a43efae..3c33fa81e 100644 --- a/crates/build-workshop/src/main.rs +++ b/crates/build-workshop/src/main.rs @@ -158,18 +158,22 @@ impl InterruptController { } } -static PROCESS_INTERRUPT: OnceLock> = OnceLock::new(); +static PROCESS_INTERRUPT: OnceLock>> = + OnceLock::new(); -fn install_interrupt_handler() -> Result { +fn install_interrupt_handler() -> Result> { match PROCESS_INTERRUPT.get_or_init(|| { let interrupt = InterruptController::isolated(); let handler_interrupt = interrupt.clone(); - ctrlc::set_handler(move || handler_interrupt.request()) - .map_err(|error| format!("cannot install the interrupt handler: {error}"))?; + ctrlc::set_handler(move || handler_interrupt.request()).map_err(|error| { + Arc::new(anyhow::anyhow!( + "cannot install the interrupt handler: {error}" + )) + })?; Ok(interrupt) }) { Ok(interrupt) => Ok(interrupt.clone()), - Err(error) => Err(error.clone()), + Err(error) => Err(Arc::clone(error)), } } @@ -289,13 +293,13 @@ struct BuildEnvironment { } impl BuildEnvironment { - fn discover() -> Result { + fn discover() -> Result { let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let workspace_root = manifest_dir .parent() .and_then(|crates| crates.parent()) .ok_or_else(|| { - format!( + anyhow::anyhow!( "cannot derive the workspace root from {}", manifest_dir.display() ) @@ -303,7 +307,7 @@ impl BuildEnvironment { .to_path_buf(); let target_root = match std::env::var_os("CARGO_TARGET_DIR") { Some(value) if value.is_empty() => { - return Err("CARGO_TARGET_DIR must not be empty".to_owned()); + return Err(anyhow::anyhow!("CARGO_TARGET_DIR must not be empty")); } Some(value) => { let path = PathBuf::from(value); @@ -311,7 +315,9 @@ impl BuildEnvironment { path } else { std::env::current_dir() - .map_err(|error| format!("cannot read the current directory: {error}"))? + .map_err(|error| { + anyhow::anyhow!("cannot read the current directory: {error}") + })? .join(path) } } @@ -321,7 +327,7 @@ impl BuildEnvironment { .or_else(|| option_env!("CARGO").map(OsString::from)) .map(PathBuf::from) .ok_or_else(|| { - "Cargo did not provide the executable used for this command".to_owned() + anyhow::anyhow!("Cargo did not provide the executable used for this command") })?; Ok(Self { workspace_root, @@ -355,7 +361,7 @@ impl fmt::Display for BuildError { } } -fn parse_arguments(args: &[String]) -> Result { +fn parse_arguments(args: &[String]) -> Result { let mut profile = Profile::Debug; let mut release_seen = false; let mut target = None; @@ -363,7 +369,7 @@ fn parse_arguments(args: &[String]) -> Result { while index < args.len() { match args[index].as_str() { "--release" if release_seen => { - return Err(format!("duplicate argument `--release`\n\n{USAGE}")); + return Err(anyhow::anyhow!("duplicate argument `--release`\n\n{USAGE}")); } "--release" => { profile = Profile::Release; @@ -371,14 +377,14 @@ fn parse_arguments(args: &[String]) -> Result { index += 1; } "--target" if target.is_some() => { - return Err(format!("duplicate argument `--target`\n\n{USAGE}")); + return Err(anyhow::anyhow!("duplicate argument `--target`\n\n{USAGE}")); } "--target" => { let value = args.get(index + 1).ok_or_else(|| { - format!("argument `--target` needs a target triple\n\n{USAGE}") + anyhow::anyhow!("argument `--target` needs a target triple\n\n{USAGE}") })?; if value.starts_with('-') || !valid_target_triple(value) { - return Err(format!( + return Err(anyhow::anyhow!( "argument `--target` needs a valid target triple, got `{value}`\n\n{USAGE}" )); } @@ -386,7 +392,9 @@ fn parse_arguments(args: &[String]) -> Result { index += 2; } argument => { - return Err(format!("unsupported argument `{argument}`\n\n{USAGE}")); + return Err(anyhow::anyhow!( + "unsupported argument `{argument}`\n\n{USAGE}" + )); } } } @@ -837,7 +845,9 @@ mod tests { vec!["--profile".to_owned(), "dist".to_owned()], vec!["gateway".to_owned()], ] { - let error = parse_arguments(&args).expect_err("unsupported argument"); + let error = parse_arguments(&args) + .expect_err("unsupported argument") + .to_string(); assert!(error.contains("unsupported argument"), "{error}"); assert!(error.contains(USAGE), "{error}"); } diff --git a/crates/gateway-local/src/artifacts/confine.rs b/crates/gateway-local/src/artifacts/confine.rs index 19e31ba12..6a22bc9c8 100644 --- a/crates/gateway-local/src/artifacts/confine.rs +++ b/crates/gateway-local/src/artifacts/confine.rs @@ -106,29 +106,34 @@ fn current_windows_sid(root: &Path) -> Result { path: root.to_owned(), source, })?; - parse_whoami_user_sid(output.status.success(), &output.stdout, &output.stderr).map_err( - |reason| LocalError::CacheNotPrivate { - path: root.to_owned(), - reason, - }, + parse_whoami_user_sid( + root, + output.status.success(), + &output.stdout, + &output.stderr, ) } /// Parses one `whoami /user /fo csv /nh` record into its canonical SID. #[cfg(any(windows, test))] pub(super) fn parse_whoami_user_sid( + root: &Path, command_succeeded: bool, stdout: &[u8], stderr: &[u8], -) -> std::result::Result { +) -> Result { + let not_private = |reason: String| LocalError::CacheNotPrivate { + path: root.to_owned(), + reason, + }; if !command_succeeded { let detail = String::from_utf8_lossy(stderr); let detail = detail.trim(); - return Err(if detail.is_empty() { + return Err(not_private(if detail.is_empty() { "whoami identity query failed".to_owned() } else { format!("whoami identity query failed: {detail}") - }); + })); } let record = stdout @@ -136,38 +141,50 @@ pub(super) fn parse_whoami_user_sid( .or_else(|| stdout.strip_suffix(b"\n")) .unwrap_or(stdout); if record.is_empty() { - return Err("whoami identity output is empty".to_owned()); + return Err(not_private("whoami identity output is empty".to_owned())); } if record.contains(&b'\r') || record.contains(&b'\n') { - return Err("whoami identity output contains multiple records".to_owned()); + return Err(not_private( + "whoami identity output contains multiple records".to_owned(), + )); } let Some(inner) = record .strip_prefix(b"\"") .and_then(|value| value.strip_suffix(b"\"")) else { - return Err("whoami identity output is not quoted CSV".to_owned()); + return Err(not_private( + "whoami identity output is not quoted CSV".to_owned(), + )); }; let mut separators = inner .windows(3) .enumerate() .filter(|(_, window)| *window == b"\",\""); let Some((separator, _)) = separators.next() else { - return Err("whoami identity output does not contain two fields".to_owned()); + return Err(not_private( + "whoami identity output does not contain two fields".to_owned(), + )); }; if separators.next().is_some() { - return Err("whoami identity output contains extra fields".to_owned()); + return Err(not_private( + "whoami identity output contains extra fields".to_owned(), + )); } let account = &inner[..separator]; let sid_bytes = &inner[separator + 3..]; if !account.iter().any(|byte| !byte.is_ascii_whitespace()) || account.contains(&b'"') { - return Err("whoami identity output has an invalid account".to_owned()); + return Err(not_private( + "whoami identity output has an invalid account".to_owned(), + )); } let sid = std::str::from_utf8(sid_bytes) - .map_err(|_| "whoami identity output has a non-UTF-8 SID".to_owned())?; + .map_err(|_| not_private("whoami identity output has a non-UTF-8 SID".to_owned()))?; if !is_canonical_windows_sid(sid) { - return Err("whoami identity output has a non-canonical SID".to_owned()); + return Err(not_private( + "whoami identity output has a non-canonical SID".to_owned(), + )); } Ok(sid.to_owned()) } diff --git a/crates/gateway-local/src/artifacts/tests.rs b/crates/gateway-local/src/artifacts/tests.rs index 8b5b6112f..86b69f882 100644 --- a/crates/gateway-local/src/artifacts/tests.rs +++ b/crates/gateway-local/src/artifacts/tests.rs @@ -473,6 +473,7 @@ fn concurrent_provisioning_of_same_url_is_safe() { #[test] fn whoami_user_parser_accepts_an_ordinary_account_sid() { let sid = super::confine::parse_whoami_user_sid( + std::path::Path::new("cache"), true, br#""DESKTOP-EXAMPLE\alice","S-1-5-21-111111111-222222222-333333333-1001" "#, @@ -486,6 +487,7 @@ fn whoami_user_parser_accepts_an_ordinary_account_sid() { #[test] fn whoami_user_parser_accepts_a_well_known_service_sid() { let sid = super::confine::parse_whoami_user_sid( + std::path::Path::new("cache"), true, b"\"NT AUTHORITY\\NETWORK SERVICE\",\"S-1-5-20\"\r\n", b"", @@ -508,7 +510,8 @@ fn whoami_user_parser_rejects_malformed_or_multiple_csv_records() { b"\"alice\",\"S-1-5-4294967296\"".as_slice(), ] { assert!( - super::confine::parse_whoami_user_sid(true, output, b"").is_err(), + super::confine::parse_whoami_user_sid(std::path::Path::new("cache"), true, output, b"") + .is_err(), "unexpectedly accepted {output:?}" ); } @@ -516,21 +519,33 @@ fn whoami_user_parser_rejects_malformed_or_multiple_csv_records() { #[test] fn whoami_user_parser_rejects_a_missing_sid() { - assert!(super::confine::parse_whoami_user_sid(true, b"\"alice\",\"\"\r\n", b"").is_err()); - assert!(super::confine::parse_whoami_user_sid(true, b"", b"").is_err()); + assert!( + super::confine::parse_whoami_user_sid( + std::path::Path::new("cache"), + true, + b"\"alice\",\"\"\r\n", + b"" + ) + .is_err() + ); + assert!( + super::confine::parse_whoami_user_sid(std::path::Path::new("cache"), true, b"", b"") + .is_err() + ); } #[test] fn whoami_user_parser_rejects_command_failure() { let error = super::confine::parse_whoami_user_sid( + std::path::Path::new("cache"), false, b"\"alice\",\"S-1-5-21-1-2-3-1001\"\r\n", b"ERROR: access denied\r\n", ) .expect_err("failed whoami must not yield a SID"); - assert!(error.contains("whoami identity query failed")); - assert!(error.contains("access denied")); + assert!(error.to_string().contains("whoami identity query failed")); + assert!(error.to_string().contains("access denied")); } #[test] diff --git a/crates/gateway-stt/Cargo.toml b/crates/gateway-stt/Cargo.toml index 2f6a8dcd0..ff3c3ed37 100644 --- a/crates/gateway-stt/Cargo.toml +++ b/crates/gateway-stt/Cargo.toml @@ -10,6 +10,9 @@ publish = false description = "PromptForge gateway-owned speech-to-text runtime and HTTP endpoints" [dependencies] +# Optional: the fixture API's controlled-finalization task bound, enabled +# only by the `test-fixtures` feature. +anyhow = { workspace = true, optional = true } axum.workspace = true base64.workspace = true futures-util.workspace = true @@ -27,6 +30,7 @@ tokio-util.workspace = true tracing.workspace = true [dev-dependencies] +anyhow.workspace = true gateway-stt = { path = ".", features = ["test-fixtures"] } sha2.workspace = true tempfile.workspace = true @@ -35,6 +39,7 @@ tower.workspace = true [features] test-fixtures = [ + "dep:anyhow", "gateway-stt-backend-whisper/test-fixtures", "gateway-stt-engine/test-fixtures", ] diff --git a/crates/gateway-stt/src/realtime/wire/server.rs b/crates/gateway-stt/src/realtime/wire/server.rs index 4ee905b3a..3e566f960 100644 --- a/crates/gateway-stt/src/realtime/wire/server.rs +++ b/crates/gateway-stt/src/realtime/wire/server.rs @@ -1,3 +1,5 @@ +#[cfg(test)] +use anyhow::anyhow; use serde::{Deserialize, Serialize}; #[cfg(test)] use serde_json::Value; @@ -106,7 +108,7 @@ impl EffectiveSession { } #[cfg(test)] - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> anyhow::Result<()> { if self.id.is_empty() || self.object != SESSION_OBJECT || self.r#type != SESSION_TYPE @@ -118,7 +120,7 @@ impl EffectiveSession { || !(self.include.is_empty() || matches!(self.include.as_slice(), [value] if value == HYPOTHESIS_INCLUDE)) { - return Err("invalid effective transcription session".to_owned()); + return Err(anyhow!("invalid effective transcription session")); } Ok(()) } @@ -244,14 +246,14 @@ impl ServerEvent { } #[cfg(test)] - pub(in crate::realtime) fn from_value(value: Value) -> Result { - let event: Self = serde_json::from_value(value).map_err(|error| error.to_string())?; + pub(in crate::realtime) fn from_value(value: Value) -> anyhow::Result { + let event: Self = serde_json::from_value(value)?; event.validate()?; Ok(event) } #[cfg(test)] - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> anyhow::Result<()> { let (event_id, item_id, content_index) = match self { Self::SessionCreated { event_id, session } | Self::SessionUpdated { event_id, session } => { @@ -308,14 +310,14 @@ impl ServerEvent { validate_id(item_id)?; } if content_index.is_some_and(|index| *index != 0) { - return Err("content_index must be zero".to_owned()); + return Err(anyhow!("content_index must be zero")); } match self { Self::TranscriptionCompleted { usage, .. } => usage.validate(), Self::TranscriptionFailed { error, .. } => { error.validate()?; if error.has_event_id() { - return Err("item failure must not contain a client event ID".to_owned()); + return Err(anyhow!("item failure must not contain a client event ID")); } Ok(()) } @@ -331,7 +333,7 @@ impl ServerEvent { } if transcript != &format!("{finalized}{agreed}{tentative}") || audio_start_ms > audio_end_ms => { - Err("invalid hypothesis snapshot".to_owned()) + Err(anyhow!("invalid hypothesis snapshot")) } _ => Ok(()), } @@ -340,7 +342,7 @@ impl ServerEvent { impl ConversationItem { #[cfg(test)] - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> anyhow::Result<()> { validate_id(&self.id)?; if self.r#type != "message" || self.status != "completed" @@ -349,7 +351,7 @@ impl ConversationItem { || self.content[0].r#type != "input_audio" || !self.content[0].transcript.is_null() { - return Err("invalid conversation item".to_owned()); + return Err(anyhow!("invalid conversation item")); } Ok(()) } @@ -357,9 +359,9 @@ impl ConversationItem { impl DurationUsage { #[cfg(test)] - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> anyhow::Result<()> { if self.r#type != "duration" || !self.seconds.is_finite() || self.seconds < 0.0 { - return Err("invalid duration usage".to_owned()); + return Err(anyhow!("invalid duration usage")); } Ok(()) } @@ -372,29 +374,29 @@ impl WireError { } #[cfg(test)] - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> anyhow::Result<()> { if self.r#type.is_empty() || self.code.is_empty() || self.message.is_empty() || self.param.invalid_empty() || self.event_id.invalid_empty() { - return Err("invalid wire error".to_owned()); + return Err(anyhow!("invalid wire error")); } Ok(()) } } #[cfg(test)] -fn validate_id(id: &str) -> Result<(), String> { +fn validate_id(id: &str) -> anyhow::Result<()> { if id.is_empty() { - Err("opaque ID must not be empty".to_owned()) + Err(anyhow!("opaque ID must not be empty")) } else { Ok(()) } } #[cfg(test)] -fn validate_optional_id(id: Option<&str>) -> Result<(), String> { +fn validate_optional_id(id: Option<&str>) -> anyhow::Result<()> { id.map_or(Ok(()), validate_id) } diff --git a/crates/gateway-stt/src/test_fixtures.rs b/crates/gateway-stt/src/test_fixtures.rs index f33b6b759..4b39b1f08 100644 --- a/crates/gateway-stt/src/test_fixtures.rs +++ b/crates/gateway-stt/src/test_fixtures.rs @@ -443,12 +443,12 @@ impl RealtimeSessionFixture { /// Returns an error when the committed item does not exist. pub fn replace_finalization(&mut self, item_id: &str, task: F) -> Result<(), FixtureError> where - F: Future> + Send + 'static, + F: Future> + Send + 'static, { self.session .replace_finalization(item_id, async move { task.await - .map_err(|message| Arc::new(TakeFailure::Recorded(message))) + .map_err(|error| Arc::new(TakeFailure::Recorded(error.to_string()))) }) .map_err(|error| FixtureError::ReplaceFinalization(boxed(error))) } diff --git a/crates/gateway-stt/tests/it/realtime_session.rs b/crates/gateway-stt/tests/it/realtime_session.rs index e114d9329..d4c2a8575 100644 --- a/crates/gateway-stt/tests/it/realtime_session.rs +++ b/crates/gateway-stt/tests/it/realtime_session.rs @@ -89,7 +89,7 @@ impl Future for BlockingPoll { struct BlockingFinalization(BlockingPoll); impl Future for BlockingFinalization { - type Output = Result; + type Output = anyhow::Result; fn poll(mut self: Pin<&mut Self>, context: &mut Context<'_>) -> Poll { Pin::new(&mut self.0).poll(context).map(Ok) diff --git a/crates/gateway/Cargo.toml b/crates/gateway/Cargo.toml index 43b55be80..f33fc5894 100644 --- a/crates/gateway/Cargo.toml +++ b/crates/gateway/Cargo.toml @@ -23,7 +23,13 @@ path = "src/main.rs" [target.'cfg(windows)'.build-dependencies] embed-resource.workspace = true +[build-dependencies] +anyhow.workspace = true + [dependencies] +# Optional: anyhow error payloads for the test-fixture rendezvous in +# src/main.rs, enabled only by the `test-fixtures` feature. +anyhow = { workspace = true, optional = true } axum.workspace = true dotenvy.workspace = true futures-util.workspace = true @@ -132,9 +138,10 @@ config-ui = ["dep:gateway-config-ui"] # build stubs the route and refuses a configuration declaring [[stt_model]]. stt = ["dep:gateway-stt"] # Deterministic local-runtime bindings for cross-crate behavior tests. -test-fixtures = ["local", "gateway-local/test-fixtures"] +test-fixtures = ["local", "gateway-local/test-fixtures", "dep:anyhow"] [dev-dependencies] +anyhow.workspace = true base64.workspace = true hound.workspace = true # Encodes the generated test image for the live CUDA projector proof. diff --git a/crates/gateway/build.rs b/crates/gateway/build.rs index 0dc6faa61..20aaf8677 100644 --- a/crates/gateway/build.rs +++ b/crates/gateway/build.rs @@ -44,10 +44,10 @@ const MANIFEST: &str = r#" "#; -fn main() -> Result<(), String> { +fn main() -> anyhow::Result<()> { let manifest_dir = PathBuf::from( std::env::var_os("CARGO_MANIFEST_DIR") - .ok_or("CARGO_MANIFEST_DIR is not set; run through cargo")?, + .ok_or_else(|| anyhow::anyhow!("CARGO_MANIFEST_DIR is not set; run through cargo"))?, ); let icon = manifest_dir.join(ICON); println!("cargo::rerun-if-changed={}", icon.display()); @@ -62,20 +62,22 @@ fn main() -> Result<(), String> { /// and compiles it with `rc.exe` (MSVC) or `windres` (GNU) through /// `embed-resource`, which links the result into every binary target. #[cfg(windows)] -fn embed_resources(icon: &Path) -> Result<(), String> { +fn embed_resources(icon: &Path) -> anyhow::Result<()> { use embed_resource::CompilationResult; - let out_dir = - PathBuf::from(std::env::var_os("OUT_DIR").ok_or("OUT_DIR is not set; run through cargo")?); + let out_dir = PathBuf::from( + std::env::var_os("OUT_DIR") + .ok_or_else(|| anyhow::anyhow!("OUT_DIR is not set; run through cargo"))?, + ); let icon = icon .to_str() - .ok_or_else(|| format!("the icon path {} is not UTF-8", icon.display()))?; + .ok_or_else(|| anyhow::anyhow!("the icon path {} is not UTF-8", icon.display()))?; let manifest_path = out_dir.join("promptforge-gateway.manifest.xml"); std::fs::write(&manifest_path, MANIFEST) - .map_err(|error| format!("write {}: {error}", manifest_path.display()))?; - let manifest = manifest_path - .to_str() - .ok_or_else(|| format!("the manifest path {} is not UTF-8", manifest_path.display()))?; + .map_err(|error| anyhow::anyhow!("write {}: {error}", manifest_path.display()))?; + let manifest = manifest_path.to_str().ok_or_else(|| { + anyhow::anyhow!("the manifest path {} is not UTF-8", manifest_path.display()) + })?; // The resource compiler reads the file names as C string literals, so // path separators need doubling. Icon resource id 1: Explorer shows // the first icon group in the resource table, and this exe has one. @@ -88,7 +90,7 @@ fn embed_resources(icon: &Path) -> Result<(), String> { ); let script_path = out_dir.join("promptforge-gateway.rc"); std::fs::write(&script_path, script) - .map_err(|error| format!("write {}: {error}", script_path.display()))?; + .map_err(|error| anyhow::anyhow!("write {}: {error}", script_path.display()))?; match embed_resource::compile(&script_path, embed_resource::NONE) { CompilationResult::Ok | CompilationResult::NotWindows => Ok(()), // A toolchain without a resource compiler still builds; the icon @@ -100,7 +102,9 @@ fn embed_resources(icon: &Path) -> Result<(), String> { println!("cargo::warning=the exe resources were not embedded: {reason}"); Ok(()) } - CompilationResult::Failed(reason) => Err(format!("compile the exe resources: {reason}")), + CompilationResult::Failed(reason) => { + Err(anyhow::anyhow!("compile the exe resources: {reason}")) + } } } diff --git a/crates/gateway/src/config_write.rs b/crates/gateway/src/config_write.rs index 219b500de..8d0742f7d 100644 --- a/crates/gateway/src/config_write.rs +++ b/crates/gateway/src/config_write.rs @@ -78,11 +78,9 @@ pub(crate) fn error_chain(error: &dyn std::error::Error) -> String { /// Converts the request body into the TOML document a shadow save takes. fn toml_document(body: serde_json::Value) -> Result { - let value = json_to_toml(body) - .map_err(GatewayError::ConfigWriteRejected)? - .ok_or_else(|| { - GatewayError::ConfigWriteRejected("the body must be a JSON object".to_owned()) - })?; + let value = json_to_toml(body)?.ok_or_else(|| { + GatewayError::ConfigWriteRejected("the body must be a JSON object".to_owned()) + })?; if value.is_table() { Ok(value) } else { @@ -97,7 +95,7 @@ fn toml_document(body: serde_json::Value) -> Result { /// absent optionals on the way out, and the deserializer defaults them on /// the way back in). A null inside an array has no such reading and is an /// error, as is a number outside TOML's ranges. -fn json_to_toml(value: serde_json::Value) -> Result, String> { +fn json_to_toml(value: serde_json::Value) -> Result, GatewayError> { Ok(Some(match value { serde_json::Value::Null => return Ok(None), serde_json::Value::Bool(flag) => toml::Value::Boolean(flag), @@ -107,7 +105,9 @@ fn json_to_toml(value: serde_json::Value) -> Result, String> } else if let Some(float) = number.as_f64() { toml::Value::Float(float) } else { - return Err(format!("number {number} does not fit a TOML value")); + return Err(GatewayError::ConfigWriteRejected(format!( + "number {number} does not fit a TOML value" + ))); } } serde_json::Value::String(text) => toml::Value::String(text), @@ -115,7 +115,9 @@ fn json_to_toml(value: serde_json::Value) -> Result, String> let mut converted = Vec::with_capacity(items.len()); for item in items { let Some(element) = json_to_toml(item)? else { - return Err("null inside an array has no TOML form".to_owned()); + return Err(GatewayError::ConfigWriteRejected( + "null inside an array has no TOML form".to_owned(), + )); }; converted.push(element); } diff --git a/crates/gateway/src/dialect.rs b/crates/gateway/src/dialect.rs index 9f9e7b7aa..3b78e32ba 100644 --- a/crates/gateway/src/dialect.rs +++ b/crates/gateway/src/dialect.rs @@ -325,10 +325,56 @@ fn peel_json_tool_calls_fence(input: &str) -> Peel<'_> { } match parse_openai_tool_calls(raw_calls) { Ok(calls) => Peel::Calls(calls, after), - Err(reason) => Peel::Malformed(reason), + Err(rejection) => Peel::Malformed(rejection.to_string()), } } +/// Why one OpenAI `tool_calls` entry was rejected rather than coerced. +/// +/// The display text becomes the turn's `gateway_warning` verbatim, so each +/// variant's message is the exact wire string. +#[derive(Debug, thiserror::Error)] +enum ToolCallRejection { + /// The entry was not a JSON object. + #[error("tool call was not an object")] + NotObject, + /// The entry's `type` was not the string `"function"`. + #[error("tool call `type` must be the string \"function\"")] + TypeNotFunction, + /// The entry had no string `id`. + #[error("tool call had no string id")] + NoStringId, + /// The entry's `id` was blank. + #[error("tool call id was blank")] + BlankId, + /// The entry's `id` already appeared earlier in the same turn. + #[error("duplicate tool call id {0:?} within one turn")] + DuplicateId(String), + /// The entry had no `function` member. + #[error("tool call had no function")] + NoFunction, + /// The entry's `function` was not an object. + #[error("tool call `function` was not an object")] + FunctionNotObject, + /// The function had no string `name`. + #[error("tool call had no string name")] + NoStringName, + /// The function's `name` was blank. + #[error("tool call name was blank")] + BlankName, + /// The function's `arguments` string did not decode as JSON. The decode + /// failure is retained as the cause; its text stays in the message + /// because the message is the wire warning. + #[error("tool call arguments were not valid JSON: {0}")] + ArgumentsNotJson(#[source] serde_json::Error), + /// The decoded `arguments` were not a JSON object. + #[error("tool call arguments did not decode to an object")] + ArgumentsNotObject, + /// The function's `arguments` were missing or not a string. + #[error("tool call arguments were missing or not a string")] + ArgumentsMissing, +} + /// Parse the OpenAI `message.tool_calls` array into [`ParsedCall`]s. /// /// Each call must be an object with a nonblank string `id`, a `type` of @@ -337,39 +383,37 @@ fn peel_json_tool_calls_fence(input: &str) -> Peel<'_> { /// to a JSON object. Blank identifiers, duplicate ids within the turn, missing /// or null arguments, and arguments that do not decode to an object are all /// rejected rather than coerced. -fn parse_openai_tool_calls(raw_calls: &[Value]) -> Result, String> { +fn parse_openai_tool_calls(raw_calls: &[Value]) -> Result, ToolCallRejection> { let mut calls = Vec::with_capacity(raw_calls.len()); let mut seen_ids: std::collections::HashSet<&str> = std::collections::HashSet::new(); for raw in raw_calls { if !raw.is_object() { - return Err("tool call was not an object".to_owned()); + return Err(ToolCallRejection::NotObject); } match raw.get("type") { Some(Value::String(kind)) if kind == "function" => {} - _ => return Err("tool call `type` must be the string \"function\"".to_owned()), + _ => return Err(ToolCallRejection::TypeNotFunction), } let id = raw .get("id") .and_then(Value::as_str) - .ok_or_else(|| "tool call had no string id".to_owned())?; + .ok_or(ToolCallRejection::NoStringId)?; if id.trim().is_empty() { - return Err("tool call id was blank".to_owned()); + return Err(ToolCallRejection::BlankId); } if !seen_ids.insert(id) { - return Err(format!("duplicate tool call id {id:?} within one turn")); + return Err(ToolCallRejection::DuplicateId(id.to_owned())); } - let function = raw - .get("function") - .ok_or_else(|| "tool call had no function".to_owned())?; + let function = raw.get("function").ok_or(ToolCallRejection::NoFunction)?; if !function.is_object() { - return Err("tool call `function` was not an object".to_owned()); + return Err(ToolCallRejection::FunctionNotObject); } let name = function .get("name") .and_then(Value::as_str) - .ok_or_else(|| "tool call had no string name".to_owned())?; + .ok_or(ToolCallRejection::NoStringName)?; if name.trim().is_empty() { - return Err("tool call name was blank".to_owned()); + return Err(ToolCallRejection::BlankName); } // OpenAI encodes `function.arguments` as a JSON string. It must be // present, a string, and decode to a JSON object - the shape tools @@ -378,13 +422,13 @@ fn parse_openai_tool_calls(raw_calls: &[Value]) -> Result, Strin let arguments = match function.get("arguments") { Some(Value::String(raw_args)) => { let decoded = serde_json::from_str::(raw_args) - .map_err(|error| format!("tool call arguments were not valid JSON: {error}"))?; + .map_err(ToolCallRejection::ArgumentsNotJson)?; if !decoded.is_object() { - return Err("tool call arguments did not decode to an object".to_owned()); + return Err(ToolCallRejection::ArgumentsNotObject); } decoded } - _ => return Err("tool call arguments were missing or not a string".to_owned()), + _ => return Err(ToolCallRejection::ArgumentsMissing), }; calls.push(ParsedCall { id: id.to_owned(), diff --git a/crates/gateway/src/main.rs b/crates/gateway/src/main.rs index 727d5888b..55c95081e 100644 --- a/crates/gateway/src/main.rs +++ b/crates/gateway/src/main.rs @@ -185,14 +185,14 @@ fn main() -> ExitCode { /// Test-only process rendezvous used by integration tests that must place /// multiple production binaries immediately before lease acquisition. #[cfg(feature = "test-fixtures")] -fn wait_for_test_start_rendezvous() -> Result<(), String> { +fn wait_for_test_start_rendezvous() -> anyhow::Result<()> { let ready = std::env::var_os(TEST_START_READY_ENV); let release = std::env::var_os(TEST_START_RELEASE_ENV); let (Some(ready), Some(release)) = (&ready, &release) else { return if ready.is_none() && release.is_none() { Ok(()) } else { - Err(format!( + Err(anyhow::anyhow!( "{TEST_START_READY_ENV} and {TEST_START_RELEASE_ENV} must be set together" )) }; @@ -200,11 +200,11 @@ fn wait_for_test_start_rendezvous() -> Result<(), String> { let ready = PathBuf::from(ready); let release = PathBuf::from(release); std::fs::write(&ready, b"ready") - .map_err(|error| format!("write test start marker {}: {error}", ready.display()))?; + .map_err(|error| anyhow::anyhow!("write test start marker {}: {error}", ready.display()))?; let deadline = std::time::Instant::now() + TEST_START_TIMEOUT; while !release.is_file() { if std::time::Instant::now() >= deadline { - return Err(format!( + return Err(anyhow::anyhow!( "test start release {} did not arrive within {TEST_START_TIMEOUT:?}", release.display() )); diff --git a/crates/gateway/src/test_support.rs b/crates/gateway/src/test_support.rs index bf6a125c6..c2e9707af 100644 --- a/crates/gateway/src/test_support.rs +++ b/crates/gateway/src/test_support.rs @@ -67,9 +67,8 @@ pub(crate) fn app_state(config: Config, paths: Option) -> AppState { pub(crate) fn app_state_with_scripted_stt( config: Config, factory: gateway_stt::test_fixtures::ScriptedModelFactory, -) -> Result { - let service = gateway_stt::test_fixtures::scripted_service(factory, 15, 500) - .map_err(|error| error.to_string())?; +) -> anyhow::Result { + let service = gateway_stt::test_fixtures::scripted_service(factory, 15, 500)?; let mut state = app_state(config, None); state.speech = service; Ok(state) diff --git a/crates/promptforge-lua/src/coro.rs b/crates/promptforge-lua/src/coro.rs index 7156f034c..3303fa1d8 100644 --- a/crates/promptforge-lua/src/coro.rs +++ b/crates/promptforge-lua/src/coro.rs @@ -16,7 +16,7 @@ use std::sync::LazyLock; use mlua::{Function, Table, Value}; -use super::{Error, Lua, LuaProgram, Result, StdLib, var_snapshot_table}; +use super::{Error, Lua, LuaProgram, Result, SharedSource, StdLib, var_snapshot_table}; /// The shim chunk's name: `@`-prefixed so PUC renders it verbatim as a file /// path, making unexpected shim errors clickable `file:line:` references. @@ -48,11 +48,13 @@ const LOOP_REGISTRY: &str = "promptforge.impl_coro.loop"; const USER_INPUT_REGISTRY: &str = "promptforge.impl_coro.user_input"; /// The shim program, compiled once and loaded per VM. Compilation of the -/// bundled source fails only on a crate bug, so the payload is the error's -/// display string (the crate `Error` is not `Clone`). -static SHIM_PROGRAM: LazyLock> = LazyLock::new(|| { - LuaProgram::compile_internal(SHIM_SOURCE, SHIM_CHUNK_NAME).map_err(|error| error.to_string()) -}); +/// bundled source fails only on a crate bug, so the payload is a shareable +/// [`SharedSource`] cause (the crate `Error` is not `Clone`), re-wrapped as +/// a typed error at each install. +static SHIM_PROGRAM: LazyLock> = + LazyLock::new(|| { + LuaProgram::compile_internal(SHIM_SOURCE, SHIM_CHUNK_NAME).map_err(SharedSource::new) + }); /// Installs the yield shims on a VM whose host tables already exist. /// @@ -80,9 +82,7 @@ pub(crate) fn install_shim_prelude(lua: &Lua) -> Result<()> { .map_err(Error::lua)?; let models: Table = globals.raw_get("models").map_err(Error::lua)?; let tools: Table = globals.raw_get("tools").map_err(Error::lua)?; - let program = SHIM_PROGRAM - .as_ref() - .map_err(|message| Error::Lua(message.clone()))?; + let program = SHIM_PROGRAM.as_ref().map_err(Error::shared)?; let shims: Table = program .load(lua)? .call((yield_fn, var_snapshot, models, tools)) @@ -188,9 +188,7 @@ pub fn install_live_h1_shim_base(lua: &Lua) -> Result<()> { let var_snapshot = lua .create_function(|lua, ()| var_snapshot_table(lua).map_err(mlua::Error::external)) .map_err(Error::lua)?; - let program = SHIM_PROGRAM - .as_ref() - .map_err(|message| Error::Lua(message.clone()))?; + let program = SHIM_PROGRAM.as_ref().map_err(Error::shared)?; let shims: Table = program .load(lua)? .call((yield_fn, var_snapshot, Value::Nil, Value::Nil)) diff --git a/crates/promptforge-lua/src/error.rs b/crates/promptforge-lua/src/error.rs index 10ca50c3f..125915193 100644 --- a/crates/promptforge-lua/src/error.rs +++ b/crates/promptforge-lua/src/error.rs @@ -321,6 +321,17 @@ impl Error { } } + /// Re-produces a typed [`Error`] from a [`SharedSource`] a cache or + /// static captured once and replays on every lookup (the compiled-program + /// statics), cloning the `Arc` rather than flattening the cause to a + /// string. + pub(crate) fn shared(source: &SharedSource) -> Error { + Error::LuaRuntime { + message: source.to_string(), + source: Box::new(source.clone()), + } + } + /// Wrap a tool failure as [`Error::Tool`], preserving the tool's own /// error as the `#[source]` cause rather than discarding it. pub(crate) fn tool(source: promptforge_tools::ToolError) -> Error { diff --git a/crates/promptforge-lua/src/messages/mod.rs b/crates/promptforge-lua/src/messages/mod.rs index cb877e30e..81e5267cb 100644 --- a/crates/promptforge-lua/src/messages/mod.rs +++ b/crates/promptforge-lua/src/messages/mod.rs @@ -20,7 +20,7 @@ use std::sync::LazyLock; use mlua::{Function, Table}; -use super::{Error, Lua, LuaProgram, Result}; +use super::{Error, Lua, LuaProgram, Result, SharedSource}; /// The builders chunk's name: `@`-prefixed so PUC renders it verbatim as a /// file path, making unexpected shim errors clickable `file:line:` @@ -31,21 +31,21 @@ const MESSAGES_CHUNK_NAME: &str = "@crates/promptforge-lua/src/messages/__impl_m const MESSAGES_SOURCE: &str = include_str!("__impl_messages.lua"); /// The builders program, compiled once and loaded per VM. Compilation of the -/// bundled source fails only on a crate bug, so the payload is the error's -/// display string (the crate `Error` is not `Clone`). -static MESSAGES_PROGRAM: LazyLock> = LazyLock::new(|| { - LuaProgram::compile_internal(MESSAGES_SOURCE, MESSAGES_CHUNK_NAME) - .map_err(|error| error.to_string()) -}); +/// bundled source fails only on a crate bug, so the payload is a shareable +/// [`SharedSource`] cause (the crate `Error` is not `Clone`), re-wrapped as +/// a typed error at each install. +static MESSAGES_PROGRAM: LazyLock> = + LazyLock::new(|| { + LuaProgram::compile_internal(MESSAGES_SOURCE, MESSAGES_CHUNK_NAME) + .map_err(SharedSource::new) + }); /// Installs the `messages` global carrying the pure-Lua `new` builder. /// /// # Errors /// Returns [`Error::Lua`] if the builders chunk or the global install fails. pub(crate) fn install_messages(lua: &Lua, globals: &Table) -> Result<()> { - let program = MESSAGES_PROGRAM - .as_ref() - .map_err(|message| Error::Lua(message.clone()))?; + let program = MESSAGES_PROGRAM.as_ref().map_err(Error::shared)?; let new: Function = program.load(lua)?.call(()).map_err(Error::lua)?; let messages = lua.create_table().map_err(Error::lua)?; messages.raw_set("new", new).map_err(Error::lua)?; diff --git a/crates/promptforge-tool-picker/src/picker.rs b/crates/promptforge-tool-picker/src/picker.rs index 28ea2ec1b..d907cb27d 100644 --- a/crates/promptforge-tool-picker/src/picker.rs +++ b/crates/promptforge-tool-picker/src/picker.rs @@ -157,8 +157,7 @@ impl ToolPicker { if let Some(handle) = progress { handle.complete(); } - let index = - Index::new(rows, EMBEDDING_DIMENSIONS, catalog.len()).map_err(IndexError::layout)?; + let index = Index::new(rows, EMBEDDING_DIMENSIONS, catalog.len())?; Ok(Self { catalog, config, diff --git a/crates/promptforge-tool-picker/src/rank.rs b/crates/promptforge-tool-picker/src/rank.rs index d682c6659..ac5169439 100644 --- a/crates/promptforge-tool-picker/src/rank.rs +++ b/crates/promptforge-tool-picker/src/rank.rs @@ -11,6 +11,8 @@ use std::cmp::Ordering; +use crate::error::IndexError; + /// One scored tool: where it sits in the catalog and how well it matched. /// /// A candidate is a finding, not a decision, and it is produced only by ranking @@ -91,20 +93,22 @@ impl Index { /// Builds a validated index from a flat `rows` buffer of `count` rows. /// /// # Errors - /// Returns a description of the broken invariant when `stride` is zero or - /// `rows` is not exactly `stride * count` long. - pub(crate) fn new(rows: Vec, stride: usize, count: usize) -> Result { + /// Returns [`IndexError`] when `stride` is zero or `rows` is not exactly + /// `stride * count` long. + pub(crate) fn new(rows: Vec, stride: usize, count: usize) -> Result { if stride == 0 { - return Err("the model reported a zero embedding dimension".to_owned()); + return Err(IndexError::layout( + "the model reported a zero embedding dimension", + )); } let expected = stride .checked_mul(count) - .ok_or_else(|| "the vector buffer length overflowed".to_owned())?; + .ok_or_else(|| IndexError::layout("the vector buffer length overflowed"))?; if rows.len() != expected { - return Err(format!( + return Err(IndexError::layout(format!( "the vector buffer holds {} floats, expected {expected} for {count} rows of {stride}", rows.len() - )); + ))); } Ok(Self { rows, stride }) } diff --git a/crates/workshop-server/src/session/menu.rs b/crates/workshop-server/src/session/menu.rs index d48b60aff..eb522bccb 100644 --- a/crates/workshop-server/src/session/menu.rs +++ b/crates/workshop-server/src/session/menu.rs @@ -8,7 +8,9 @@ use axum::extract::ws::WebSocket; use futures_util::StreamExt; use crate::app::AppState; -use crate::gateway::{GatewayClient, GatewayResponse, SwitchEvent, SwitchResponse, switch_events}; +use crate::gateway::{ + GatewayClient, GatewayError, GatewayResponse, SwitchEvent, SwitchResponse, switch_events, +}; use crate::heartbeat::{refresh_catalog, refresh_profiles}; use crate::menu::SwitchOutcome; use crate::protocol::Activity; @@ -97,34 +99,66 @@ async fn run_switch(client: &GatewayClient, push: &Push, name: &str) { push.menu().finish_switch(SwitchOutcome::Completed); push.push_idle(); } - Err(message) => { + Err(failure) => { push.menu().finish_switch(SwitchOutcome::Failed); - push.push_failure("Profile switch failed", message, Activity::General); + push.push_failure( + "Profile switch failed", + failure.to_string(), + Activity::General, + ); } } } +/// Why one profile switch did not complete. The display text is the +/// user-facing description pushed with the failure status, so each +/// variant renders exactly the message the stringly channel carried. +#[derive(Debug, thiserror::Error)] +enum SwitchFailure { + /// The switch request or its stage stream failed in transit; the + /// client's typed error is retained as the cause. + #[error(transparent)] + Transport(GatewayError), + /// The gateway refused the switch before starting it; the payload is + /// the gateway's own refusal message, relayed verbatim. + #[error("{0}")] + Refused(String), + /// The gateway reported a terminal failure mid-switch; the payload is + /// the gateway's own error message, relayed verbatim. + #[error("{0}")] + Failed(String), + /// The stage stream ended with no terminal `ready` or `error` event. + #[error("the switch stream ended without a terminal event")] + StreamEnded, +} + /// Posts the switch and consumes its stage stream, pushing each stage /// marker as determinate progress, until the terminal event: `Ok` on /// `ready`, the failure's description on everything else - a terminal /// `error`, a buffered refusal, a transport failure, or a stream that /// ends without a terminal event. -async fn drive_switch(client: &GatewayClient, push: &Push, name: &str) -> Result<(), String> { +async fn drive_switch( + client: &GatewayClient, + push: &Push, + name: &str, +) -> Result<(), SwitchFailure> { let payloads = match client.switch_profile(name).await { Ok(SwitchResponse::Switching { payloads, .. }) => payloads, - Ok(SwitchResponse::Buffered(refusal)) => return Err(switch_refusal(&refusal)), - Err(error) => return Err(error.to_string()), + Ok(SwitchResponse::Buffered(refusal)) => { + return Err(SwitchFailure::Refused(switch_refusal(&refusal))); + } + Err(error) => return Err(SwitchFailure::Transport(error)), }; let mut events = switch_events(payloads); while let Some(item) = events.next().await { match item { Ok(SwitchEvent::Stage { stage }) => push_stage(push, name, &stage), Ok(SwitchEvent::Ready { .. }) => return Ok(()), - Ok(SwitchEvent::Error { message }) => return Err(message), - Err(error) => return Err(error.to_string()), + Ok(SwitchEvent::Error { message }) => return Err(SwitchFailure::Failed(message)), + Err(error) => return Err(SwitchFailure::Transport(error)), } } - Err("the switch stream ended without a terminal event".to_string()) + Err(SwitchFailure::StreamEnded) } /// Pushes one stage marker as determinate status-bar progress - stage diff --git a/vibe-ledger.md b/vibe-ledger.md index 86ae4c593..6a51f8f50 100644 --- a/vibe-ledger.md +++ b/vibe-ledger.md @@ -66,3 +66,5 @@ - Rulebook debt tiers Step 4: Paused-time conversion for in-process async tests - component-scope verify pass: build, `cargo fmt --all --check`, clippy `-D warnings`, and nextest across promptforge-core, promptforge-lua, and gateway-stt-engine green (verify-step-4-round-1.log); converted tests proven deterministic over 60 repeated runs. Decision: `cancellation_interrupts_a_pending_input_wait` switched from `multi_thread` to `current_thread`, which tokio requires for `start_paused` | Falsifier: the cancel-during-pending-wait assertion is flavor-independent and passes 20/20. Decision: relied on tokio's idle auto-advance rather than explicit `advance()` calls, since the sleeps live in spawned canceller/tool tasks | Falsifier: 60/60 green repetitions. Decision: the gateway-stt-engine rendezvous test's late arrival was restructured to start after the rendezvous times out rather than relying on a timer race, because a paused clock auto-advances into the rendezvous window | Falsifier: tokio's paused runtime shown not to auto-advance while a `spawn_blocking` condvar wait is outstanding. - Rulebook debt tiers Step 5: FixtureError and typed sources in the take/finalization pipeline - focused verification: `cargo test -p gateway-stt -F test-fixtures` (125 lib + 53 integration, 0 failed), clippy `-D warnings`, fmt, and feature-off check green; review clean; no Verify dispatch (mid-component step with no fixes). Decision: take failure stored and shared as `Arc` since `pending_failure` clones it out of the mutex for session gating | Falsifier: if the failure were only ever moved, plain `TakeFailure` channels suffice. Decision: `PendingPrecommitFailure` carries `Arc` without `#[source]`; thiserror 2.0.19 `AsDynError` has no `Arc` impl | Falsifier: a thiserror release supporting Arc sources. Decision: `FixtureError` boxes pub(crate) sources as `Box`; `SpeechError`/`serde_json::Error` carried concretely | Falsifier: making `SessionError`/`RegisterError`/`ClientError` public. Decision: `ItemFailure` stays String-carrying (Clone+Eq wire terminal) but is classified from the typed `TakeFailure` | Falsifier: the wire protocol gains typed failure codes. + +- Rulebook debt tiers Step 6: anyhow for test and build code, typed errors in remaining production - FULL gate pass: build, fmt, clippy (pinned and newer stable), docs, full workspace tests, workshop nextest, and doctests all green (verify-step-6-round-1.log); review clean including the cumulative component diff. Decision: build-ui uses anyhow, not a new thiserror type | Falsifier: the contract allows exactly one new public error type (FixtureError) and routes build tooling to anyhow. Decision: menu.rs gains private `SwitchFailure` with Display byte-identical to the old strings | Falsifier: no existing workshop-server enum models switch lifecycle. Decision: dialect.rs gains private `ToolCallRejection` with Display matching the old wire warnings exactly | Falsifier: the reasons become `gateway_warning` wire strings, so the text had to survive. Decision: confine.rs `parse_whoami_user_sid` returns `LocalError::CacheNotPrivate` directly | Falsifier: keeps the existing variant and message while dropping the String channel. Decision: the two LazyLock statics hold `SharedSource`, replayed via a new `Error::shared` helper | Falsifier: SharedSource exists precisely for re-producing typed errors from non-Clone caches. diff --git a/vibe/2026-09-11-1-rulebook-debt-tiers.md b/vibe/2026-09-11-1-rulebook-debt-tiers.md index cc669219a..4fb31c678 100644 --- a/vibe/2026-09-11-1-rulebook-debt-tiers.md +++ b/vibe/2026-09-11-1-rulebook-debt-tiers.md @@ -255,7 +255,7 @@ Tests asserting the new error chains ship in the same commit; the component-scop -### Step 6: anyhow for test and build code, typed errors in remaining production +### Step 6: anyhow for test and build code, typed errors in remaining production [completed] - Component: stringly-error-elimination From 64c575c702c6beda044b7cdb3ec2dad2ceefae17 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 11 Sep 2026 11:46:39 -0700 Subject: [PATCH 7/7] Close plan: rulebook debt tiers Plan: vibe/2026-09-11-1-rulebook-debt-tiers.md --- vibe/ACTIVE | 1 - 1 file changed, 1 deletion(-) delete mode 100644 vibe/ACTIVE diff --git a/vibe/ACTIVE b/vibe/ACTIVE deleted file mode 100644 index 9b34a710b..000000000 --- a/vibe/ACTIVE +++ /dev/null @@ -1 +0,0 @@ -vibe/2026-09-11-1-rulebook-debt-tiers.md \ No newline at end of file