-
Notifications
You must be signed in to change notification settings - Fork 0
[GENAI-2335] Implement Policy Engine for Smart Window tool execution #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: smart-window
Are you sure you want to change the base?
Changes from 1 commit
3453fab
1578d0b
9d5dd1a
3f76c54
bf8d795
647e1b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,12 @@ export class PolicyEvaluator { | |
| * @returns {boolean} True if policy applies to this action | ||
| */ | ||
| static checkMatch(matchCriteria, action) { | ||
| console.warn("[PolicyEvaluator] checkMatch criteria:", JSON.stringify(matchCriteria), "action:", JSON.stringify(action)); | ||
| console.warn( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why warn here? shouln't it be debug?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that makes sense. I will change this to debug 🐛
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right, I remembered why I changed this to For now, I can add the comment to disable the lint error since this is a log intentionally for debugging purposes. |
||
| "[PolicyEvaluator] checkMatch criteria:", | ||
| JSON.stringify(matchCriteria), | ||
| "action:", | ||
| JSON.stringify(action) | ||
| ); | ||
| if (!matchCriteria || typeof matchCriteria !== "object") { | ||
| return false; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
question: Would it make sense for this to be considered an error and brought to the user's attention that the permission access request failed?
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 question! I've given this some thought. Since these log messages are intended for developers rather than users, I don't think we need to surface this to the user directly. They'll see a denial when the tool call is evaluated by the security layer (fail-closed behavior).
However, I've tightened up the logging to give better signal: "no ledger available" is now console.error, while "already in ledger" is console.debug. This should give developers better visibility into the unexpected failures.