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: 50 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".
…ure-gate # Conflicts: # AGENTS.md # Cargo.toml # app/src-tauri/Cargo.toml
…ure-gate # Conflicts: # Cargo.toml # app/src-tauri/Cargo.toml
|
Maintainer review — addressing the two Codex bot findings (I made no code changes; assessment only): ① "Hide disabled Meet capabilities from about_app" — declining (false positive against convention). ② "Gate the backend Meet socket path as well" — valid; the PR's slim-build claim is overstated. This is a real gap, but it sits at the same seam as the PR's own documented out-of-scope note (the startup.rs/#4796 runtime-divergence follow-up). Recommended resolution for the author: either (a) add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Approving — the gate is correct and CI is fully green (14 pass / 4 skipping, including Rust Quality, Rust Feature-Gate Smoke (gates off), and both Core and Tauri coverage lanes). Frontend Checks correctly skips as out-of-changed-area for a Rust-only change.
Verified:
- Facade+stub follows the established
voice/web3pathfinder pattern;agent_meetings/stub.rsmirrors the always-on caller surface, socore/all.rsneeds no call-sitecfg. - Default build is byte-identical; both directions covered by registered-when-on / absent-when-off tests.
- Swept for dangling refs: with
meetoff, the only Meet code still compiling ismeet_agent::wav(dependency-free) plus the stub. No uncovered external references to gated symbols.
One accepted caveat, tracked rather than blocking — flagging it explicitly so the record is accurate:
The PR body states the slim build has "no Meet bot Socket.IO path" under Performance / security. That claim is not delivered as written. The backend→renderer Meet relay stays ungated and compiled with meet off:
src/core/socketio.rs— theagent_meetingsbridge (io_agent_meetingsat :636,agent_meetings:joinedemit at :1181) has nocfg(feature = "meet")guard anywhere in the file.src/openhuman/channels/socket/event_handlers.rs— thebot:*handlers likewise carry no meet gate.
This is not a default-build regression and not a correctness defect — the gate does what it says for compile-time surface, and this is consistent with the PR's existing out-of-scope note. But the attack-surface half of that claim does not hold today. Approving on the basis that the socket relay is scoped as a follow-up under #4796 (runtime gating).
Requested follow-up (non-blocking): either soften the security bullet in the PR body to match what ships, or open a #4796 child to gate socketio.rs / event_handlers.rs. Worth doing before anyone relies on the slim build for attack-surface reduction.
Credit where due: Codex flagged this; it is a genuine finding and was verified independently against the PR head.
Resolve sibling-gate conflicts with tinyhumansai#4915 (meet gate, tinyhumansai#4800) by union: - Cargo.toml: keep both `meet` and `mcp` in default + both feature blocks - app/src-tauri/Cargo.toml: forward both `meet` and `mcp` - AGENTS.md: keep both gate-table rows and both gate sections - src/core/all_tests.rs: keep all four both-direction gate tests
…umansai#4915) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
3 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