build: split the Cargo gates into a contributor set and a product set - #5468
Conversation
📝 WalkthroughWalkthroughThe PR separates contributor and product Rust feature sets. It adds product-feature parsing and forwarding validation, gates feature-dependent integration tests, and applies product features across CI, coverage, and mock test workflows. ChangesProduct Feature Forwarding
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ProductManifest
participant ProductFeaturesShell
participant CIWorkflows
participant Cargo
ProductManifest->>ProductFeaturesShell: define product feature gates
ProductFeaturesShell->>CIWorkflows: provide comma-separated feature list
CIWorkflows->>Cargo: run tests, clippy, and coverage with product features
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.9223 · 424,334 in / 14,685 out · 37,504 cached (9%) · minimax/minimax-m3, moonshotai/kimi-k3
critique: $0.1312 · 43,882 in / 107 out · 768 cached (2%) · moonshotai/kimi-k3
security: $0.7420 · 251,789 in / 5,282 out · 34,304 cached (14%) · moonshotai/kimi-k3
tests: $0.0167 · 44,673 in / 2,712 out · 0 cached (0%) · minimax/minimax-m3
commits: $0.0103 · 22,209 in / 3,306 out · 1,536 cached (7%) · minimax/minimax-m3
description: $0.0162 · 44,999 in / 2,439 out · 896 cached (2%) · minimax/minimax-m3
| # Strip comments and blank lines, then join with commas. Refuse to emit an | ||
| # empty list: a lane silently running with NO features would look green while | ||
| # covering nothing, which is the failure mode this whole guard exists to stop. | ||
| LIST="$(sed -e 's/#.*//' -e 's/[[:space:]]//g' "$FILE" | grep -v '^$' | paste -sd, -)" |
There was a problem hiding this comment.
product-features.sh empty-list exit-2 path is not covered by a test
product-features.sh exits 2 if the parsed list is empty or the file is missing, which is the load-bearing behaviour the comment above it claims: "a lane silently running with NO features would look green while covering nothing". The only shell-side test is the shell script and the JS parser agree on the product set, which exercises the happy path with the checked-in file. There is no test that runs product-features.sh against an empty file (or a missing file) and asserts exit 2 — meaning a regression that quietly emits an empty string instead of failing would pass the test suite.
rule Did behaviour change, and does a test now fail if that behaviour regresses? ·
| openhuman_core::core::bus::init() | ||
| .await | ||
| .expect("bus init"); | ||
| openhuman_core::core::bus::init().await.expect("bus init"); |
There was a problem hiding this comment.
Unannounced bus-API rename threaded through tests and src
The diff silently renames crate::core::event_bus to crate::core::bus (with events::DomainEvent moved alongside it) and rewrites call sites across src/core/*, src/openhuman/**, and many tests/*.rs files to match. That is a substantial refactor — the old register_native_global::<...> free function becomes BUS.native().register::<...>, the old synchronous raw_receiver() becomes an async subscription that a new test helper polls. The PR title and body describe none of this. If this is the repair-on-main commit stacked ahead of the gate split, the description needs to call it out, or — better — this part should land separately so the gate split review stays focused.
rule PR body says it splits the default feature list, rewrites the forwarding guard, adds required-featuresto four test targets, adds atui smoke lane, and updates the clippy lane. It does not mention any core-bus rename, API change, or migration. ·
| // safe to call pre-boot with no `#[cfg(test)]` guard. | ||
| crate::core::event_bus::publish_global( | ||
| crate::core::event_bus::DomainEvent::MemoryDriverBindFailed { | ||
| crate::core::bus::BUS.publish( |
There was a problem hiding this comment.
Guard MemoryDriverBindFailed publishers against the new bus
memory::binding::build calls BUS.publish(...) rather than the old crate::core::event_bus::publish_global(...). The PR body does not call this out, and it is a behaviour-level change: the old call site is documented as a no-op before init, the new one is documented in the surrounding publish_guard_denied comment as the same no-op pre-boot, but the call chain through tinybus::NativeRegistry is different. Reviewers should at least see this called out in the description.
rule The PR is described as a build-time / feature-flag split, but it also rewrites the global event-bus surface. ·
| # gated-off domains, so every CI lane that builds or tests "the product" now | ||
| # passes `--features "$(scripts/ci/product-features.sh)"`. If you add a lane, | ||
| # decide which of the two sets it is testing and say so. | ||
| default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging"] |
There was a problem hiding this comment.
medulla is contributor-default but never on the product list
medulla stays in [features] default (9 contributor gates as claimed) but is absent from scripts/ci/product-features.txt. It is also the only entry besides tui in INTENTIONALLY_NOT_FORWARDED. So every contributor pays for the medulla orchestration-backend client on every edit, and the product never uses it. If the goal was a lean contributor set, leaving medulla in is the opposite of what the PR describes — it should be moved to product-only or dropped, otherwise the package count claimed (356) is over-stated. Confidence is moderate because I cannot see the full dep graph from here.
rule Repository coding rules: "Decide whether a new CI lane tests the contributor or product feature set and say so in a comment; pass --features "$(bash scripts/ci/product-features.sh)" for product lanes." ·
`[features] default` was the product set, so every contributor paid for the whole product on every edit: web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds, the cpal/hound/arboard/enigo/rdev stack behind `voice`+`inference`, `contacts`' macOS objc2 cohort, `crash-reporting`'s sentry tree, `tui`'s ratatui. A bare `cargo check` — and therefore rust-analyzer on every keystroke — resolved 567 packages and ran 7 native C/C++ builds. `default` is now the CONTRIBUTOR set: 9 gates that cost almost nothing to compile, so `cargo check` still typechecks nearly the whole tree. Measured: **567 -> 356 packages, 7 -> 5 native builds.** The kernel floor is untouched at 307/284/5 — this axis does not intersect the `flows` profile. THIS DOES NOT CHANGE WHAT SHIPS. `app/src-tauri` has declared `openhuman_core` with `default-features = false` since tinyhumansai#1061 and never inherited `default`; that is precisely why a forwarding guard had to exist at all. `check-feature-forwarding.mjs` worked by diffing the shell's forwarding list against `[features] default`. That is a SUBSET check, and it was sound only while `default` was the product set. Shrinking `default` makes it weaker with every gate removed and vacuous if `default` ever empties — silently re-arming and ~93k Sentry events. Landing the flip without the rewrite would have been the same bug with a longer fuse. The product set is now explicit, in `scripts/ci/product-features.txt`, and the guard asserts three things: 1. the shell forwards EXACTLY that file — set equality, both directions. A dropped gate fails on `missing`; a gate the shell grew that the product never claimed fails on `unexpected`. Equality cannot pass vacuously. 2. every name in the file is a gate the core actually declares (catches a typo or a gate renamed out from under it). 3. every `default` gate is forwarded or allow-listed — the original check, retained because it still catches a contributor-set gate nobody shipped. Two parsers read that file — this one and `product-features.sh`, which the CI lanes use to build `--features`. A test asserts they agree, because if they drifted CI would compile a different set than the guard checks. A lane that relied on default features silently stops covering the product. Every lane that builds or tests the product now passes `--features "$(bash scripts/ci/product-features.sh)"`: core clippy, the unit lane, the changed-files coverage lane, and `scripts/test-rust-with-mock.sh`. Clippy runs TWICE, once per set — a lint that only fires with the gates off (an import left unused once a domain is compiled out) is invisible to the product lane and would be red on every contributor's machine while CI is green. Four `tests/*.rs` targets name symbols that only exist behind `voice`, `web3`, `inference` or `crash-reporting`, so they now declare `required-features`. Without it a bare `cargo test` fails to COMPILE, in files the contributor did not touch. With it cargo SKIPS them — the same silent-skip trap `--bins` without `bin-tools` already had, which is why the lane comments spell it out: `json_rpc_e2e` alone is >12k lines of RPC contract coverage. `tui` ends up in NEITHER set (default-OFF and deliberately not forwarded), so nothing in CI would compile it at all. The feature-gate-smoke lane now checks it explicitly; a future gate in that position needs the same. Note `required-features` is the blunt instrument — it costs the whole target when a gate is off, where tinyhumansai#5021's per-symbol `#[cfg]` cleanup would keep the ungated tests running. Taken deliberately for these four; noted in Cargo.toml. Verified: contributor `cargo check --lib --tests`, product `cargo check --all-targets --features <product>`, `--no-default-features --features tui`, the 27 guard self-tests, the kernel-floor ratchet, and `cargo fmt --all --check`. Co-authored-by: Medulla <medulla@tinyhumans.ai>
03c84b6 to
722f104
Compare
coderabbitai
Bot
added
the
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 358-360: Update the Product row in the feature-set comparison
table to report 16 gates instead of 15, while leaving the other package and
native-build counts unchanged.
In `@scripts/ci/rust-coverage-changed.sh`:
- Around line 40-51: Update the coverage feature setup around PRODUCT_FEATURES
and llvm_cov to include the bin-tools feature, matching the feature gate used by
rsut-core-tests. Ensure llvm_cov passes this augmented feature set for coverage
commands such as --bins, --doc, and --test while preserving the existing
no-feature handling for clean, report, and show-env.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 02340847-f81e-4925-b0ae-7ef9ed521285
📒 Files selected for processing (12)
.github/workflows/ci-lite.yml.github/workflows/test-reusable.ymlAGENTS.mdCargo.tomldocs/library-minimal-recipe.mdscripts/__tests__/feature-forwarding.test.mjsscripts/ci/check-feature-forwarding.mjsscripts/ci/product-features.shscripts/ci/product-features.txtscripts/ci/rust-coverage-changed.shscripts/lib/feature-forwarding.mjsscripts/test-rust-with-mock.sh
| | **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. ~356 packages / 5 native builds. | | ||
| | **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 15 gates. ~567 packages / 7 native builds. | | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the product-gate count.
scripts/ci/product-features.txt contains 16 gates, not 15. Update this count so the feature-set contract is accurate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 358 - 360, Update the Product row in the feature-set
comparison table to report 16 gates instead of 15, while leaving the other
package and native-build counts unchanged.
| PRODUCT_FEATURES="$(bash scripts/ci/product-features.sh)" | ||
|
|
||
| llvm_cov() { | ||
| bash scripts/ci-cancel-aware.sh cargo llvm-cov "$@" | ||
| # `clean` and `report` are cargo-llvm-cov subcommands that take no feature | ||
| # selection; passing --features to them is an error. | ||
| case "${1:-}" in | ||
| clean | report | show-env) | ||
| bash scripts/ci-cancel-aware.sh cargo llvm-cov "$@" | ||
| return | ||
| ;; | ||
| esac | ||
| bash scripts/ci-cancel-aware.sh cargo llvm-cov --features "${PRODUCT_FEATURES}" "$@" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 '^\[\[bin\]\]|required-features\s*=\s*\["bin-tools"\]' Cargo.toml
rg -n -C 4 'PRODUCT_FEATURES|llvm_cov.*--bins' scripts/ci/rust-coverage-changed.shRepository: tinyhumansai/openhuman
Length of output: 3856
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- scripts/ci/product-features.sh ---\n'
cat -n scripts/ci/product-features.sh
printf '\n--- relevant coverage script section ---\n'
sed -n '1,130p' scripts/ci/rust-coverage-changed.sh | cat -n
printf '\n--- feature declarations and bin requirements ---\n'
sed -n '1,110p' Cargo.toml | cat -n
printf '\n--- workflow references for coverage features ---\n'
rg -n -C 3 'coverage|rust-coverage-changed|bin-tools|PRODUCT_FEATURES|--bins' .github scripts/ci Cargo.tomlRepository: tinyhumansai/openhuman
Length of output: 251
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- scripts/ci/product-features.sh ---'
cat -n scripts/ci/product-features.sh
printf '%s\n' ''
printf '%s\n' '--- relevant coverage script section ---'
sed -n '1,130p' scripts/ci/rust-coverage-changed.sh | cat -n
printf '%s\n' ''
printf '%s\n' '--- feature declarations and bin requirements ---'
sed -n '1,110p' Cargo.toml | cat -n
printf '%s\n' ''
printf '%s\n' '--- workflow references for coverage features ---'
rg -n -C 3 'coverage|rust-coverage-changed|bin-tools|PRODUCT_FEATURES|--bins' .github scripts/ci Cargo.tomlRepository: tinyhumansai/openhuman
Length of output: 50378
Add bin-tools to coverage feature selection.
[[bin]] targets in Cargo.toml require bin-tools, but PRODUCT_FEATURES comes from scripts/ci/product-features.txt, whose source does not include bin-tools. Use a coverage feature set that includes the same bin-tools gate used by rsut-core-tests so llvm_cov --bins, --doc, and --test do not skip gated library tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci/rust-coverage-changed.sh` around lines 40 - 51, Update the
coverage feature setup around PRODUCT_FEATURES and llvm_cov to include the
bin-tools feature, matching the feature gate used by rsut-core-tests. Ensure
llvm_cov passes this augmented feature set for coverage commands such as --bins,
--doc, and --test while preserving the existing no-feature handling for clean,
report, and show-env.
What this change touches12 files, +623 -47 across 7 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["scripts/ci<br/>4 files +194 -19<br/>2 findings"]:::blocking
n1["scripts/lib<br/>1 file +138 -3"]:::changed
n2["root<br/>2 files +107 -19<br/>2 findings"]:::flagged
n3["scripts/__tests__<br/>1 file +116 -0"]:::changed
n4[".github/workflows<br/>2 files +49 -5"]:::changed
n5["scripts<br/>1 file +11 -1<br/>1 finding"]:::blocking
n6["docs<br/>1 file +8 -0"]:::changed
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
There was a problem hiding this comment.
The previously-blocking findings are resolved. Clearing the changes request.
$0.0107 · 89,494 in / 37,746 out · 67,660 cached (76%) · z-ai/glm-5.2
critique: $0.0054 · 23,064 in / 21,723 out · 18,086 cached (78%) · z-ai/glm-5.2
security: $0.0011 · 10,245 in / 3,915 out · 7,889 cached (77%) · z-ai/glm-5.2
tests: $0.0015 · 19,011 in / 4,578 out · 13,985 cached (74%) · z-ai/glm-5.2
description: $0.0021 · 20,292 in / 6,823 out · 14,511 cached (72%) · z-ai/glm-5.2
tinysweeper
Bot
added
the
priority: p3
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0183 · 127,792 in / 68,246 out · 98,775 cached (77%) · z-ai/glm-5.2
critique: $0.0096 · 53,170 in / 37,403 out · 41,847 cached (79%) · z-ai/glm-5.2
security: $0.0042 · 35,318 in / 15,331 out · 27,986 cached (79%) · z-ai/glm-5.2
tests: $0.0018 · 18,987 in / 5,548 out · 13,747 cached (72%) · z-ai/glm-5.2
description: $0.0027 · 20,317 in / 9,964 out · 15,195 cached (75%) · z-ai/glm-5.2
| # Strip comments and blank lines, then join with commas. Refuse to emit an | ||
| # empty list: a lane silently running with NO features would look green while | ||
| # covering nothing, which is the failure mode this whole guard exists to stop. | ||
| LIST="$(sed -e 's/#.*//' -e 's/[[:space:]]//g' "$FILE" | grep -v '^$' | paste -sd, -)" |
There was a problem hiding this comment.
Empty-list guard is unreachable: set -e + pipefail aborts before the check
With set -euo pipefail active, the assignment LIST=... runs a pipeline whose middle stage is grep -v '^$'. When product-features.txt contains only comments and blank lines, sed emits only empty lines, so grep -v '^$' finds zero matching lines and exits with status 1. Because pipefail is set, the entire pipeline inherits that non-zero status, and set -e then terminates the script immediately — before line 28 is ever evaluated. The intended guard at lines 28–31 (emit a helpful message, exit 2) is therefore unreachable in the exact scenario it was written to catch; the actual behaviour is a silent exit with code 1. This is also why the empty-list exit-2 path has no test: it cannot be exercised as written. Appending || true to the pipeline (or to the grep stage) would let LIST become the empty string so the explicit check can fire.
[RULE] set -e with pipefail makes the empty-list guard unreachable ·
| # x402_twit_sh_live) are silently SKIPPED and the run still exits 0 — the same | ||
| # trap `--features bin-tools` already guards for the `src/bin/` targets. | ||
| # Source of truth: scripts/ci/product-features.txt. | ||
| PRODUCT_FEATURES="$(bash "$REPO_ROOT/scripts/ci/product-features.sh")" |
There was a problem hiding this comment.
Guard against an empty PRODUCT_FEATURES before passing it to cargo
The new code captures product-features.sh output into PRODUCT_FEATURES but never checks whether the script succeeded. If product-features.sh exits non-zero (the prior review flagged an uncovered exit-2 path on an empty list) and the shell does not abort (the diff shows no set -e guard here), PRODUCT_FEATURES will be empty and the cargo invocation becomes --features ",bin-tools" — a malformed feature list that either causes a cargo parse error or, if cargo tolerates the leading comma, silently degrades to bin-tools alone. The latter is precisely the "silently SKIPPED, still exits 0" trap the comment says this change exists to prevent. The code should either fail fast on a non-zero exit, or conditionally build the feature list so an empty PRODUCT_FEATURES does not produce a stray leading comma.
[RULE] unhandled-error-paths ·
| | **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. ~356 packages / 5 native builds. | | ||
| | **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 15 gates. ~567 packages / 7 native builds. | |
There was a problem hiding this comment.
Gate counts in prose (9 / 15) don't match the table (5 ON / 11 ON)
The introductory text claims the Contributor set has "9 cheap gates" and the Product set has "15 gates", but the table below lists 12 gates total, of which only 5 are marked Contrib ON and 11 are marked Product ON. The numbers cannot be reconciled with the table as shown. Either the table is missing rows for gates that exist in default and/or product-features.txt, or the stated counts are wrong. A contributor reading "9 cheap gates" and then counting ON entries in the Contrib column will be misled either way. If the table is intended to be comprehensive, the counts should match; if it is not, that should be stated.
| | **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. ~356 packages / 5 native builds. | | |
| | **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 15 gates. ~567 packages / 7 native builds. | | |
| Make the table's ON-counts match the prose counts, or add the missing gate rows, or clarify that the table is not exhaustive. |
[RULE] N/A ·
| # measuring code that ships, and the diff-coverage gate would pass a PR whose | ||
| # changed lines were never compiled. Source of truth: | ||
| # scripts/ci/product-features.txt. | ||
| PRODUCT_FEATURES="$(bash scripts/ci/product-features.sh)" |
There was a problem hiding this comment.
Guard against empty product-features output before passing --features
The output of product-features.sh is captured into PRODUCT_FEATURES with no check that it is non-empty. If that script ever returns empty output (exit 0, no stdout), the downstream call becomes cargo llvm-cov --features "", which is equivalent to the default contributor feature set — precisely the situation the added comment warns against. In that case the diff-coverage gate would pass a PR whose changed lines in gated domains were never compiled, and there would be no loud failure to signal it. An explicit guard such as [ -n "$PRODUCT_FEATURES" ] after the assignment would make the failure audible.
| PRODUCT_FEATURES="$(bash scripts/ci/product-features.sh)" | |
| Guard against empty output, e.g. add `if [ -z "$PRODUCT_FEATURES" ]; then log "product-features.sh returned no features"; exit 1; fi` immediately after the assignment. |
**[RULE] ** ·
| # gated-off domains, so every CI lane that builds or tests "the product" now | ||
| # passes `--features "$(scripts/ci/product-features.sh)"`. If you add a lane, | ||
| # decide which of the two sets it is testing and say so. | ||
| default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging"] |
There was a problem hiding this comment.
Move medulla out of the contributor default or onto the product list
existing_code = default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging"]
This PR is the one that explicitly splits the contributor default from the product set. The comment block says the product set lives in scripts/ci/product-features.txt and is forwarded by the desktop shell. The prior review established that medulla is not in that product file. This PR keeps medulla in the contributor default while it is absent from the product set, so contributors and rust-analyzer typecheck and test medulla-gated code paths that the shipped binary never includes. The result is a coverage gap: CI lanes that run cargo test (default features) exercise medulla-gated logic, while the product lanes that pass --features "$(scripts/ci/product-features.sh)" do not, so medulla-gated code can ship untested. If medulla gates security-relevant logic, the product binary takes paths no contributor lane has validated.
[RULE] Prior finding still standing ·
tinysweeper
Bot
added
priority: p1
1 participant
Why
[features] defaultwas the product set, so every contributor paid for the whole product on every edit — web3's ethers/secp256k1 cohort,documents' zstd/bzip2 native builds, the cpal/hound/arboard/enigo/rdev stack behindvoice+inference,contacts' macOS objc2 cohort,crash-reporting's sentry tree,tui's ratatui. A barecargo check, and therefore rust-analyzer on every keystroke, resolved 540 packages and ran 7 native C/C++ builds.What
defaultis now the contributor set — 9 gates that cost almost nothing to compile, socargo checkstill typechecks nearly the whole tree.[features] default)scripts/ci/product-features.txt)Kernel floor is untouched at
305/282/4— this axis does not intersect theflowsprofile.This does not change what ships.
app/src-taurihas declaredopenhuman_corewithdefault-features = falsesince #1061 and never inheriteddefault— which is precisely why a forwarding guard had to exist at all.The guard rewrite is in this PR on purpose
check-feature-forwarding.mjsworked by diffing the shell's forwarding list against[features] default. That is a subset check, sound only whiledefaultwas the product set. Shrinkingdefaultmakes it weaker with every gate removed, and vacuous ifdefaultever empties — silently re-arming #4901, wherevoiceshipped missing for three minor versions across 56 users and ~93k Sentry events. Landing the flip without the rewrite would have been the same bug with a longer fuse.The product set is explicit now, and the guard asserts three things:
missing; a gate the shell grew that the product never claimed fails onunexpected. Equality cannot pass vacuously.defaultgate is forwarded or allow-listed — the original check, retained because it still catches a contributor-set gate nobody shipped.Two parsers read that file: the guard, and
product-features.sh, which the CI lanes use to build--features. A test asserts they agree — if they drifted, CI would compile a different set than the guard checks. 27 self-tests pass.The fallout, and how it is handled
A lane that relied on default features silently stops covering the product. Every lane that builds or tests the product now passes
--features "$(bash scripts/ci/product-features.sh)": core clippy, the unit lane, the changed-files coverage lane, andscripts/test-rust-with-mock.sh.Clippy runs twice, once per set. A lint that only fires with the gates off — an import left unused once a domain is compiled out — is invisible to the product lane and would be red on every contributor's machine while CI stayed green. The contributor graph is 353 packages against the product's 540, so it is cheap.
Four
tests/*.rstargets name symbols that only exist behindvoice,web3,inferenceorcrash-reporting, so they declarerequired-features. Without it a barecargo testfails to compile, in files the contributor did not touch. With it cargo skips them — the same silent-skip trap--binswithoutbin-toolsalready had, which is why the lane comments spell it out:json_rpc_e2ealone is >12k lines of RPC contract coverage.tuilands in neither set (default-OFF and deliberately not forwarded), so nothing in CI would compile it at all. The feature-gate-smoke lane now checks it explicitly; a future gate in that position needs the same treatment.Reviewer notes
required-featuresis the blunt instrument: it costs the whole target when a gate is off, where CI: run the full gates-off test suite (blocked on task_local stack overflow) #5021's per-symbol#[cfg]cleanup would keep the ungated tests in those files running. Taken deliberately for these four and noted inCargo.toml— push back if you would rather wait for CI: run the full gates-off test suite (blocked on task_local stack overflow) #5021.docs/library-minimal-recipe.mdgets a superseded-note rather than a rewrite; it is a measurement record from a past session and itsdefaultlist was already stale (it namesdesktop-automation, which no longer exists).Verification
Summary by CodeRabbit
Enhancements
Documentation
Tests