-
-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[WIP] Auto mode #3
Conversation
r2ai/interpreter.py
Outdated
file_dir = os.path.dirname(__file__) | ||
sys.path.append(file_dir) | ||
import index | ||
|
||
r2clippy = False | ||
have_rlang = False | ||
sysprint = print |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a r2lang.write
? Or just keep using sys.stdout.write
? It seems to not like it when I use r2lang.print
and sys.stdout.write
together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhm.. but r2lang.X is buffering output into RCons, wont be shown until flushed to the console. its not suposed to be used as in live mode. If thats fine we can add it in the rlang api. otherwise i would suggest to buffer the output while printing it and then just do one call at the end? do this work for u?
you can also use the builtins.print. but that wont be captured by r2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah makes sense, i dug into the r2lang code a bit and have a branch with a r2lang.write exposed but im not sure we even need r2 to be aware of the output of the LLM stream. Do you think we need to?
If you're ok with using the builtins.print or even just how i have it with stdout.write, I'll keep it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only reason to have r2 aware of the output from r2ai is when its going to be used for scripting it via r2pipe/r2js, and i made r2ai.interactive to use builtins.print or r2lang.print depending on that variable just to let the user have an interactive output or a bfufered one via rcons.
so i think auto should just honor this behaviour. what do you think?
r2ai/interpreter.py
Outdated
sys.stdout.write('\x1b[1;32mRunning \x1b[4m' + "#!python temp.py" + '\x1b[0m\n') | ||
sysprint(args["command"]) | ||
sysprint("") | ||
r2lang.cmd('#!python temp.py > temp_output') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can also redirect the output to an internal file like this:
r2lang.cmd('#!python temp.py > $tmp')
output = r2lang.cmd('cat $tmp')
the files starting with $ are internal to r2, so wont create any file in disk..
if reading from disk is necessary better use the slurp api from utils.py, and dont forget to remove it after using it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh nice, thanks!
theres also this PoC https://github.com/radareorg/r2ai/blob/master/examples/funcall.py and this https://local-llm-function-calling.readthedocs.io/en/latest/generation.html but im also fine to merge it liek this and introduce local function calling later in a separate pr |
let me just clean a few things up on Monday and we can release it as first pass, has all the functionality r2d2 has already |
Cool! yeah i see that r2d2 auto mode is really cool i did some tests few days ago changing the prompt to give some hints about how to use r2frida, pdg, and other commands in a better way and the results were amazing. So im looking forward to use the gemma model as well as other local models like the phi ones with vectordb containing all those hints. |
Take a look at this paper: https://arxiv.org/pdf/2402.11814.pdf With r2, in my tests, it can often correctly set breakpoints, read memory and modify instructions to bypass whatever it needs. |
m = delta.content | ||
if m is not None: | ||
msgs.append(m) | ||
sys.stdout.write(m) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept this, otherwise there's no point in streaming, so you would sit there waiting for output for a long time. Hope that's ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i dont think it makes much sense to buffer output when using the auto mode
if tool_call["function"]["name"] == "r2cmd": | ||
args = json.loads(tool_call["function"]["arguments"]) | ||
builtins.print('\x1b[1;32mRunning \x1b[4m' + args["command"] + '\x1b[0m') | ||
res = r2lang.cmd(args["command"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to get the error message from r2lang.cmd
? It prints in r2 but it's not returned here. It prevents the LLM from seeing the error, so it's hard for it to recover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, there are different ways but none of them is a clean solution. ive been thinking in designing r2pipe2 to handle binary input/output, as well as capture command return code and stderr contents as well. but it's not yet implemented and i prefer to wait until i get it right.
Right now there are the following solutions:
- use r2pipe-side, which runs a callback everytime something is written to stderr (async) only for js and v, not py
- use 2> foo and then read the contents of foo in a separate command (note that for some reason '2>$foo' is not working. i opened an issue and will fix that too
And right now im adding a new option to make rlog use rcons, so the error messages will be printed to the same buffer as the output and you can get the error message in the same output.
ideally it will be good to have a cmd call that returns output, error and return code i think, i can add a separate cmd method in the rlang api for that later on. which seems doable. not sure if i will be missing any usecase here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, i'll update when the radare2 PR lands. This would be the biggest deal for functionality, so the LLM can recover from errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's merged :) e log.cons=true
. there's also log.color in case you want to keep the colors for command output and disable the ones from logs
I think it's ok right now, lmk if you want me to change anything. I'll switch to figuring out support for local models. |
@@ -78,6 +79,7 @@ def do_POST(self): | |||
help_message = """Usage: r2ai [-option] ([query] | [script.py]) | |||
r2ai . [file] interpret r2ai script with access to globals | |||
r2ai :aa run a r2 command | |||
r2ai :auto [prompt] query LLM that can interact with r2 (WIP, OpenAI only atm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats the only thing i dislike from this PR . the : character is suposed to run r2 commands. and auto is not an r2 command. it will be better to use a different thing like adding a new flag (-a and -A are taken already, but we can change them if needed), or we can use the "' " syntax as you did in r2d2, the "'" thing conflicts with the r2 syntax, but as long as there's a space after the quote it is fine for me to use it that way.
i will merge it the way it is because i dont think this is a blocker for anything, and we have plenty of time to discuss that later. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, i didn't even think about it when i started with ":auto", i'll change it to "' " in the next round
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
No description provided.