feat(core): compile-time meet feature gate (#4800)#4915
Conversation
Defines the compile-time gate for the three Meet domains. Default-ON, so the desktop build stays byte-identical; slim builds opt out via --no-default-features. Unlike `voice`, this gate sheds no dependencies — the Meet domains have zero exclusive crates. `meet_agent::wav` is a hand-rolled RIFF writer written precisely so Meet never needed `hound`, which `voice` already owns and sheds. The gate's value is compile-time surface, not the dep tree; Cargo.lock is unchanged, which confirms it.
…yhumansai#4800) Applies one module pattern per domain, chosen by whether always-compiled code reaches a non-registration symbol: - meet -> leaf-gate; its only outside reference is the registry. - meet_agent -> facade + carve-out, no stub; nothing outside the Meet domain calls its gated submodules. - agent_meetings -> facade + stub; the heartbeat planner and two subscriber registrations reach in, so stub.rs supplies no-op equivalents and those callers need no cfg. The stub's no-ops are semantically exact rather than placeholders: a register_* that registers nothing is the intended disabled behaviour, and handle_calendar_meeting_candidate returning false means "Meet published no card", so the planner correctly falls through to its generic reminder. CARVE-OUT: meet_agent::wav stays ungated — desktop_companion (Platform, always-on) calls pack_pcm16le_mono_wav and needs the real implementation. It is dependency-free, so leaving it compiled costs nothing. Verified: gating it fails the disabled build loudly at desktop_companion/pipeline.rs with E0433. That failure must be fixed by reverting the cfg, never by stubbing the function — a stub would trade a compile error for green CI while silently corrupting companion STT. Recorded in the module doc.
Adds meet_controllers_registered_when_feature_on and its negative twin
meet_controllers_absent_when_feature_off, covering all three namespaces.
The negative half is the one that proves the gate removes anything — a
gate that never dropped a controller would still pass the positive test.
Also cfg's the group_for_namespace("meet") assert in group_mapping_smoke:
the helper is registry-derived, so with meet compiled out the namespace is
absent and the lookup correctly returns None. The neighbouring assert at
:765 needs no change — "meet" is in the *absent* list there, which holds
either way.
Note CI's smoke lane runs cargo check only and never compiles test code,
so a disabled-build test break is invisible to it; run the disabled test
lane locally after touching any gated surface.
The shell depends on the core with default-features = false and had no features key, so a default-ON gate would silently drop from the shipped app. Written as a multi-line array so the sibling gates in this epic append cleanly rather than conflicting on one line.
…umansai#4800) Records the three module patterns and the rule that picks between them, the wav carve-out and why stubbing it is dangerous, and the fact that the gate sheds no dependencies — so that last point isn't re-litigated by the next reader expecting a voice-style dep drop.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 882798ac63
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Google Meet call-join request validation (shell handles the webview) | ||
| // Google Meet call-join request validation (shell handles the webview). | ||
| // Gated behind the `meet` feature. | ||
| #[cfg(feature = "meet")] |
There was a problem hiding this comment.
When this cfg removes the Meet controllers in --no-default-features builds, the always-on about_app catalog still returns the meet.join_call and meet_agent.live_loop entries unconditionally from src/openhuman/about_app/catalog_data.rs, so openhuman.about_app_list/search/lookup advertises Intelligence > Calls even though the corresponding RPC methods are now method-not-found. The repo instructions describe about_app as the user-facing capability catalog to update when features are removed, so please cfg or filter those catalog entries alongside this gate.
Useful? React with 👍 / 👎.
| ); | ||
| // Agent meetings — backend-delegated Meet bot via Socket.IO | ||
| // (gated with meet). | ||
| #[cfg(feature = "meet")] |
There was a problem hiding this comment.
This gates the agent_meetings controllers, but the backend Socket.IO Meet path is still compiled and active when meet is off: src/openhuman/socket/event_handlers.rs continues accepting bot:* events and publishing BackendMeet* events, and src/core/socketio.rs still broadcasts those as agent_meetings:* to renderer sockets. In a slim build connected to the backend while a stale or misrouted bot session emits transcripts/replies, the supposedly disabled Meet feature still processes and exposes meeting data; please guard those bot handlers/bridges with the same feature gate.
Useful? React with 👍 / 👎.
…ure-gate # Conflicts: # AGENTS.md # Cargo.toml # app/src-tauri/Cargo.toml
2 participants
Summary
meetfeature (default-ON) gatingmeet(562 LOC) +meet_agent(5,298) +agent_meetings(8,909) — ≈14.7k LOC — composing with the runtimeDomainSet::meetflag from feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796.cargo build --no-default-features --features "<gates without meet>".meet_agent::wav::pack_pcm16le_mono_wavstays ungated because the always-ondesktop_companionSTT path calls it. Stubbing it would compile clean and silently feed a 0-byte WAV to cloud STT.meet = []) — the dep shed was pre-paid, see below.meet) + facade/carve-out no-stub (meet_agent) + facade+stub (agent_meetings).Problem
#4795 wants one compile-time feature gate per subsystem family; #4796 shipped the runtime
DomainSetaxis. This PR adds the compile-time half for the Meet family.Three of the epic's DoD bullets are inapplicable here. Stating them with evidence rather than leaving boxes that imply missing work:
DomainSetflag onCoreBuilder" — already landed by feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796 (PR feat(core): runtime DomainSet composition axis — group-tagged registry + ambient filter (#4796) #4808) for every gate. This PR is compile-time only.meet = [](empty dep list) is the proof. This is not an oversight:meet_agent::wavis a hand-rolled 79-LOC RIFF writer with zerousestatements, written precisely so Meet never neededhound— the crate thevoicegate already sheds. The dependency shed was pre-paid.tools/ops.rssays so verbatim. There is nothing to omit.The real risk in this family is not the gate — it is the carve-out below.
Solution
Cargo.toml:meet = []added todefault— dep list intentionally empty, commented so nobody "fixes" it later.meet_agent::wav::pack_pcm16le_mono_wavstays compiled in all builds: the always-ondesktop_companiondomain (Platform group) calls it to pack PCM for STT. This is verified, not assumed — deliberately gating it produceserror[E0433]: cannot find 'wav' in 'meet_agent', then reverted. So the warning in the module doc is tested, not aspirational. Note the failure mode this avoids: a stub here would compile clean and silently hand a 0-byte WAV to cloud STT — a silent-corruption bug, not a build error.meet(registration sites want absence, so no stub).meet_agent— the module facade stays,wavstays compiled, the live STT/LLM/TTS loop is gated.agent_meetings—stub.rsmirrors the always-on caller surface with disabled-error / empty bodies, socore/all.rsneeds no call-sitecfg.app/src-tauri/Cargo.toml):openhuman_coreisdefault-features = false, someetis added explicitly to keep the domain in the shipped desktop app. Verified viacargo tree -e features -i openhuman.rust-feature-gate-smokelane is a deny-by-default allow-list (--no-default-features --features tokenjuice-treesitter), so a new default-ON gate is excluded automatically and the disabled combination is already covered.AGENTS.md): gate-table row + thewavcarve-out and its inverted dependency.When off: the
meet/meet_agent/agent_meetingsnamespaces are absent from/schemaand their methods return a clean JSON-RPC unknown-method (-32000);desktop_companionSTT is unaffected.Submission Checklist
core/all_tests.rs; thewavcarve-out verified by deliberately gating it and observing the compile error.#[cfg]attrs, a Cargo feature, a stub facade, docs);diff-coverreports no coverable lines. The directional gate tests cover the on/off behaviour. — Diff coverage ≥ 80%## Relatedmeet = [].docs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## RelatedsectionImpact
meeton and the build is byte-identical.desktop_companionSTT is unaffected in both configs by design (thewavcarve-out).Related
DomainSetseam (PR feat(core): runtime DomainSet composition axis — group-tagged registry + ambient filter (#4796) #4808); follows the feat(core): feature gate — voice #4803 voice pathfinder and feat(core): feature gate — media generation #4804 media leaf gate (PR feat(core): compile-time media feature gate (media_generation + image) (#4804) #4840). Sibling gates: feat(core): feature gate — flows/workflows #4797 (flows), feat(core): feature gate — skills #4798 (skills), feat(core): feature gate — MCP #4799 (mcp), feat(core): feature gate — web3 #4802 (web3, PR feat(web3): compile-time web3 feature gate for wallet/web3/x402 (#4802) #4855).wavout of the gated tree.meet_agent::wavis an inverted dependency — an always-on domain (desktop_companion) reaching into a gated module. It should live somewhere neutral; the carve-out is the correct fix for this PR, not the end state.startup.rsregisters Meet subscribers unconditionally whilejsonrpc.rsgates them onplan.meet— a pre-existing feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796 runtime divergence, out of scope here.voiceandtokenjuice-treesitterare currently not forwarded inapp/src-tauri/Cargo.toml(whoseopenhuman_coredep isdefault-features = false), so those two domains are missing from the shipped desktop app today. Pre-existing regression, out of scope here — being fixed epic-level.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/4800-meet-feature-gate882798ac63f98a283d04ff47042212fcb7f41c66(5 commits, 9 files, +186/-4)Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo fmtclean · enabledcargo check0 errors · disabledcargo check --no-default-features --features tokenjuice-treesitter0 errors · clippy clean in both directions.app/src-tauri/Cargo.toml(one dep line), no Tauri Rust source touched — forwarding verified viacargo tree -e features -i openhuman— Tauri fmt/check (if changed)Boot smoke (both directions, 2 runs for repeatability) — both configs built, linked, booted, and served
/rpc:/schema.-32000); the process survives.Validation Blocked
command:cargo test --no-default-featureserror:2 pre-existing failures onmaintoday —group_mapping_smokeandharness_excludes_gated_namespaces, both asserting thevoicenamespace uncfg'd (fallout from the merged voice gate feat(core): feature gate — voice #4803). CI cannot see them: the feature-gate smoke lane runscargo check, which never compiles test code.impact:Not introduced by this PR and not fixed by it. This PR cfg'd its own asserts and left voice's alone; the voice asserts are a separate, epic-level fix that is pending. Scoped enabled-config tests and both-directioncargo checkare green.Behavior Changes
meetcompile-time gate. Default build unchanged.Parity Contract
/rpc); gate OFF yields absence at every registration site. Thedesktop_companionSTT path keeps a realwavimplementation in both configs — the carve-out is enforced by the compiler (verified by deliberately gating it and observingE0433), not by convention, specifically so it cannot silently degrade to a 0-byte WAV. Composes with, and does not replace, the runtimeDomainSet::meetguard from feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796.Duplicate / Superseded PR Handling