Skip to content

feat: enable physical device for iOS profiling#38

Open
stachbial wants to merge 13 commits intomainfrom
@stachbial/ios-profiler-for-physical-device
Open

feat: enable physical device for iOS profiling#38
stachbial wants to merge 13 commits intomainfrom
@stachbial/ios-profiler-for-physical-device

Conversation

@stachbial
Copy link
Copy Markdown
Collaborator

@stachbial stachbial commented Mar 23, 2026 •
edited
Loading

  • renamed list-simulators to list-devices which behaves as previously with optional arg that triggers xcrun devicectl list .... run
  • links it to ios-profiler-start tool

@stachbial stachbial requested a review from latekvo March 23, 2026 19:03
@latekvo
Copy link
Copy Markdown
Member

latekvo commented Mar 23, 2026

I would much prefer a single tool that lists both devices and simulators, then CLEARLY labels which device is a simulator and which is a physical device. Then the agent should have it CLEARLY explained what are the limitations & reasons for using a simulator over a device and vice versa.

Unless there are arguments against this approach, my arguments are:

  • We have to clearly explain the differences between the approaches either way, we don't want the agent to confuse these options
  • Having separate tools still is more confusing than having one no matter how clearly you explain the difference with seemingly no quantifiable reason for having these tools split
  • More tools take up more context, having a single one takes up less context.

@stachbial
Copy link
Copy Markdown
Collaborator Author

stachbial commented Mar 23, 2026

I would much prefer a single tool that lists both devices and simulators, then CLEARLY labels which device is a simulator and which is a physical device. Then the agent should have it CLEARLY explained what are the limitations & reasons for using a simulator over a device and vice versa.

Unless there are arguments against this approach, my arguments are:

  • We have to clearly explain the differences between the approaches either way, we don't want the agent to confuse these options
  • Having separate tools still is more confusing than having one no matter how clearly you explain the difference with seemingly no quantifiable reason for having these tools split
  • More tools take up more context, having a single one takes up less context.

I hear you, that was my initial thought actually. Then I thought: due to the fact that most of Argent's features are heavily bound to simserver, querying the physical devices all the time would seem redundant and steering it with just a flag - harder to grasp for the agent. The standalone tool is rather easier to distinguish in such use case.
Regarding the simulator vs physical device differences explanation, I am not very found of that approach - I think we should always default to simulator and talk about/work with real devices if the user explicitly says so in the prompt (current implementation).

@stachbial
Copy link
Copy Markdown
Collaborator Author

stachbial commented Mar 26, 2026 •
edited
Loading

I would much prefer a single tool that lists both devices and simulators, then CLEARLY labels which device is a simulator and which is a physical device. Then the agent should have it CLEARLY explained what are the limitations & reasons for using a simulator over a device and vice versa.
Unless there are arguments against this approach, my arguments are:

  • We have to clearly explain the differences between the approaches either way, we don't want the agent to confuse these options
  • Having separate tools still is more confusing than having one no matter how clearly you explain the difference with seemingly no quantifiable reason for having these tools split
  • More tools take up more context, having a single one takes up less context.

I hear you, that was my initial thought actually. Then I thought: due to the fact that most of Argent's features are heavily bound to simserver, querying the physical devices all the time would seem redundant and steering it with just a flag - harder to grasp for the agent. The standalone tool is rather easier to distinguish in such use case. Regarding the simulator vs physical device differences explanation, I am not very found of that approach - I think we should always default to simulator and talk about/work with real devices if the user explicitly says so in the prompt (current implementation).

Eventually, a unified list-devices tool was made with physical device fetch optionally executed based on boolean arg. cc: @latekvo @pFornagiel

@stachbial
chore: rm unnecessary await from js-runtime-debbugger
d712aba
@stachbial
Merge remote-tracking branch 'origin/main' into @stachbial/ios-profil…
a74e6a7 
…er-for-physical-device

# Conflicts:
#	packages/skills/skills/argent-simulator-interact/SKILL.md
#	packages/ui/src/api/client.ts
latekvo
latekvo reviewed Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@latekvo latekvo left a comment

Choose a reason for hiding this comment

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

Nice. Probably also adjust the title to reflect what's being done in this PR.

Comment thread docs/argent_guide.md Outdated
Comment thread docs/cli-usage.md Outdated
Comment thread docs/distribution-guide.md Outdated
Comment thread docs/npm-migration-guide.md Outdated
Comment thread docs/npm-migration-guide.md Outdated
Comment thread docs/npm-migration-guide.md Outdated
Comment thread packages/mcp/scripts/bundle-tools.cjs
@latekvo
Copy link
Copy Markdown
Member

latekvo commented Mar 31, 2026 •
edited
Loading

I'm so lost right now. What did I just review? The comments are here but the diff is from a different PR?

edit:

hmm could this be the cause?
image

@stachbial stachbial changed the title enable ios profiler session for physical device enable physical device for iOS profiling Mar 31, 2026
@stachbial
Copy link
Copy Markdown
Collaborator Author

stachbial commented Mar 31, 2026 •
edited
Loading

I'm so lost right now. What did I just review? The comments are here but the diff is from a different PR?

@latekvo Yep It looks like You reviewed a different PR, probably a a stale diff in your cache 👀

latekvo
latekvo reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@latekvo latekvo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work, i wish we could merge this really soon ❤️
Just noticed i had these 2 comments pending for the last week - they're just nitpicks.

Comment thread packages/tool-server/src/blueprints/js-runtime-debugger.ts Outdated
Comment thread packages/tool-server/src/utils/ios-device.ts Outdated
@stachbial stachbial changed the title enable physical device for iOS profiling feat: enable physical device for iOS profiling Apr 16, 2026
@stachbial
Merge remote-tracking branch 'origin/main' into @stachbial/ios-profil…
79ad5a6 
…er-for-physical-device

# Conflicts:
#	README.md
#	packages/skills/skills/argent-react-native-app-workflow/SKILL.md
#	packages/tool-server/src/tools/simulator/list-simulators.ts
#	packages/tool-server/src/utils/setup-registry.ts
@stachbial
Merge remote-tracking branch 'origin/main' (tree already reconciled i…
f6af860 
…n prior merge)
@stachbial
chore: prettier formatting on merged files
8408426
latekvo added a commit that referenced this pull request Apr 17, 2026
@latekvo
fix: correctness bugs surfaced by review — parser, launcher, temp-file
a59376e 
Three independent bugs in the Android-path code that reviewers repro'd:

1. uiautomator entity decoder double-decoded. The decoder ran numeric
   references as one replace pass, then each of the five named entities
   as its own pass. An ampersand decoded in the first pass fed straight
   into the second: `<` (correct XML encoding of the literal
   string `&lt;`) collapsed to `<`, violating XML 1.0 §4.6. Replaced
   with a single regex alternation so every match is consumed once.

2. `launch-app` Android path used two different launch mechanisms — a
   blocking `am start -W` when the caller passed an `activity`, and a
   fire-and-forget `monkey … LAUNCHER 1` when they didn't. The monkey
   path returned as soon as the intent was injected, leaving a window
   where describe/tap raced a still-forking process. Unified on
   `am start -W` by resolving the default activity up-front via
   `cmd package resolve-activity --brief`. Also replaced the brittle
   `/Error|Exception/ && !/Status: ok/` matcher with a positive match
   on `Status: ok` — the old regex false-succeeded on `Status: null`
   (activity threw in onCreate) and would have false-failed if Android
   ever dropped the `Status:` banner from a release that keeps benign
   strings like `Activity: com.example.ErrorReportingActivity` in the
   output.

3. `describe` Android path shell-chained cleanup with `&&`, so a
   failing `uiautomator dump` (keyguard, MFA flap, secure overlay)
   short-circuited before `rm -f` ever ran and leaked a file per
   attempt under /data/local/tmp. One-char fix: trailing `; rm -f`
   instead of `&& rm -f`.

Regression tests added for all three: `&amp;lt;` / `&#38;lt;` /
`&#x26;amp;` stay literal, `am start` success/failure permutations,
and a shell-string assertion pinning the `;` before `rm -f`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@latekvo latekvo latekvo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stachbial @latekvo