-
Notifications
You must be signed in to change notification settings - Fork 0
switching skill from python to bash #3
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
Conversation
… to help people get to an API key)
📝 WalkthroughWalkthroughThis pull request replaces the Python-based TinyFish extraction helper with a Bash-based implementation and updates documentation accordingly. The Python Sequence Diagram(s)mermaid 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@skills/tinyfish-web-agent/scripts/extract.sh`:
- Around line 38-41: The --proxy case in the argument-parsing branch (where
PROXY_COUNTRY is set) doesn't validate that a following argument exists and is
not another flag, which under set -euo pipefail can produce an unbound
parameter/shift error; update the --proxy handling (the case that sets
PROXY_COUNTRY) to first check that "$2" is present and does not start with '-'
and if it is missing or looks like a flag print a clear error message and exit
non-zero, otherwise assign PROXY_COUNTRY="$2" and shift 2 as before.
- Around line 49-61: The current JSON_URL/JSON_GOAL escaping only handles
backslashes and quotes and breaks on newlines/tabs/control chars; replace the
sed-based escaping for JSON_URL and JSON_GOAL with a proper JSON string encoding
step (e.g., use jq -R -s `@json` or a small Python json.dumps invocation) so
newlines/tabs/control characters are escaped correctly, then use those encoded
values when constructing PAYLOAD (adjust how you combine them so you don’t
double-quote the JSON-encoded output). Update the assignments to JSON_URL and
JSON_GOAL and the PAYLOAD construction to use the new, safely encoded strings.
In `@skills/tinyfish-web-agent/SKILL.md`:
- Line 22: Replace the bare URL on the line "You need a Mino API key. Get one
at: https://mino.ai/api-keys" with a Markdown autolink by wrapping the URL in
angle brackets (e.g., use <https://mino.ai/api-keys>) so it satisfies
markdownlint rule MD034; update the text in SKILL.md accordingly.
- Around line 27-29: The fenced code block containing the environment export
(the lines with export MINO_API_KEY="your-key-here") needs a language identifier
to satisfy MD040; update the opening fence from ``` to ```bash so the block is
marked as shell/bash (e.g., change the code fence that wraps export
MINO_API_KEY="your-key-here" to start with ```bash).
| --proxy) | ||
| PROXY_COUNTRY="$2" | ||
| shift 2 | ||
| ;; |
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.
Validate --proxy has a country argument.
With set -euo pipefail, --proxy without a value can trigger an unbound parameter/shift error instead of a clear message.
🛠️ Suggested fix
--proxy)
+ if [ $# -lt 2 ] || [[ "$2" == --* ]]; then
+ echo "Error: --proxy requires COUNTRY" >&2
+ exit 1
+ fi
PROXY_COUNTRY="$2"
shift 2
;;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --proxy) | |
| PROXY_COUNTRY="$2" | |
| shift 2 | |
| ;; | |
| --proxy) | |
| if [ $# -lt 2 ] || [[ "$2" == --* ]]; then | |
| echo "Error: --proxy requires COUNTRY" >&2 | |
| exit 1 | |
| fi | |
| PROXY_COUNTRY="$2" | |
| shift 2 | |
| ;; |
🤖 Prompt for AI Agents
In `@skills/tinyfish-web-agent/scripts/extract.sh` around lines 38 - 41, The
--proxy case in the argument-parsing branch (where PROXY_COUNTRY is set) doesn't
validate that a following argument exists and is not another flag, which under
set -euo pipefail can produce an unbound parameter/shift error; update the
--proxy handling (the case that sets PROXY_COUNTRY) to first check that "$2" is
present and does not start with '-' and if it is missing or looks like a flag
print a clear error message and exit non-zero, otherwise assign
PROXY_COUNTRY="$2" and shift 2 as before.
| # Build JSON payload — escape URL and goal for safe embedding | ||
| JSON_URL=$(printf '%s' "$URL" | sed 's/\\/\\\\/g; s/"/\\"/g') | ||
| JSON_GOAL=$(printf '%s' "$GOAL" | sed 's/\\/\\\\/g; s/"/\\"/g') | ||
|
|
||
| PAYLOAD="{\"url\":\"${JSON_URL}\",\"goal\":\"${JSON_GOAL}\"" | ||
|
|
||
| if [ "$STEALTH" = true ]; then | ||
| PAYLOAD="${PAYLOAD},\"browser_profile\":\"stealth\"" | ||
| fi | ||
|
|
||
| if [ -n "$PROXY_COUNTRY" ]; then | ||
| PAYLOAD="${PAYLOAD},\"proxy_config\":{\"enabled\":true,\"country_code\":\"${PROXY_COUNTRY}\"}" | ||
| fi |
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.
Harden JSON escaping to handle newlines/tabs/control chars.
Current escaping only handles \ and ". Multi-line goals (common) will produce invalid JSON.
🛠️ Suggested fix
+# Escape input for JSON string context
+json_escape() {
+ local s="$1"
+ s=${s//\\/\\\\}
+ s=${s//\"/\\\"}
+ s=${s//$'\n'/\\n}
+ s=${s//$'\r'/\\r}
+ s=${s//$'\t'/\\t}
+ s=${s//$'\f'/\\f}
+ s=${s//$'\b'/\\b}
+ printf '%s' "$s"
+}
+
# Build JSON payload — escape URL and goal for safe embedding
-JSON_URL=$(printf '%s' "$URL" | sed 's/\\/\\\\/g; s/"/\\"/g')
-JSON_GOAL=$(printf '%s' "$GOAL" | sed 's/\\/\\\\/g; s/"/\\"/g')
+JSON_URL=$(json_escape "$URL")
+JSON_GOAL=$(json_escape "$GOAL")
@@
if [ -n "$PROXY_COUNTRY" ]; then
- PAYLOAD="${PAYLOAD},\"proxy_config\":{\"enabled\":true,\"country_code\":\"${PROXY_COUNTRY}\"}"
+ JSON_PROXY=$(json_escape "$PROXY_COUNTRY")
+ PAYLOAD="${PAYLOAD},\"proxy_config\":{\"enabled\":true,\"country_code\":\"${JSON_PROXY}\"}"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Build JSON payload — escape URL and goal for safe embedding | |
| JSON_URL=$(printf '%s' "$URL" | sed 's/\\/\\\\/g; s/"/\\"/g') | |
| JSON_GOAL=$(printf '%s' "$GOAL" | sed 's/\\/\\\\/g; s/"/\\"/g') | |
| PAYLOAD="{\"url\":\"${JSON_URL}\",\"goal\":\"${JSON_GOAL}\"" | |
| if [ "$STEALTH" = true ]; then | |
| PAYLOAD="${PAYLOAD},\"browser_profile\":\"stealth\"" | |
| fi | |
| if [ -n "$PROXY_COUNTRY" ]; then | |
| PAYLOAD="${PAYLOAD},\"proxy_config\":{\"enabled\":true,\"country_code\":\"${PROXY_COUNTRY}\"}" | |
| fi | |
| # Escape input for JSON string context | |
| json_escape() { | |
| local s="$1" | |
| s=${s//\\/\\\\} | |
| s=${s//\"/\\\"} | |
| s=${s//$'\n'/\\n} | |
| s=${s//$'\r'/\\r} | |
| s=${s//$'\t'/\\t} | |
| s=${s//$'\f'/\\f} | |
| s=${s//$'\b'/\\b} | |
| printf '%s' "$s" | |
| } | |
| # Build JSON payload — escape URL and goal for safe embedding | |
| JSON_URL=$(json_escape "$URL") | |
| JSON_GOAL=$(json_escape "$GOAL") | |
| PAYLOAD="{\"url\":\"${JSON_URL}\",\"goal\":\"${JSON_GOAL}\"" | |
| if [ "$STEALTH" = true ]; then | |
| PAYLOAD="${PAYLOAD},\"browser_profile\":\"stealth\"" | |
| fi | |
| if [ -n "$PROXY_COUNTRY" ]; then | |
| JSON_PROXY=$(json_escape "$PROXY_COUNTRY") | |
| PAYLOAD="${PAYLOAD},\"proxy_config\":{\"enabled\":true,\"country_code\":\"${JSON_PROXY}\"}" | |
| fi |
🤖 Prompt for AI Agents
In `@skills/tinyfish-web-agent/scripts/extract.sh` around lines 49 - 61, The
current JSON_URL/JSON_GOAL escaping only handles backslashes and quotes and
breaks on newlines/tabs/control chars; replace the sed-based escaping for
JSON_URL and JSON_GOAL with a proper JSON string encoding step (e.g., use jq -R
-s `@json` or a small Python json.dumps invocation) so newlines/tabs/control
characters are escaped correctly, then use those encoded values when
constructing PAYLOAD (adjust how you combine them so you don’t double-quote the
JSON-encoded output). Update the assignments to JSON_URL and JSON_GOAL and the
PAYLOAD construction to use the new, safely encoded strings.
Python had some issues when the agent tries to install it
Instead,