Skip to content

Fix non-admin scan failures - #15

Merged
hewigovens merged 2 commits into
mainfrom
fix-non-admin-scan-cli-path
Apr 27, 2026
Merged

Fix non-admin scan failures#15
hewigovens merged 2 commits into
mainfrom
fix-non-admin-scan-cli-path

Conversation

@hewigovens

Copy link
Copy Markdown
Owner

Summary

  • skip unreadable child folders during app scans and show a non-fatal warning
  • add a macOS UI test that covers an unreadable folder next to a readable app
  • add a reusable --scan-folder startup option and document it
  • improve CLI installer PATH detection by checking the login shell, and simplify the fish hint
  • bump AppDetective to 1.4.1 build 7

Validation

  • just test
  • just release-dry-run 1.4.1
  • CI passed on the branch before the version bump; this push will run CI again

Notes

This PR is draft while the 1.4.1 dry-run build is tested on a standard non-admin macOS user account.

- Hero image switches from the magnifyingglass.circle.fill SF Symbol to NSImage.applicationIconName so the running app's icon is shown instead of a generic system glyph.
- Drop the two-paragraph permission/folder explanation; the choose-folder dialog already conveys what's happening.
- Add a one-line tip pointing to the App Detective menu's "Install Command Line Tool…" item for users who want to inspect a single app from a terminal.
- Tighten vertical spacing now that there's less copy.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces the ability to specify a startup scan folder via command-line arguments and improves the CLI installation process by providing shell-specific path hints and checking the login shell's PATH. It also enhances the scanning engine to gracefully handle and report permission-denied errors in subdirectories without failing the entire scan. Feedback highlights a potential UI hang due to synchronous process execution on the main thread, an invalid macOS deployment target in the project configuration, a logic flaw in argument parsing, and a potential deadlock when reading shell output.

Comment thread AppDetective/AppDetective/Services/CLIInstallerService.swift Outdated
Comment thread AppDetective/project.yml Outdated
Comment thread AppDetective/AppDetective/App/LaunchArguments.swift Outdated
Comment thread AppDetective/AppDetective/Services/CLIInstallerService.swift Outdated

@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: 71abe96d5b

ℹ️ 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 thread AppDetective/AppDetective/Services/CLIInstallerService.swift Outdated
Comment thread AppDetective/AppDetective/Services/CLIInstallerService.swift Outdated
@hewigovens
hewigovens force-pushed the fix-non-admin-scan-cli-path branch from 71abe96 to 80d541d Compare April 27, 2026 06:47
@hewigovens
hewigovens merged commit f39f24c into main Apr 27, 2026
2 checks passed
@hewigovens
hewigovens deleted the fix-non-admin-scan-cli-path branch April 27, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant