Skip to content

Conversation

@g-linville
Copy link
Member

@g-linville g-linville commented Jun 14, 2024

for #483

This creates a little wrapper type around the monitor, to be used only for credential calls, so that we do not print the plaintext output to the console.

Update: this instead excludes the output of credential tools from the CallFinish Event entirely, so the Monitor never has a chance to see it.

@ibuildthecloud
Copy link
Contributor

ibuildthecloud commented Jun 14, 2024

There might be a different way by just looking at the tool category in the monitor when it's printing. This is effectively what the TUI does, we don't print output for tool category "context", "provider" or "credential". For the CLI logs I would still like provider and context, but it make sense maybe to not print the credential? I'm not 100% why we care though.

@ibuildthecloud
Copy link
Contributor

Oh looking at this a big more, I think we probably need to drop the output in the place where we write the event. The event should not have the clear text of a password in it. Regardless if it's printed. But I'm still lost on how the event even has the clear text... Soo many layers. What madness have we created.

@g-linville
Copy link
Member Author

Oh looking at this a big more, I think we probably need to drop the output in the place where we write the event. The event should not have the clear text of a password in it. Regardless if it's printed. But I'm still lost on how the event even has the clear text... Soo many layers. What madness have we created.

@ibuildthecloud I can figure this out. I will probably have to plumb some gross thing through all the layers to get it to work, but as you said, that's the nature of the madness.

@g-linville g-linville marked this pull request as draft June 14, 2024 14:09
Signed-off-by: Grant Linville <[email protected]>
@g-linville g-linville force-pushed the fix-cred-tool-output branch from 4ca869f to bf402f9 Compare June 14, 2024 14:55
@g-linville g-linville marked this pull request as ready for review June 14, 2024 15:09
@ibuildthecloud ibuildthecloud self-requested a review June 14, 2024 16:28
Signed-off-by: Grant Linville <[email protected]>
@g-linville g-linville merged commit f8d3b44 into gptscript-ai:main Jun 14, 2024
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.

3 participants