fix(privileged-context): Reject statements with helpful error message (fixes bug 2027926)#61
fix(privileged-context): Reject statements with helpful error message (fixes bug 2027926)#61dmose wants to merge 1 commit intomozilla:mainfrom
Conversation
dmose
changed the title
|
Note that this may also be useful for non-privileged contexts, but I started just fixing the version of the issue I happened to be running into. That seems like it could be a follow-on fix, if needed. |
juliandescottes
left a comment
juliandescottes
left a comment
There was a problem hiding this comment.
Makes sense and following up for regular script evaluation as well.
dmose
force-pushed
the
fix-statement-fails
branch
3 times, most recently
from
968200d to
115b26f
Compare
Models sometimes pass JavaScript statements (const/let/var declarations) to evaluate_chrome_script, which wraps input as return(expression), which causes tool aborts, resulting in slowdown and wasted tokens. Adds isLikelyStatement() validation that detects statement keywords and returns a clear error message suggesting the IIFE workaround. Also updates tool description to warn against statement usage.
dmose
force-pushed
the
fix-statement-fails
branch
from
115b26f to
93864bf
Compare
|
OK, ready to merge, I think. But I don't have merge privs, so I'll have to defer doing that myself. :-) |
juliandescottes
left a comment
juliandescottes
left a comment
There was a problem hiding this comment.
@dmose two little things to adjust if you are still around today, otherwise I can merge and followup.
| @@ -0,0 +1,127 @@ | |||
| /** | |||
| * Tests for statement detection and rejection in privileged context tools | |||
There was a problem hiding this comment.
The suggestion was to move the existing tests to privileged-context-state.test.ts (since they are not really testing a tool, but rather the side effect of using a tool on the privileged context "state")
|
|
||
| vi.mock('../../src/index.js', () => ({ | ||
| getFirefox: () => mockGetFirefox(), | ||
| })); |
There was a problem hiding this comment.
Can we just move that to Privileged Context Tool Handlers and drop the comment now ?
2 participants
Fixes bug 2027926 - Models sometimes pass JavaScript statements (
const/let/vardeclarations) toevaluate_privileged_script, which wraps input asreturn(expression). This regularly causes issues likeError: Script execution failed: SyntaxError: expected expression, got keyword 'const', which slow things down and wastes tokens.Adds some verbiage to the tool description explicitly telling callers not to pass statements without wrapping them in an IIFE, along with an
isLikelyStatement()heuristic that detects statement keywords and returns a clear error message suggesting an IIFE.