feat: preloaded skills + external-services category - #115
Conversation
…ategory Add 'preloaded: true' frontmatter to 25 skills that should always appear in the system prompt with full descriptions. All other skills are hidden behind category summaries. New external-services/ category for CLI integrations: - porkbun-cli (merged from porkbun + 6 sub-skills) - copilot-money-cli (renamed from copilot-money) - appfigures-cli (renamed from appfigures) - dataforseo-cli (renamed from dataforseo) - google-ads-cli (renamed from google-ads) - meta-ads-cli (renamed from meta-ads) - prometheus-cli (renamed from prometheus) - higgsfield (found and added) - stably-cli, firecrawl, bird-twitter, apple-search-ads (moved) Also fixed DESCRIPTION.md files to use proper YAML frontmatter format so category descriptions appear in the system prompt. Works with the runtime change in exiao/hermes-agent#6 which adds preloaded-aware rendering to build_skills_system_prompt().
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds several new AI tools and CLI integrations, including Apple Search Ads, DataForSEO, Copilot Money, and Prometheus. It also updates existing skill descriptions to mark them as preloaded. My feedback focuses on improving the robustness of CSV parsing in shell scripts, correcting an API method in the Google Ads documentation, aligning JWT expiration times, and improving error handling and code maintainability in the provided scripts.
|
|
||
| # Build JSON array from CSV (skip header if present) | ||
| local keywords_json="[]" | ||
| while IFS=',' read -r text match bid; do |
There was a problem hiding this comment.
The while IFS=',' read ... loop for parsing the CSV file is not robust. It will fail if any field, such as the keyword text, contains a comma. This could lead to incorrect data being processed or script failures. Consider using a more robust parsing method, like a small Python script with its csv module, which can handle quoted fields and commas within values correctly.
| fi | ||
|
|
||
| local negatives_json="[]" | ||
| while IFS=',' read -r text match; do |
There was a problem hiding this comment.
Similar to keywords.sh, the CSV parsing here using while IFS=',' read ... is fragile. It will break if the keyword text contains a comma. This can lead to unexpected behavior and data corruption. It's recommended to switch to a more robust CSV parser, for instance by using Python's csv module, to handle such edge cases properly.
| const conversions = stats.getConversions(); | ||
| const ctr = stats.getClickThroughRate(); // As decimal (0-1) | ||
| const cpc = stats.getAverageCpc(); // Average cost per click | ||
| const cpa = stats.getAveragePageviews(); // NOTE: check specific metric |
There was a problem hiding this comment.
The example code incorrectly uses stats.getAveragePageviews() to retrieve the Cost Per Acquisition (CPA). The correct method is stats.getCostPerConversion(). This is a significant error in an API reference document that could lead to incorrect script implementations and flawed performance analysis.
| const cpa = stats.getAveragePageviews(); // NOTE: check specific metric | |
| const cpa = stats.getCostPerConversion(); // NOTE: check specific metric |
|
|
||
| # Create JWT payload | ||
| now=$(date +%s) | ||
| exp=$((now + 15552000)) # 180 days |
There was a problem hiding this comment.
The JWT expiration time for the client secret is set to 180 days in this example script, but the implementation in scripts/asa-auth.sh uses a 1-hour expiration. While both are technically valid according to Apple's documentation, this inconsistency can be confusing. For better maintainability and security, it's recommended to align the documentation with the shorter, safer 1-hour expiration used in the actual script.
| exp=$((now + 15552000)) # 180 days | |
| exp=$((now + 3600)) # 1 hour |
| local body | ||
| body=$(_build_report_body "$start" "$end" "DAILY" "localSpend" "1000") | ||
| local response | ||
| response=$(asa_api POST "/reports/campaigns/${cid}/keywords" "$body") 2>/dev/null || continue |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null can hide important error messages from the asa_api function, such as authentication failures or invalid requests. While this might be intended to ignore campaigns without keyword data, it makes debugging other potential issues difficult. A better approach would be to handle specific error cases inside the loop or modify asa_api to support a quiet mode.
| #### 5d — Generate Each Creative | ||
|
|
||
| ```bash | ||
| GEMINI_API_KEY=$(python3 -c "import json, os; d=json.load(open(os.path.expanduser('~/.openclaw/openclaw.json'))); print(d.get('skills',{}).get('entries',{}).get('nano-banana-pro',{}).get('apiKey','') or d['env']['vars'].get('GEMINI_API_KEY',''))" 2>/dev/null) |
There was a problem hiding this comment.
This one-liner to extract the GEMINI_API_KEY is overly complex and hard to maintain. It attempts to read from a JSON configuration file with multiple fallbacks, all within a single shell command. This should be refactored into a dedicated helper script or function for clarity, testability, and easier debugging.
1 participant
Summary
Two-tier skill loading: 26 skills get
preloaded: truein frontmatter so they always appear in the system prompt with full descriptions. Everything else is hidden behind category summaries. Requires the runtime change in exiao/hermes-agent#6.New:
external-services/categoryConsolidated all third-party service CLI skills into one category:
porkbun-cliporkbun+ 6 sub-skillsdevops/porkbun/copilot-money-clicopilot-moneyfinance/appfigures-cliappfiguresmarketing/dataforseo-clidataforseomarketing/google-ads-cligoogle-adsmarketing/meta-ads-climeta-adsmarketing/prometheus-cliprometheusmarketing/higgsfieldcreative/stably-clistably-clidevops/firecrawlfirecrawlcoding/bird-twitterbird-twittermarketing/apple-search-adsApple Search Adsmarketing/Preloaded skills (26)
All get
preloaded: truein frontmatter:web-search, recall, writer, plan, skill-creator, render-cli, sentry-debug, grok-search, mcporter, bloom-cli, babysit-pr, ralph-mode, firecrawl, stably-cli, porkbun-cli, copilot-money-cli, higgsfield, appfigures-cli, bird-twitter, apple-search-ads, dataforseo-cli, google-ads-cli, meta-ads-cli, prometheus-cli, last30days, dogfood
DESCRIPTION.md fixes
Fixed category DESCRIPTION.md files to use proper YAML frontmatter format so category descriptions appear in the system prompt.
Token impact