fix: default apps listing to user-installed#541
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b407be931
ℹ️ 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".
| appsFilter === 'all' | ||
| ? 'Showing all apps, including system apps.' | ||
| : 'Showing user-installed apps. Use --all to include system apps.'; | ||
| return [apps.join('\n'), hint].filter(Boolean).join('\n'); |
There was a problem hiding this comment.
When agent-device apps is used in non-JSON mode by scripts or agents that consume stdout as a newline-delimited app list, this appends a non-app hint as an extra stdout line, so downstream selection can treat Showing user-installed apps... or Showing all apps... as an app identifier. Other command metadata/hints in the CLI are written to stderr, so keep stdout to app rows and emit this guidance separately.
Useful? React with 👍 / 👎.
Architecture ReviewSmall, focused PR (174/-20, 18 files). Read every patch in full. Direction overallRight call. Defaulting What's well done
Concerns
Forward-looking suggestionThe pattern "platform helper returns data + policy hint" will repeat (devices, sessions, filtered snapshots). Consider a richer envelope from the helpers: type AppListResult = {
apps: AppInfo[];
filter: 'user-installed' | 'all';
hiddenCount?: number;
};Then the CLI is a renderer, not a policy decider, and programmatic SDK consumers get the same signal (right now the hint exists only in the CLI's human path). VerdictShip it — the direction is good and the consistency win is real. The DRY-violating 7-place default and the eager-import asymmetry are the only items I'd touch before merge; the rest are nice-to-haves for a follow-up that adds the Generated by Claude Code |
a5881ea to
3a35837
Compare
3a35837 to
bb02bbe
Compare
Summary
appsdiscovery to user-installed apps and keep--allas the explicit full-inventory mode.Validation
/Users/thymikee/.nvm/versions/node/v24.13.0/bin/pnpm formatPATH=/Users/thymikee/.nvm/versions/node/v24.13.0/bin:$PATH /Users/thymikee/.nvm/versions/node/v24.13.0/bin/pnpm check:quick/Users/thymikee/.nvm/versions/node/v24.13.0/bin/pnpm exec vitest run src/utils/__tests__/args.test.ts src/__tests__/cli-config.test.ts src/__tests__/cli-client-commands.test.ts src/platforms/android/__tests__/index.test.ts src/platforms/android/__tests__/app-helpers.test.ts src/platforms/ios/__tests__/index.test.ts src/daemon/handlers/__tests__/session.test.tsPATH=/Users/thymikee/.nvm/versions/node/v24.13.0/bin:$PATH /Users/thymikee/.nvm/versions/node/v24.13.0/bin/pnpm buildemulator-5554:./bin/agent-device.mjs apps --platform androidshowedDev (com.expensify.chat.dev)and hid system packages by default;./bin/agent-device.mjs apps --platform android --allshowed system apps such asSettings (com.android.settings).Touched 18 files. Scope expanded from Android inventory to shared apps defaults so CLI, daemon, Android helper APIs, and Apple helper APIs do not disagree.