-
Notifications
You must be signed in to change notification settings - Fork 811
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
fix the problem of verification_code not injected into string navigation goal #603
Conversation
<!-- ELLIPSIS_HIDDEN --> | 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 58e4e577d7f23cdf49b9b3546d63caf8bcf92965 | |--------|--------| ### Summary: Modified `_build_navigation_payload` in `skyvern/forge/agent.py` to handle string payloads by appending a verification code placeholder if needed. **Key points**: - **Modified** `_build_navigation_payload` in `skyvern/forge/agent.py`. - **Added** handling for `final_navigation_payload` when it is a string. - **Appends** verification code placeholder to string payload if `task.totp_verification_url` is present and placeholder is missing. ---- Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev) <!-- ELLIPSIS_HIDDEN -->
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.
❌ Changes requested. Reviewed everything up to b6a7ec4 in 48 seconds
More details
- Looked at
29
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_6uGJKvjrwlb7wo0O
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and 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.
❌ Changes requested. Incremental review on b6a7ec4 in 53 seconds
More details
- Looked at
28
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_njyPiErtKJ3zDfMa
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
final_navigation_payload[SPECIAL_FIELD_VERIFICATION_CODE] = VERIFICATION_CODE_PLACEHOLDER | ||
elif ( | ||
isinstance(final_navigation_payload, str) | ||
and SPECIAL_FIELD_VERIFICATION_CODE not in final_navigation_payload |
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 check SPECIAL_FIELD_VERIFICATION_CODE not in final_navigation_payload
is not valid for string types as it assumes final_navigation_payload
is a dictionary. This could lead to incorrect behavior when final_navigation_payload
is a string.
and SPECIAL_FIELD_VERIFICATION_CODE not in final_navigation_payload | |
isinstance(final_navigation_payload, str) and SPECIAL_FIELD_VERIFICATION_CODE not in final_navigation_payload.split() |
Summary:
Modified
_build_navigation_payload
inskyvern/forge/agent.py
to handle string payloads by appending a verification code placeholder if needed.Key points:
_build_navigation_payload
inskyvern/forge/agent.py
.final_navigation_payload
when it is a string.task.totp_verification_url
is present and placeholder is missing.Generated with ❤️ by ellipsis.dev