feat: Playwright skills (screenshot, browser) + scrape backend - #53
Merged
Conversation
…crape backend Add two new auto-invoked skills that leverage Playwright as an optional dependency: - screenshot: capture web pages as PNG/PDF (full-page, viewport, element, device emulation) - browser: general browser automation (click, fill, navigate, extract, codegen, E2E testing) Also add Playwright as a new backend in the existing scrape skill, slotted between Jina Reader and Firecrawl for JS-heavy pages without paid API dependencies. Playwright stays optional — users install it themselves via `npx playwright install chromium`. Skills check for availability and provide install instructions when missing. Research in homebase/docs/research/browser-automation-for-devkit.md covers the full landscape comparison (Playwright, shot-scraper, Puppeteer, Selenium, Rod, Crawlee, Skyvern, Browser Use, Stagehand, Steel, Browserbase, AgentQL).
Critical: - scrape: remove fabricated `npx playwright cr -e` subcommand (not a real Playwright CLI command) — replace with file-based .mjs script that reads the URL from process.argv, invoked via shell variable. Kills shell injection and script injection via URL contents. - scrape: expand URL validation to match screenshot's full blocklist (include 169.254.169.254 cloud metadata IP). Require reject-and-stop reporting in batch mode. - browser: expand one-line URL validation to full blocklist matching scrape and screenshot. Add cloud metadata IP. Require reject-and-stop. Important: - scrape: fallback chain contradicted itself (Jina→Playwright→WebFetch vs Jina→Playwright→Firecrawl→WebFetch). Now single linear chain: Jina → Playwright → Firecrawl → WebFetch. Require reporting which backend actually served the result + why prior backends failed, so silent content substitution (paywall/cookie-wall) can't sneak through. - scrape: remove paywall contradiction — listed Playwright as good for paywalled content while Rules forbade scraping paywalled content. - screenshot: element-selector block self-contradicted (told agent not to interpolate into -e while showing a node -e example with interpolation placeholders). Rewrite as file-based script reading from process.argv with shell variable pass-through. - browser: wrap browser.close() in its own try/catch inside finally so a close-time crash can't mask the real error via JS exception replacement semantics. Apply to all three script examples. - browser: troubleshooting table for "Timeout exceeded" was training the agent to retry with longer waits instead of surfacing the failure. Now requires reporting first, investigating second. Review sources: pr-review-toolkit code-reviewer, silent-failure-hunter, and Codex rescue agent (gemini rescue failed to return output).
From code-simplifier and comment-analyzer findings:
browser:
- Trim "When to Use" table (redundant with description frontmatter)
- Collapse Step 1 preflight (3 lines vs 14)
- Merge Parse + Clarify into one section
- Remove mid-step restatements of the "no raw interpolation" rule
(kept once in Rules section only)
- Remove credential-restatement after Mode D (covered in Rules)
- Remove dead "Auth state reuse / storageState" rule (no example,
not actionable)
screenshot:
- Collapse Step 1 preflight (same as browser)
- Merge Step 2 (Parse) + Step 3 (Validate) into one "Parse and Validate"
step — validation is part of parsing the URL arg
- Fix --viewport-size example: concrete "1280,800" instead of
"{width},{height}" placeholder (could be left unsubstituted)
- Renumber subsequent steps (old Step 4 → 3, Step 5 → 4)
- Rewrite Rules: full URL validation block (matching other skills) +
close-safely rule to match browser pattern. Drop vague "URL validation
first" bullet; rule is now concrete.
scrape:
- Remove "or as auto-fallback for JS-heavy sites" parenthetical —
contradicted the clean linear fallback chain in Error Handling
- Remove duplicate "record this in the backend-ran report" from Step 2
(error handling section already mandates it)
- Clarify "tell the user once per session" for Playwright install hint
Net: ~65 lines removed across three files, zero functionality loss.
Final: browser 161L, screenshot 105L, scrape 156L = 422L total.
# Conflicts: # README.md
Add screenshot + browser to ROADMAP's skill enumeration so validate-counts CI job passes.
5uck1ess
added a commit
that referenced
this pull request
Apr 10, 2026
Ships the critical fixes a full mega-pr review on #52 uncovered, beyond the binary-shipping bug already addressed in the first commit on this branch. All findings from silent-failure-hunter and code-reviewer marked CRITICAL or HIGH are addressed here. CRITICAL — shell injection in command steps runCommand interpolated {{input}} and {{step-id}} into `sh -c <cmd>`. Any LLM-chosen input or contaminated prior-step output executed shell. Command and gate strings are now literal YAML; values are passed via $DEVKIT_INPUT and $DEVKIT_OUT_<step_id> env vars. The engine validator rejects {{...}} in command/gate strings at parse time with an explicit "shell injection mitigation" error. Both engine.runCommand and mcp.runCommand were affected; both fixed. The 5 self-* workflows (self-test, self-lint, self-improve, self-perf, self-migrate) were updated to use $DEVKIT_INPUT. Regression tests: TestRunWorkflowCommandRejectsInterpolation, TestRunWorkflowCommandEnvInput, TestRunWorkflowCommandEnvPriorOutput. CRITICAL — PreToolUse guard bypassable hooks/devkit-guard.sh blocklist matched 11 hardcoded tool names and missed Task, SlashCommand, TodoWrite, BashOutput, any mcp__* tool, and every future tool Claude Code adds. Claude could trivially drive a command step by calling Task (subagent) or any non-listed tool. Fixed by inverting to an allowlist: during command steps only mcp__devkit* and TodoWrite are permitted; everything else returns exit 2 with a clear diagnostic including the attempted tool name. CRITICAL — stop-guard and guard had opposite fail-closed policies devkit-stop-guard approved on corrupt session.json; devkit-guard blocked. A corrupt state file mid-workflow would silently end the session. Both now fail closed with a diagnostic and handle the TOCTOU race between the -f test and python open() by treating FileNotFoundError as "no session" (matching pre-check intent). HIGH — CLAUDE_PLUGIN_DATA unset silent no-op Both guard hooks exited 0 with no output when the env var was missing. Combined with the binary-shipping bug, enforcement was an invisible no-op. Added stderr warnings so the degraded state is observable. HIGH — loop until / branch when substring false-positives strings.Contains("no failures found", "fail") returned true, silently terminating loops or routing branches incorrectly. Same bug in EvalBranch. Both now use word-boundary matching (grep -w semantics) via a new containsWord helper: the sentinel must be bounded on both sides by a non-alphanumeric/underscore character. Accepts idiomatic forms like "TINY: short fix" and "attempt 2: ALL_PASSING" while rejecting accidental substrings. Regression tests cover both directions. HIGH — completeWorkflow swallowed git.CommitAll errors A failed final commit was logged to stderr only, so the user saw "WORKFLOW COMPLETE" with uncommitted branch work. Now collected into a warnings list and surfaced in the MCP response body, alongside any DB update or state-clear failures. HIGH — runCommand discarded stdout/stderr on non-ExitError failures When the error wasn't an *exec.ExitError (missing binary, permission denied, ctx cancelled), the captured combined stream was thrown away. Now returned alongside the wrapped error so users see the actual cause. Also distinguishes context.DeadlineExceeded (returns exit 124 + "timed out after 5m") from generic execution failure. MEDIUM — YAML typos silently ignored yaml.Unmarshal accepted unknown fields, so "commnd: foo" (missing e) parsed as a no-op step. Now uses NewDecoder(bytes).KnownFields(true); typos fail loudly at parse time. MEDIUM — make sync-version clobbered manual bumps Triggering any make target on a feature branch rewrote plugin.json to the latest git tag, silently undoing manual version bumps. Now only writes when the git tag is strictly higher than the current plugin.json version (semver ordering via sort -V). MEDIUM — pr-gate.sh violated project shell rules Missing `set -euo pipefail`, used /tmp/devkit-pr-gate-done shared across concurrent projects, and arithmetic fallback that silently skipped the gate on any failure. Now strict mode, per-user cooldown file under $XDG_CACHE_HOME/devkit, and explicit numeric-regex guard on the timestamp comparison. Stale docs - src/cmd/status.go, README.md, CONTRIBUTING.md, CHANGELOG.md — replaced `devkit workflow run <name>` with the correct `devkit workflow <name> "<description>"`. There is no `run` subcommand; src/cmd/workflow.go defines `Use: "workflow [name] ..."`. - README.md architecture block — reframed the "binary ships in bin/" claim to describe the wrapper + download flow introduced by this PR. - workflows/pr-ready.yml description — updated to reflect all 8 steps (validate, necessity, lint, test, security, changelog, create-pr, monitor) rather than the stale 5-step list from before pr-monitor was folded in. - hooks/hooks.json top-level description — now mentions the workflow guard and completion guard hooks that were silently added in 2.1.0. - ROADMAP.md — 19 → 22 skills (stale count from before the Playwright skills were added in PR #53). - CHANGELOG.md — new 2.1.2 section documenting every fix in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
screenshotskill — capture web pages as PNG/PDF via Playwright (full-page, viewport, element, device emulation)browserskill — general browser automation (click, fill, navigate, extract, codegen, E2E testing)scrapeskill with Playwright as a new backend for JS-heavy pages (slotted between Jina Reader and Firecrawl)Both new skills are auto-invoked via description triggers, not slash commands. Playwright is an optional dependency — users install it themselves with
npx playwright install chromium. Skills detect availability and provide install instructions if missing.Why Playwright
Researched 12 browser automation tools. Playwright wins on:
@playwright/cli) built for AI agent use casesFull research:
homebase/docs/research/browser-automation-for-devkit.mdTest plan
npx playwright --versiondetection works in all three skillsscrapestill falls back gracefully when Playwright isn't installedscreenshotcaptures viewport, full-page, element, PDF modesbrowsercodegen, form-fill, and data-extraction flows workfile://, private IPs, credential-in-URL attacks