refactor: move inline cfg-gated platform imports to top-level use statements by PranavAgarkar07 · Pull Request #1564 · tinyhumansai/openhuman

PranavAgarkar07 · 2026-05-12T15:09:40Z

Summary

Move inline use statements behind platform cfg guards (#[cfg(unix)], #[cfg(windows)]) to the top of their respective files, following the Rust convention of top-level imports.

Changes

File	Import Moved	Reason
`src/core/auth.rs`	`std::os::unix::fs::OpenOptionsExt`	Used in `write_token_file()` behind `#[cfg(unix)]`
`src/openhuman/update/core.rs`	`std::os::unix::fs::PermissionsExt`	Used in `stage_update()` behind `#[cfg(unix)]`
`src/openhuman/service/windows.rs`	`std::os::windows::process::CommandExt`	Used in `is_task_exists_windows()` (file is already windows-only)
`src/openhuman/service/common.rs`	`std::os::windows::process::CommandExt`	Used in `no_window()` behind `#[cfg(windows)]`

Closes #684

Summary by CodeRabbit

Refactor
- Reorganized platform-specific imports and conditional compilation so Unix- and Windows-only behaviors are gated at the module level, improving cross-platform build correctness and maintainability.

…tements Move inline statements behind and guards to the top of their respective files, following Rust convention of top-level imports. Changes: - src/core/auth.rs: Move to top with #[cfg(unix)] - src/openhuman/update/core.rs: Move to top with #[cfg(unix)] - src/openhuman/service/windows.rs: Move to top (file is windows-only) - src/openhuman/service/common.rs: Move to top with #[cfg(windows)] Closes tinyhumansai#684

coderabbitai · 2026-05-12T15:10:42Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 14b461c0-f3af-4e0d-9229-8def6d1b9a39

📥 Commits

Reviewing files that changed from the base of the PR and between 4db8741 and 7728801.

📒 Files selected for processing (3)

src/core/auth.rs
src/openhuman/service/windows.rs
src/openhuman/update/core.rs

✅ Files skipped from review due to trivial changes (1)

src/core/auth.rs

🚧 Files skipped from review as they are similar to previous changes (1)

src/openhuman/service/windows.rs

📝 Walkthrough

Walkthrough

Consolidates platform-only extension-trait imports to module-level #[cfg(...)] blocks in auth, update, and service modules, and ensures Windows spawned commands set the CREATE_NO_WINDOW flag before execution.

Changes

Import consolidation and Windows no-window flag

Layer / File(s)	Summary
Unix imports consolidation in auth `src/core/auth.rs`	`OpenOptionsExt` and `PermissionsExt` moved to a module-level `#[cfg(unix)]` import; local in-block imports removed from `write_token_file` and the `token_file_has_owner_only_permissions` test.
Windows CommandExt consolidation and no-window flag `src/openhuman/service/common.rs`, `src/openhuman/service/windows.rs`	`std::os::windows::process::CommandExt` imported at module level behind `#[cfg(windows)]`; `no_window` uses that import, and `is_task_exists_windows` sets `creation_flags(0x0800_0000)` (CREATE_NO_WINDOW) before reading process output.
Unix imports consolidation in update `src/openhuman/update/core.rs`	`PermissionsExt` moved to a module-level `#[cfg(unix)]` import; executable-permission staging still sets mode `0o755`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

tinyhumansai/openhuman#1498: Modifies the same Windows process-spawn sites and CREATE_NO_WINDOW usage.

Suggested reviewers

senamakel

Poem

🐰 A tidy hop, imports climb with grace,

Windows whispers "no window" in place,
Unix modes march to a single line,
Small changes, neat, everything's fine,
🎩✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements the primary coding requirement to move inline imports to top-level with cfg guards, but does not address the issue's scope items for logs/diagnostics and tests.	Add logging/diagnostics for debugging and include tests validating the refactored behavior, or clarify why these were deferred.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the core refactoring: moving inline cfg-gated imports to top-level use statements across multiple files.
Out of Scope Changes check	✅ Passed	All changes are directly related to moving inline platform-specific imports to top-level use statements with appropriate cfg guards.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/auth.rs (1)
240-250: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing PermissionsExt import causes test compilation failure.

Line 247 calls .mode() on std::fs::Permissions, which requires the std::os::unix::fs::PermissionsExt trait. This import is missing from the file, preventing compilation of the test.

Add the missing import with the existing Unix-specific imports:
 #[cfg(unix)]
 use std::os::unix::fs::OpenOptionsExt as _;
+#[cfg(unix)]
+use std::os::unix::fs::PermissionsExt as _;
🤖 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 `@src/core/auth.rs` around lines 240 - 250, The test
token_file_has_owner_only_permissions calls .mode() on std::fs::Permissions but
is missing the Unix-specific trait; add use std::os::unix::fs::PermissionsExt to
the Unix-only imports in this file (near the #[cfg(unix)] tests) so the .mode()
method on Permissions resolves and the test compiles. Ensure the import is gated
by #[cfg(unix)] alongside the token_file_has_owner_only_permissions test and
related Unix-only imports.

🤖 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.

Outside diff comments:
In `@src/core/auth.rs`:
- Around line 240-250: The test token_file_has_owner_only_permissions calls
.mode() on std::fs::Permissions but is missing the Unix-specific trait; add use
std::os::unix::fs::PermissionsExt to the Unix-only imports in this file (near
the #[cfg(unix)] tests) so the .mode() method on Permissions resolves and the
test compiles. Ensure the import is gated by #[cfg(unix)] alongside the
token_file_has_owner_only_permissions test and related Unix-only imports.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18ff9b70-047d-48a3-b8ea-3607c533cb33

📥 Commits

Reviewing files that changed from the base of the PR and between 15c7442 and 4db8741.

📒 Files selected for processing (4)

src/core/auth.rs
src/openhuman/service/common.rs
src/openhuman/service/windows.rs
src/openhuman/update/core.rs

graycyrus

Review Summary

Mechanical refactor hoisting 4 inline use statements to file-level — 3 of 4 are correct. The fourth breaks Unix test compilation. Two minor style nits below.

graycyrus · 2026-05-12T18:43:44Z

@@ -32,6 +32,9 @@ use std::io::Write as _;
 use std::path::Path;
 use std::sync::OnceLock;



[critical] PermissionsExt import removed from the test at line 243 but never added here. This breaks token_file_has_owner_only_permissions compilation on Unix — .mode() on std::fs::Permissions requires the trait.

Suggested fix — merge both Unix trait imports into a single brace group:

#[cfg(unix)] use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};

graycyrus · 2026-05-12T18:43:44Z

 use std::io::Write;
 use std::path::PathBuf;

+#[cfg(unix)]


[minor] The project consistently uses as _ for extension traits imported only for their methods (Write as _ on line 4, OpenOptionsExt as _ in auth.rs). This should match:

Suggested change

#[cfg(unix)]

#[cfg(unix)]

use std::os::unix::fs::PermissionsExt as _;

graycyrus · 2026-05-12T18:43:45Z

 use crate::openhuman::config::Config;
 use anyhow::Result;
 use std::fs;
+use std::os::windows::process::CommandExt;


[nitpick] This compiles fine because the whole file is gated via #[cfg(windows)] mod windows; in mod.rs. A one-line comment would prevent a future maintainer from wondering whether this cross-compiles:

// File is `#[cfg(windows)]`-gated at the module level — no per-item guard needed. use std::os::windows::process::CommandExt;

…omment

…tements (tinyhumansai#1564) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>

PranavAgarkar07 requested a review from a team May 12, 2026 15:09

coderabbitai Bot reviewed May 12, 2026

View reviewed changes

coderabbitai Bot previously approved these changes May 12, 2026

View reviewed changes

graycyrus requested changes May 12, 2026

View reviewed changes

fix: add missing PermissionsExt import, use as_ convention, add cfg c…

7728801

…omment

PranavAgarkar07 dismissed coderabbitai[bot]’s stale review via 7728801 May 12, 2026 18:54

coderabbitai Bot approved these changes May 12, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into pr/1564

a3e2c74

senamakel merged commit 59bbf67 into tinyhumansai:main May 13, 2026
18 checks passed

senamakel added a commit to senamakel/openhuman that referenced this pull request May 13, 2026

refactor: move inline cfg-gated platform imports to top-level use sta…

b09332b

…tements (tinyhumansai#1564) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: move inline cfg-gated platform imports to top-level use statements#1564

refactor: move inline cfg-gated platform imports to top-level use statements#1564
senamakel merged 3 commits into
tinyhumansai:mainfrom
PranavAgarkar07:refactor/inline-imports-top-level-684

PranavAgarkar07 commented May 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 12, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

graycyrus left a comment

Uh oh!

graycyrus May 12, 2026

Uh oh!

graycyrus May 12, 2026

Uh oh!

graycyrus May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -32,6 +32,9 @@ use std::io::Write as _;
		use std::path::Path;
		use std::sync::OnceLock;

	#[cfg(unix)]
	#[cfg(unix)]
	use std::os::unix::fs::PermissionsExt as _;

Conversation

PranavAgarkar07 commented May 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

graycyrus left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

graycyrus May 12, 2026

Choose a reason for hiding this comment

Uh oh!

graycyrus May 12, 2026

Choose a reason for hiding this comment

Uh oh!

graycyrus May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PranavAgarkar07 commented May 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading