Skip to content

feat(core): gate the desktop-automation cluster behind a default-ON feature (#5049) - #5061

Merged
senamakel merged 17 commits into
tinyhumansai:mainfrom
YellowSnnowmann:feat/desktop-automation-gate-5049
Jul 21, 2026
Merged

feat(core): gate the desktop-automation cluster behind a default-ON feature (#5049)#5061
senamakel merged 17 commits into
tinyhumansai:mainfrom
YellowSnnowmann:feat/desktop-automation-gate-5049

Conversation

@YellowSnnowmann

@YellowSnnowmann YellowSnnowmann commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a default-ON desktop-automation Cargo feature that compiles out the ~24.5k-LOC desktop-automation cluster — accessibility, screen_intelligence, autocomplete, desktop_companion, and the computer agent-tool family — when disabled.
  • Uses the established facade + stub.rs pattern (voice/web3) and the skills type carve-out rule: inert type modules stay ungated so enabled/disabled builds share one definition.
  • Wires the runtime DomainSet::desktop_automation axis (feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796); the three cluster controllers unregister and the 15 screen_intelligence_* + 4 computer agent tools become absent (not error-degraded) when off.
  • Sheds the exclusive uiautomation dependency (proven via cargo tree -i); forwarded to the desktop shell.
  • With the gate on, the build is byte-identical to today (Phase 1 = gate only; crate extraction is a filed follow-up).

Problem

None of this cluster belongs in a headless or embedded core — it is desktop-only by nature and the second-largest ungated family after channels. Keeping it unconditional inflates the embedded openhuman_core binary and RAM footprint (#5046) and keeps OS-automation dependencies in every build. The cluster is not a leaf: it is reached from always-on code and, critically, the accessibility module is consumed by the independently-toggled voice gate — so the gate must be built carefully to keep every feature combination compiling.

Solution

  • accessibility → facade + a rich, voice-aware stub. The stub reproduces the exact public surface reached by voice, text_input, app_state, and siblings, byte-for-byte; where the real module has a non-macOS arm it mirrors that arm (e.g. overlay/validate return Ok), so a voice-on / desktop-automation-off build behaves like a non-desktop platform. automate::run<B> is generic so the caller's &RealBackend binds without stubbing the backend trait.
  • autocomplete / screen_intelligence / desktop_companion → facade + minimal stubs (empty registration aggregators + the always-on caller surface; carved status types re-exported).
  • computer → leaf gate; per-element #[cfg] on the tool registrations (the mcp/web3 idiom).
  • Type carve-out: GlobeHotkey* moved into ungated accessibility::types; AutocompleteStatus / AccessibilityStatus kept ungated. Zero type duplication.
  • DomainSet: DomainGroup::DesktopAutomation + field + full/harness/none presets + allows arm + preset tests; the three aggregators retagged from Platform.
  • Both-ways tests in core/all_tests.rs + tools/ops_tests.rs pin registration/tool presence in both directions.
  • The enigo / arboard / rdev deps are not claimed as shed — enigo is shared with voice, rdev/arboard are voice-owned. Only uiautomation is exclusive.

Verified locally across all four combinations (on/on, off/on-voice, off/off) plus fmt, clippy, feature-forwarding, and the uiautomation shed proof.

Submission Checklist

  • Tests added or updated — desktop_automation_controllers_registered_when_feature_on / ..._absent_when_feature_off and screen_intelligence_tools_absent_when_feature_off (asserts the tools are absent, not degraded).
  • Diff coverage ≥ 80% — the both-ways gate tests cover the changed registration/gating lines; the stub bodies are exercised by the disabled-build compile + tests.
  • Coverage matrix updated — N/A: compile-time gate over existing behaviour; no new user-facing feature row.
  • All affected feature IDs listed — N/A.
  • No new external network dependencies — none; uiautomation is made optional, not added.
  • Manual smoke checklist updated — N/A: gate-on build is byte-identical to the shipped app.
  • Linked issue closed via Closes #NNNN/A: this is Phase 1 (the gate). Phase 2 (extraction into its own crate) is a separate follow-up; see ## Related.

Impact

  • Build/binary: slim/headless builds can now drop the desktop-automation cluster and the uiautomation dependency. The shipped desktop app is unchanged (default-ON, forwarded to the shell).
  • Compatibility: no wire-contract or controller-schema change with the gate on. With the gate off, the cluster's /rpc methods become unknown-method and its tools are absent by design.

Related

Changes since initial submission (review-driven)

  • Slim-build clippy fix (6544a6b1b) — gate the PermissionState import in globe.rs to macOS; the GlobeHotkey* type carve-out left it used only by macOS code, so it was an unused import (Linux -D warnings).
  • Disabled-build test gating (299e9ec65, CodeRabbit surfaced a real bug) — six pre-existing tests hard-asserted the now-gated surface and were not #[cfg]-gated in lockstep (the AGENTS.md testing gotcha; CI's cargo check-only smoke lane never caught them): the DESKTOP_ALWAYS_ON / DESKTOP_DEFAULT_OFF test consts, all_tools_includes_computer_control_when_enabled, and three core:: tests (cli/jsonrpc/legacy_aliases). Disabled-build core:: is now green.
  • Diagnostics on disabled paths (299e9ec65, CodeRabbit) — grep-friendly debug! on the screen-intelligence RPC/CLI + autocomplete-start rejections and the accessibility disabled actions (no user text/PII).
  • Voice mic path fix (0b76e1b05, Codex — real bug) — the accessibility stub returned Unknown mic permission, which broke voice dictation preemptively in a voice-on / desktop-automation-off build. The stub now returns Granted, matching the real Linux/non-macOS cpal arm.

Pre-existing, out of scope: three knowledge_* disabled-build tool tests fail because they assert skills/flows-gated tools ungated — unrelated to this gate; they fail on main too (CI never runs the disabled-build test suite).
⚠️ This PR also contains an unrelated #[cfg(test)]-only event-bus fix cherry-picked from #5063 — see the pinned "Note for reviewers" comment. It self-drops on rebase once #5063 merges.


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: feat/desktop-automation-gate-5049

Summary by CodeRabbit

  • New Features

    • Added a desktop automation feature covering autocomplete, screen intelligence, accessibility, desktop companion, and computer-control capabilities.
    • Desktop automation is enabled by default in standard builds.
    • Slim or headless builds can omit desktop automation while retaining compatible interfaces.
  • Bug Fixes

    • Disabled desktop automation now reports clear unavailable or unsupported statuses instead of attempting inactive operations.
    • Tool and controller availability now correctly follows the selected build configuration.

…eature (tinyhumansai#5049)

Add a default-ON `desktop-automation` Cargo feature compiling out the
accessibility / screen_intelligence / autocomplete / desktop_companion /
computer cluster (~24.5k LOC) when off, via the voice/web3 facade+stub pattern
and the skills type carve-out (inert types stay ungated; stubs re-export the one
real definition). The accessibility stub is voice-aware: it mirrors the non-macOS
real arms so a `voice`-on / `desktop-automation`-off build behaves like a
non-desktop platform.

Sheds the exclusive `uiautomation` dependency (proven via `cargo tree -i`). Wires
the `DomainSet::desktop_automation` runtime axis and retags the three cluster
controllers; the 15 `screen_intelligence_*` and 4 `computer` tools become absent
(not error-degraded) when off. Both-ways gate tests pin both directions. Forwarded
to the desktop shell. With the gate on the build is byte-identical.
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds a desktop-automation feature, gates automation domains and tools, introduces disabled-build stubs, adds runtime domain filtering, and updates controller, CLI, registry, and CI tests for feature-enabled and feature-disabled builds.

Changes

Desktop automation feature and runtime gating

Layer / File(s) Summary
Feature and domain wiring
Cargo.toml, app/src-tauri/Cargo.toml, src/core/all.rs, src/core/runtime/builder.rs, src/core/legacy_aliases.rs, src/core/all_tests.rs
Adds the feature and Windows dependency gate, forwards the feature to the application, introduces DomainGroup::DesktopAutomation, and wires controller and DomainSet filtering.
Disabled domain facades
src/openhuman/accessibility/*, src/openhuman/autocomplete/*, src/openhuman/desktop_companion/*, src/openhuman/screen_intelligence/*
Gates automation implementations, preserves shared types, and provides inert APIs for builds without desktop automation.
Tool registry gating
src/openhuman/tools/*
Conditionally compiles desktop automation tools and classifies them under DomainGroup::DesktopAutomation, with matching registry expectations.
Feature-gate validation
src/core/autocomplete_cli_adapter.rs, src/core/cli_tests.rs, src/core/jsonrpc_tests.rs, .github/workflows/ci-lite.yml
Gates autocomplete CLI and validation tests, adds a disabled-build CLI response, and updates CI feature-gate detection.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

Suggested labels: feature, rust-core

Suggested reviewers: m3ga-mind

Poem

I’m a bunny with a feature flag,
Gating tools in a tidy zigzag.
Stubs nap when automation’s away,
While full builds hop into play.
Cargo carrots roll in line—
Desktop magic now compiles fine!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: gating the desktop-automation cluster behind a default-on feature.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

…inyhumansai#5049)

The `GlobeHotkey*` type carve-out left `PermissionState` (the type) named only by
the macOS listener's permission check; the non-macOS stubs read
`detect_permissions().input_monitoring` without naming the type. Gate the import
to macOS so the Linux/Windows build is warning-clean under `cargo clippy -D warnings`
(the core-crate clippy CI lane, which was red on this).
…urface (tinyhumansai#5049)

The desktop-automation gate unregisters the autocomplete / screen_intelligence
controllers when off, but three pre-existing core tests hard-asserted that surface
and were not #[cfg]-gated in lockstep, so `cargo test --lib --no-default-features
--features tokenjuice-treesitter core::` failed (the testing gotcha AGENTS.md calls
out; the CI smoke lane runs `cargo check` only and never compiled these tests).

- legacy_aliases::is_compiled_out_method now also treats autocomplete_ /
  screen_intelligence_ / companion_ methods as compiled-out when the feature is
  off (composed with the existing mcp handling), so the frontend-catalog drift
  test keeps asserting on everything else.
- grouped_schemas_contains_migrated_namespaces gates its `autocomplete` assertion.
- invoke_autocomplete_status_rejects_unknown_param is gated on the feature.

Disabled-build core:: suite now 656 passed / 0 failed (was 654 / 3). Default build
unchanged: all three run + pass; clippy -D warnings clean.
@YellowSnnowmann
YellowSnnowmann marked this pull request as ready for review July 20, 2026 11:05
@YellowSnnowmann
YellowSnnowmann requested a review from a team July 20, 2026 11:05

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30a4476355

ℹ️ 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".

Comment thread src/openhuman/accessibility/stub.rs Outdated
@coderabbitai coderabbitai Bot added feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Jul 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@src/openhuman/accessibility/stub.rs`:
- Around line 27-229: Add namespaced debug!/trace! diagnostics to disabled
accessibility flows in src/openhuman/accessibility/stub.rs (lines 27-229),
covering entry, disabled branches, state changes, returned errors, and outcomes
for validate_focused_target, focused-text and paste functions,
globe_listener_start/poll/stop, permission functions, automation-state
functions, and automate::run; do not log user text or PII. Also add equivalent
grep-friendly diagnostics in src/openhuman/autocomplete/stub.rs (lines 21-79)
for omitted registration and disabled engine status/start/stop behavior,
including relevant entry/exit and error or state results.

In `@src/openhuman/screen_intelligence/stub.rs`:
- Around line 77-89: Add grep-friendly diagnostic logging before each disabled
rejection: log the rejected accessibility_capture_image_ref RPC in
src/openhuman/screen_intelligence/stub.rs lines 77-89, the rejected
screen-intelligence CLI command in src/openhuman/screen_intelligence/stub.rs
lines 122-124, and the rejected autocomplete start command in
src/core/autocomplete_cli_adapter.rs lines 99-113. Preserve each existing
disabled outcome and use the project’s established logging mechanism.

In `@src/openhuman/tools/ops_tests.rs`:
- Around line 2168-2201: Update the DESKTOP_ALWAYS_ON expectations in the
desktop_tools_are_registered test so screen_intelligence_status and
screen_intelligence_capture_now are included only under the desktop-automation
feature, matching the existing per-entry cfg gating and feature-off absence
contract.
🪄 Autofix (Beta)

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

Run ID: 38fd8d38-df19-4776-a883-f7fa574108f4

📥 Commits

Reviewing files that changed from the base of the PR and between d13a559 and 30a4476.

📒 Files selected for processing (28)
  • Cargo.toml
  • app/src-tauri/Cargo.toml
  • src/core/all.rs
  • src/core/all_tests.rs
  • src/core/autocomplete_cli_adapter.rs
  • src/core/cli_tests.rs
  • src/core/jsonrpc_tests.rs
  • src/core/legacy_aliases.rs
  • src/core/runtime/builder.rs
  • src/openhuman/accessibility/globe.rs
  • src/openhuman/accessibility/mod.rs
  • src/openhuman/accessibility/stub.rs
  • src/openhuman/accessibility/types.rs
  • src/openhuman/autocomplete/core/engine.rs
  • src/openhuman/autocomplete/core/mod.rs
  • src/openhuman/autocomplete/core/text.rs
  • src/openhuman/autocomplete/mod.rs
  • src/openhuman/autocomplete/stub.rs
  • src/openhuman/autocomplete/types.rs
  • src/openhuman/desktop_companion/mod.rs
  • src/openhuman/desktop_companion/stub.rs
  • src/openhuman/screen_intelligence/mod.rs
  • src/openhuman/screen_intelligence/stub.rs
  • src/openhuman/tools/impl/mod.rs
  • src/openhuman/tools/impl/system/mod.rs
  • src/openhuman/tools/mod.rs
  • src/openhuman/tools/ops.rs
  • src/openhuman/tools/ops_tests.rs

Comment thread src/openhuman/accessibility/stub.rs
Comment thread src/openhuman/screen_intelligence/stub.rs
Comment thread src/openhuman/tools/ops_tests.rs
…bled paths (tinyhumansai#5049)

From CodeRabbit review + a disabled-build test audit:

- Gate the desktop-automation entries in the DESKTOP_ALWAYS_ON / DESKTOP_DEFAULT_OFF
  test constants and the all_tools_includes_computer_control_when_enabled test with
  #[cfg(feature = "desktop-automation")] (matching the existing mcp gating). Without
  this the disabled-build tools::ops::tests suite expected screen_intelligence_* /
  mouse / keyboard tools that the gate compiles out — the AGENTS.md testing gotcha,
  invisible to CI's cargo-check-only smoke lane.
- Add grep-friendly debug diagnostics to the disabled entry points: the
  screen_intelligence capture-image-ref RPC + CLI rejections, the autocomplete-start
  CLI rejection, and the accessibility paste / globe-start / automate::run disabled
  actions (never logging user text or PII).

Disabled-build tools::ops::tests now pass except three pre-existing knowledge_*
failures (skills/flows-gated tools asserted ungated — unrelated to this gate; they
fail on main too, since CI never runs the disabled-build test suite).
…lake

Under cfg(test) the crate's ~12.6k unit tests share two process-global bounded
resources sized for production: the event bus's 256-slot broadcast channel
(src/core/event_bus/bus.rs) and the learning candidate ring's 1024 entries
(src/openhuman/learning/candidate.rs, whose drainer does not run in the unit
suite). Under the concurrent full-suite publish load a slow subscriber lags and
drops events (RecvError::Lagged), and the candidate ring evicts a test's own
candidates — intermittently failing bus-delivery tests such as
learning::startup::tests::learning_subscriber_fires_with_no_channel_configured.

Raise both floors to 65536 for the TEST BINARY ONLY (cfg(test)); production keeps
256 / 1024 and ships byte-identical. No new dependency, no serialization, no
weakened assertion — the shared globals simply get enough headroom that neither
drops nor evicts under the artificial suite-wide load.
…s with DA off (tinyhumansai#5049)

Codex review: in a slim build with `voice` enabled but `desktop-automation`
disabled, `voice::audio_capture::record_on_thread` reads mic permission through
`accessibility::detect_microphone_permission` and treats `Unknown` as
"request-then-recheck, else fail". The stub returned `Unknown` and
`request_microphone_access` is a no-op, so dictation failed before ever opening
the mic — even where the mic is usable.

The real Linux / non-macOS implementation cpal-probes and returns `Granted` when
an input device exists. Match that permissive non-desktop arm in the stub
(`detect_microphone_permission` + the `microphone` field of `detect_permissions`
now return `Granted`) so voice proceeds and any real capture error surfaces from
cpal, instead of a preemptive permission failure.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 20, 2026
@YellowSnnowmann

YellowSnnowmann commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

📌 Note for reviewers — the event-bus change here is an unrelated, self-healing flake fix

This PR carries a small test-only change that is not part of its objective:

  • src/openhuman/learning/startup.rs (test module)
  • src/openhuman/web_chat/event_bus.rs (test module)

They fix two pre-existing full-suite flakes — learning::startup::tests::learning_subscriber_fires_* and web_chat::event_bus::tests::automation_halt_subscriber_handle_publishes_correct_payload.

Root cause (corrected): not a capacity problem. Both tests asserted against the process-global broadcast bus, so under the ~12,600-test parallel suite they received events published by other concurrent tests — cross-test interference, which no channel-capacity bump can fix. (An earlier capacity-floor attempt was pushed and then reverted here once CI proved it ineffective.)

Fix — real isolation:

  • learning::startup now publishes a DocumentCanonicalized event on a private EventBus::create(64) with a locally-subscribed EmailSignatureSubscriber, instead of the shared global bus. A synchronous register_email_signature_subscriber guard test covers the channel-independent registration path (fix(learning): email-signature subscriber + rebuild trigger silently depend on channels being configured #5003).
  • web_chat::event_bus drains foreign events from the shared web-channel bus and matches on event == "automation_halt" rather than taking whatever is first in the buffer.

Production ships byte-identical — the diff is confined to #[cfg(test)] code; no new deps, no weakened assertion.

Why it's in this PR: this PR changes Cargo.toml, which triggers the full coverage suite where the flake reproduces. #5063 (the canonical standalone fix) only gets scoped coverage and cannot self-verify — so the same commit is cherry-picked here so this PR's coverage lane can go green. It self-drops on rebase once #5063 merges to main.

Pre-existing CI red unrelated to this PR (not merge-blocking — not in pr-ci-gate)

…city floor

The earlier test-only capacity floor (5c49a96) did not fix the flake:
the two failing tests share the process-global broadcast bus, so under the
parallel suite they receive events published by *other* tests — a capacity
bump cannot fix cross-test interference. Replace it with real isolation.

- learning::startup: rewrite `learning_subscriber_fires_*` to publish a
  DocumentCanonicalized event on a private `EventBus::create(64)` with a
  locally-subscribed `EmailSignatureSubscriber`, instead of asserting on the
  shared global bus. Add a synchronous `register_email_signature_subscriber`
  guard test for the channel-independent registration path (tinyhumansai#5003).
- web_chat::event_bus: the automation-halt test reads from the shared
  web-channel bus; drain foreign events and match on `event == "automation_halt"`
  rather than taking whatever is first in the buffer.
- Revert the `#[cfg(test)]` capacity floors in event_bus/bus.rs and
  learning/candidate.rs back to origin/main — no longer needed.

@CodeGhost21 CodeGhost21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the desktop-automation gate. Overall this is high-quality, idiomatic work: the facade + stub.rs pattern is applied cleanly, the type carve-out (GlobeHotkey*accessibility::types) is correct with zero duplication, both-directions tests pin the gate, and the dependency-shed claim is disciplined (only uiautomation is exclusive). The detect_microphone_permission() → Granted and PermissionState import fixes are genuine catches.

I verified there are no callers of the macOS-only permission exports (detect_screen_recording_permission, etc.) outside the gated tree, so the stub omitting them is safe on a slim macOS build.

Approve-with-follow-up. The one thing worth pushing on before this pattern proliferates is a slim-build CI lane (see the Cargo.toml comment) — the rest are non-blocking nits filed inline.

Comment thread Cargo.toml
#
# NOTE: only `uiautomation` is exclusive and thus shed. `enigo` is shared with the
# `voice` domain; `rdev` / `arboard` are voice-owned — none of those are dropped here.
desktop-automation = ["dep:uiautomation"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Structural — highest concern) CI never compiles or tests the disabled build.

The safety of every stub↔real signature parity here rests entirely on a manual local --no-default-features build. The PR body itself notes CI's cargo check-only smoke lane never caught the six pre-existing test-gating bugs CodeRabbit surfaced, and that three knowledge_* disabled-build tests already fail on main undetected.

This gate multiplies the feature combinations that can silently rot. Strongly recommend a follow-up (or this PR) adding a slim cargo check --no-default-features --features <minimal> lane — otherwise the next contributor who adds an always-on caller of a gated symbol breaks the slim build with no signal. Good candidate to file alongside the Phase 2 extraction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and I think keeping the full slim test lane out of this PR is the right scope call. You're correct that the Rust Feature-Gate Smoke (gates off) lane only cargo checks the disabled build and never runs its test suite — which is exactly how the pre-existing gated-test breakage went unnoticed.

What this PR does add on that front: it extends the self-maintaining gated-test guard so the allowlist can't silently drift as the gate surface grows — desktop-automation is now in the guard regex and the new core/autocomplete_cli_adapter.rs is in EXPECTED (8e9ce8b). That's still a compile-time guard, not a disabled-build test run.

A dedicated slim cargo test --no-default-features --features <minimal> lane belongs with the Phase 2 extraction follow-up rather than expanding this PR — noting it here so it's tracked alongside that work.

/// (screen-recording / accessibility / input-monitoring) are unknown when the
/// gate is off; microphone mirrors `detect_microphone_permission` above so the
/// `voice`-on / `desktop-automation`-off build keeps a usable mic state.
pub fn detect_permissions() -> PermissionStatus {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Minor nit) Unknown vs the real non-macOS Unsupported.

The real non-macOS detect_permissions returns PermissionState::Unsupported for screen_recording / accessibility / input_monitoring, but this stub returns Unknown. Elsewhere the stub docs claim the disabled build "behaves like a non-desktop platform" — this slightly contradicts that.

I confirmed no always-on / voice caller branches on these three fields (only microphone is read outside the gated tree), so it's cosmetic. For exactness, Unsupported would better match the stated intent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 8be6f19. The disabled-build stub detect_permissions() now returns PermissionState::Unsupported for screen_recording / accessibility / input_monitoring, matching the real non-macOS detect_permissions (permissions.rs:267, pinned by detect_permissions_non_macos_reports_unsupported_for_desktop_perms) and the sibling struct that already used Unsupported. microphone stays Granted. Confirmed no always-on/voice caller branches on those three fields, and the disabled build (--no-default-features --features tokenjuice-treesitter) compiles clean.

Comment thread src/openhuman/learning/startup.rs Outdated
// channel runtime is ever constructed in this test.
let _ = crate::openhuman::memory::global::init(tmp.path().join("workspace"));
register_learning_subscribers(tmp.path().to_path_buf());
async fn learning_subscriber_fires_on_document_event() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Scope) Unrelated flaky-test fix riding along.

This isolated-bus refactor is a test-only flake fix, not gate work. Not blocking, but it inflates the review surface — ideally it lands on its own PR (the sibling web_chat/event_bus.rs change is at least disclosed as cherry-picked from #5063 and self-drops on rebase).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point on scope. I've kept the isolated-bus refactor here deliberately: it fixes a full-suite flake that this PR's own gates-off test + coverage lanes would otherwise hit intermittently, so dropping it would destabilize this branch's checks. It's test-only and low-risk. Happy to extract it to a standalone PR if you'd prefer the clean separation — say the word and I'll pull it out and rebase.

Comment thread src/openhuman/web_chat/event_bus.rs Outdated
// suite other tests publish to it too, so take THIS subscriber's next
// `automation_halt` event rather than whatever happens to be first in the
// buffer (which flaked as an "event name mismatch").
fn next_automation_halt(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Very minor) Narrow flake window remains.

In the "resumed" block, a concurrent test publishing an automation_halt event between the first drain and the resumed publish could be returned instead of the intended event. Much less likely than the bug this fixes, and test-only — noting for completeness.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tightened in 88f7b94. next_automation_halt now filters on the engaged payload (true for halted, false for resumed) in addition to the event name, so a concurrent test's halt/resume on the shared web-channel bus can no longer be returned in place of the intended event. The 6 web_chat::event_bus tests pass. (The helper is cherry-picked from #5063; the same tightening can land there on rebase.)

…A off

The disabled-build stub returned PermissionState::Unknown for
screen_recording / accessibility / input_monitoring, but the real
non-macOS detect_permissions reports these as Unsupported. Align the
stub so the desktop-automation-off build matches the documented
"behaves like a non-desktop platform" intent. Cosmetic: no always-on
or voice caller branches on these three fields (only microphone is read
outside the gated tree), which stays Granted.
The self-maintaining gated-test guard grepped only
(voice|media|web3|meet|mcp|skills|flows), so the new desktop-automation
gate's test files were invisible to it — the lane would stay green while
silently under-covering the gate as its tests grow. Add desktop-automation
to the regex and add the now-matched core/autocomplete_cli_adapter.rs to
the EXPECTED allowlist.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 21, 2026
next_automation_halt filtered only on the event name, but automation_halt
is emitted for both engaged=true (halted) and engaged=false (resumed). A
concurrent test publishing to the shared web-channel bus between the drain
and the resumed publish could be returned in place of the intended event.
Filter on the expected engaged payload so each phase takes only its own
event.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 21, 2026
…tion-gate-5049

# Conflicts:
#	src/openhuman/learning/startup.rs
#	src/openhuman/web_chat/event_bus.rs
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 21, 2026
YellowSnnowmann and others added 3 commits July 21, 2026 18:11
The tinyhumansai#5084 tui gate added `#[cfg(feature = "tui")]` guards, the optional
ratatui/crossterm deps, the forwarding allowlist entry, and the docs — but
never declared the `tui` feature in [features] or added it to `default`.
That left `feature = "tui"` an unknown cfg value, so `cargo clippy -p
openhuman -- -D warnings` failed with unexpected_cfgs (Rust Quality lane).
Declare `tui = ["dep:ratatui", "dep:crossterm"]` and add it to default,
matching the documented default-ON intent (already allowlisted in
check-feature-forwarding.mjs).
…ell self-test

The 'real shell manifest forwards every real core default' self-test asserted
every default-ON gate is forwarded to the desktop shell, ignoring the
INTENTIONALLY_NOT_FORWARDED allowlist that the CI checker respects. Once the
tui gate (tinyhumansai#5084) became default-ON — a terminal subcommand deliberately not
shipped in the Tauri shell — the two disagreed and Scripts Self-Tests failed
with 'core default gate not forwarded to the shell: tui'.

Move the allowlist into scripts/lib/feature-forwarding.mjs (one source of truth)
and have the self-test skip allowlisted gates, matching the checker. The lane is
path-gated so this was dormant on main; it surfaced here.
@senamakel
senamakel merged commit 4019a9a into tinyhumansai:main Jul 21, 2026
8 of 9 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e13c1806f

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +100 to +102
pub fn globe_listener_start() -> Result<GlobeHotkeyStatus, String> {
log::debug!("[accessibility] globe_listener_start disabled: desktop-automation off");
Ok(disabled_globe_status())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve Fn dictation when voice is kept

In a macOS slim build with voice enabled but desktop-automation disabled, the voice server still uses the Globe/Fn listener for its default Fn hotkey: voice::server::start_globe_hotkey_listener calls globe_listener_start() and treats !status.running as a startup error. This stub always returns a non-running status, so voice_server.auto_start or the standalone voice server fails before dictation can start in that feature combination. The Globe listener needs the same voice carve-out as the microphone path, or voice needs a working fallback when desktop automation is compiled out.

Useful? React with 👍 / 👎.

YellowSnnowmann added a commit to YellowSnnowmann/openhuman that referenced this pull request Jul 21, 2026
Reconcile dep-gating with main (now incl. tinyhumansai#5052/tinyhumansai#5060/tinyhumansai#5061/tinyhumansai#5062):
- Cargo.toml: union gating (optional axum/sentry/ppt-rs + http-server/
  inference/documents/crash-reporting features) with main's tower + tui deps
  and desktop-automation/tui features; default = union of both feature sets.
- app/src-tauri: shell forwards crash-reporting/http-server/desktop-automation
  (feature-forwarding gate passes; tui allowlisted).
- all_tests.rs: union of http-server + desktop-automation gate tests.
- types.rs: keep main's redirect_links security denylist entry (tinyhumansai#5052).
Cargo.lock + app/src-tauri/Cargo.lock resynced (--locked passes); cargo fmt,
feature-forwarding gate, and clippy -D warnings all clean locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants