fix: credentials: block output while running credential tools #261
Conversation
Signed-off-by: Grant Linville <[email protected]>
g-linville
requested review from
thedadams,
njhale,
StrongMonkey,
iwilltry42,
keyallis and
tylerslaton
|
Talked with Darren and I'm gonna refactor this a bit. (Edit: refactor complete.) |
Signed-off-by: Grant Linville <[email protected]>
| pause := context2.GetPauseFuncFromCtx(ctx) | ||
| unpause := pause() | ||
| defer unpause() |
There was a problem hiding this comment.
This could technically be simplified down to defer context2.GetPauseFuncFromCtx(ctx)()() but I was afraid that would be too confusing, so I split it into three lines to make it clearer what is happening here.
There was a problem hiding this comment.
Part of me would love to see this as defer context2.GetPauseFuncFromCtx(ctx)()(), but I think the split is the right idea.
There was a problem hiding this comment.
And then you'd be a terrible person. What maniac would do a chained function call in a single line defer :)
| pause := context2.GetPauseFuncFromCtx(ctx) | ||
| unpause := pause() | ||
| defer unpause() |
There was a problem hiding this comment.
Part of me would love to see this as defer context2.GetPauseFuncFromCtx(ctx)()(), but I think the split is the right idea.
for #256
One of the goals of the credentials framework was to block output to the terminal while running
sys.prompt, since it prompts the user directly in the terminal. I originally implemented this by storing the Monitor into the Context and then pulling it out of the Context and running itsPausefunction in the implementation ofsys.prompt. However, this does not work, as credential tools pretty much always have to callsys.promptwith a new gptscript subprocess.This new implementation instead stores the Pause function inside of the context and calls it within the Engine code prior to running a credential tool. This way, the Monitor is paused in the correct process.