feat: ship an agent skill in the package - #404
Conversation
Agents misuse unlighthouse in ways the docs encourage: the dashboard binary never exits, several config keys are silently ignored, and some doc examples crash the scan. The Skill ships in the npm tarball so agents get the tested behaviour for 0.18.2.
✅ Deploy Preview for unlighthouse-crux-api canceled.
|
|
Checked by hand against a packed 0.18.2 build in a throwaway consumer, scanning a local fixture site (1 to 4 URLs per run):
|
@unlighthouse/cli
@unlighthouse/client
@unlighthouse/core
@unlighthouse/server
unlighthouse
unlighthouse-ci
commit: |
Action required: The pull request must be open or merged into the default branch. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one factual slip in the Skill plus a docs-consistency note.
Reviewed changes
- Ships an agent Skill — adds
packages/unlighthouse/skills/unlighthouse/SKILL.md, written against 0.18.2, covering setup, binary choice, discovery/sampling behaviour, config examples, known traps, output paths and debug tips. - Packages the Skill — adds
"skills"to thefilesarray inpackages/unlighthouse/package.jsonso it lands in the tarball. - Docs links — replaces the
npx skilld add unlighthousetip with a skilld.dev link inREADME.mdanddocs/1.guide/1.getting-started/0.installation.md, and adds a skilld.dev badge to the README.
I cross-checked the Skill's factual claims against the source and they hold up: unlighthouse-ci deleting outputPath, ci.buildStatic config being overridden, the throttle/cache traps, the 50-URL sitemap threshold, --cookies/--extra-headers splitting, the authenticate signature, the createUnlighthouse()-without-provider throw, the cluster.close() throw and the include-skipping-/ hang. One claim does not.
ℹ️ The docs the Skill calls out are still wrong
The Traps section tells agents that authenticate({ page }) and report.score.performance come "from the docs" and crash the scan, but the docs themselves are left untouched. Agents retrieve those pages directly, so the Skill's correction only helps the subset that reads the Skill first. Since these are documentation fixes (not package bugs), aligning the source docs is cheap and doesn't force the Skill's trap lines to be removed.
Technical details
# Docs out of sync with the Skill's corrected examples
## Affected sites
- `docs/1.guide/guides/authentication.md:136` — `authenticate({ page })` crashes; hook receives the `Page` directly.
- `docs/1.guide/guides/0.config.md:172-173` — `report.score.performance` crashes; score lives at `report.report.score` and categories are an array.
- `docs/1.guide/guides/dynamic-sampling.md:66` and `docs/1.guide/recipes/large-sites.md:42` — say the default is 5; `constants.ts:184` is 8.
- `docs/1.guide/recipes/improving-accuracy.md:43`, `docs/1.guide/guides/0.config.md:103`, `docs/1.guide/1.getting-started/0.unlighthouse-cli.md:107`, `docs/1.guide/1.getting-started/0.installation.md:110` — say samples are averaged; the median run is used.
## Required outcome
- The examples users and agents copy should not crash, and stated defaults should match 0.18.2.
## Open questions for the human
- Is fixing these docs in scope for this PR, or deliberately deferred to keep it Skill-only?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
|
||
| ## Config | ||
|
|
||
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page, averaged). |
There was a problem hiding this comment.
With scanner.samples > 1 the scan does not average: runLighthouseTask picks a median run via computeMedianRun(samples) (packages/core/src/puppeteer/tasks/lighthouse.ts:187-190). "averaged" here would mislead an agent reading scores.
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page, averaged). | |
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page; with more than one, the median run is used, not an average). |
There was a problem hiding this comment.
Important
The fix/published-types merge fixed the package bugs that most of the Skill's Traps section documents. Shipping the Skill as-is would tell agents to work around behaviour that no longer exists in the code it ships with — see the inline comment on the Traps section.
Reviewed changes
- Merged
origin/fix/published-types— retargets the branch atfix/published-typesand pulls its post-0.18.2 fix stack into the branch (safe output clearing, CLI flag/config merge, scan hang + close fixes, docs corrections, published-types compile). The Skill, README tip, installation-doc tip, andfilesentry are unchanged from the prior review.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
The stacked fix PRs remove most traps: output folders, flag merging, throttle, hangs, and the programmatic API. The cookies trap is new and was checked against a local fixture.
There was a problem hiding this comment.
ℹ️ No critical issues — the incremental commit cleanly brings the Skill's
Trapsin line with the shipped code. One minor factual note onscanner.samplesis carried over below.
Reviewed changes
- Rewrote the Skill's
Traps— the stale 0.18.2 workarounds are replaced with the fixed behaviour they ship with: output-folder ownership (.unlighthouse-output/Refusing to clear ...),scanner.throttle: false, configci.buildStatic, the first-separator header split, theinclude-skips-/exit 1, and the cookie/extraHeadersnote. - Refreshed behaviour and config copy — flag-vs-config precedence,
unlighthouse-ciexit-code meanings, default simulated throttling, static build via config, the no-provider programmatic example, and the typedtask-completecategories. - Dropped the 0.18.2 pin — the Skill now states it is tested against the release that ships it.
I checked each new claim against the source and they hold: outputDir.ts refuses a foreign folder and writes the marker, resolveThrottling maps throttle: false to throttlingMethod: 'provided', pickCiOptions only sets passed flags (so config ci.buildStatic survives), unlighthouse.ts:349 emits No routes left to scan and both binaries exit 1, resolveConfig.ts:174 maps extraHeaders into lighthouseOptions.extraHeaders (so the Cookie-header workaround is sound), and worker.ts:308 fires task-complete unawaited so a throwing hook terminates unlighthouse-ci with exit 1.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
|
||
| ## Config | ||
|
|
||
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page, averaged), `scanner.throttle` (`true`). |
There was a problem hiding this comment.
With scanner.samples > 1 the scan does not average: runLighthouseTask picks a median run via computeMedianRun(samples) (packages/core/src/puppeteer/tasks/lighthouse.ts:187-190). "averaged" here would mislead an agent reading scores.
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page, averaged), `scanner.throttle` (`true`). | |
| - `scanner.device` (`'mobile'`), `scanner.samples` (1, runs per page; with more than one, the median run is used), `scanner.throttle` (`true`). |
There was a problem hiding this comment.
✅ No new issues found in the new commit.
Reviewed changes — reviewed the SKILL.md guidance added since the b75c751 review, which documents three behaviours fixed on the fix/request-identity base.
- Documented local vs. remote throttling — the Skill now says throttling is simulated for a remote
siteand off for a local one (localhost,127.0.0.1), with an explicitscanner.throttleor--throttlewinning (resolveConfig.ts:28reads the user-supplied value, so the defaulttrueinconstants.tsno longer forces it). - Documented redirect following — the Skill states the scan adopts the redirect target when
siteredirects (http→https, apex→www), matchingvalidateHost(packages/cli/src/util.ts:27-29), which both binaries call. - Corrected the cookie guidance — the
Trapsentry claiming cookies never reach the Lighthouse request is gone; the auth section now says the hook's cookies go with every request, including Lighthouse, matchingresolveRequestHeadersbeing spread intolighthouseOptions.extraHeadersper task (packages/core/src/puppeteer/tasks/lighthouse.ts:150).
One prior minor wording note on scanner.samples ("averaged") remains open and is not re-raised here.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
harlan-github-agent
Bot
added
harlan-agent-running
🤖 MERGED
GitHub merged this pull request. No material findings were recorded. The pull request closed. |
harlan-github-agent
Bot
added
harlan-agent-ready
🤖 READY · 90/100
|
harlan-github-agent
Bot
removed
the
harlan-agent-ready
1 participant
Stacked on #411.
❓ Type of change
📚 Description
Agents that drive unlighthouse keep tripping on the same things: they run
unlighthousein a script and it never exits, they copy regex into--exclude-urls, and they point--output-pathsomewhere that holds other files.Adds a Skill at
packages/unlighthouse/skills/unlighthouse/SKILL.mdand ships it in theunlighthousetarball. The README and the installation doc swap the oldnpx skilld addtip for the skilld.dev link and badge.Only
unlighthouseships the Skill. It carries both binaries; the@unlighthouse/*packages have no separate users.Writing the Skill turned up 21 package bugs. The fixes sit below this PR (#409, #405, #406, #407, #408, #411), so the Skill describes the fixed behaviour and should merge with them.