Skip to content

test(pi): validate package integrity in CI (PF-3852) - #43

Open
Zechereh wants to merge 1 commit into
zach/pf-3852-pi-publishfrom
zach/pf-3852-pi-tests
Open

test(pi): validate package integrity in CI (PF-3852)#43
Zechereh wants to merge 1 commit into
zach/pf-3852-pi-publishfrom
zach/pf-3852-pi-tests

Conversation

@Zechereh

Copy link
Copy Markdown
Contributor

Stacked on #42. Answers "is there a way to test that this package works?"

The package ships no code, so nothing here fails loudly. That is the whole problem:

  • a malformed skill description → pi silently does not load that skill
  • a dropped files entry → the tarball publishes and installs, just with skills missing
  • a tool the skills teach but that is not in directTools → the model calls something never registered

None of it surfaces until a user hits it. These checks make each one fail at PR time instead.

pi/test/validate.mjs

Dependency-free, runs locally as node test/validate.mjs, excluded from the tarball.

Check Catches
manifest pi.mcp/pi.skills pointing at missing paths; a dropped pi-package keyword, which is the entire gallery discovery mechanism
mcp type: "http" creeping back (the adapter has no such field); the ${VAR:-} interpolation form, which ships as a literal header value; empty or duplicated directTools
skills the frontmatter name/description rules pi enforces, name↔directory mismatch, duplicate names, and skill-relative .md references that do not resolve
skills-vs-directTools a tool taught by the skills but not registered
files a missing files entry, or test/ leaking into the tarball

Every check is negative-tested — each was made to fire by introducing exactly the regression it guards, then reverted.

It found two real bugs, both fixed in the branches below

  1. batch_status / batch_cancel were taught by the automation skill but missing from directTools — a model following that skill would have called tools that never registered. Fixed in feat(pi): package scaffold and MCP registration (PF-3852) #40 (now 10 tools).
  2. tinyfish-authenticated cited references/anti-bot.md and references/goals.md, which live under tinyfish-automation and so did not resolve from where that skill sits. Fixed in feat(pi): TinyFish skills and README (PF-3852) #41 to use resolvable ../tinyfish-automation/... paths. grok/ still has this one — worth a separate look, not touched here.

That is two bugs in a package I had already verified by hand in a live pi session, which is the argument for the checks existing.

pi-ci.yml

Two jobs.

validate — runs the validator plus tarball verification on PRs. pi-publish.yml already verifies the tarball, but only on push to main, so nothing was validating pi/ on the PR that breaks it.

install — installs pi and pi-mcp-adapter the way a user does, registers the package, and asserts the adapter resolves server tiny-fish_pi__tinyfish with directTools intact. That is the contract the skills are written against, and it lives in someone else's package: if the adapter changes how it derives server names, that should break our build rather than our users. No model, no credentials, no network calls to us — deterministic and free. Verified locally by running the job's exact commands.

Deliberately not covered

Validating directTools names against the live MCP server's tools/list would catch product-side drift — a server-side rename silently stops a tool registering. It needs a TinyFish API key, and this repo has no such secret (ux-labs uses MINO_PROD_API_KEY). Worth a scheduled job later; flagging rather than half-building it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP

The package ships no code, so nothing here fails loudly. A malformed skill
description means pi silently does not load that skill; a dropped `files` entry
means the tarball publishes and installs with skills missing; a tool the skills
teach but that is not in `directTools` means the model calls something that was
never registered. All three are invisible until a user hits them.

`pi/test/validate.mjs` — dependency-free, runs locally as `node test/validate.mjs`:

| Check | Catches |
|---|---|
| manifest | `pi.mcp`/`pi.skills` pointing at missing paths; a dropped `pi-package` keyword, which is the entire gallery discovery mechanism |
| mcp | `type: "http"` creeping back (the adapter has no such field); the `${VAR:-}` interpolation form, which ships as a literal header; empty or duplicated `directTools` |
| skills | frontmatter name/description rules pi enforces, name/directory mismatch, duplicate names, and skill-relative `.md` references that do not resolve |
| skills-vs-directTools | a tool taught by the skills but not registered |
| files | a missing `files` entry, or `test/` leaking into the tarball |

Written after the check found two real bugs in the branch below it, both now
fixed there: `batch_status`/`batch_cancel` were taught by the automation skill
but absent from `directTools`, and `tinyfish-authenticated` cited
`references/anti-bot.md` and `references/goals.md`, which live under
`tinyfish-automation` and so did not resolve from where that skill sits. The
second is inherited from `grok/`, which still has it.

Every check is negative-tested: each one was made to fire by introducing exactly
the regression it guards.

`pi-ci.yml` runs the validator plus tarball verification on PRs — pi-publish.yml
already checks the tarball, but only on push to main, so nothing was validating
`pi/` on the PR that breaks it.

The second job installs pi and `pi-mcp-adapter` the way a user does, registers
the package, and asserts the adapter resolves server `tiny-fish_pi__tinyfish`
with `directTools` intact. That is the contract the skills are written against
and it lives in someone else's package, so a silent upstream change to the name
derivation should break our build rather than our users. No model, no
credentials, no network calls to us — deterministic and free.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBf5rnVYQYcxE8bLjfQuUP
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 4e6a580b-2b97-421b-8bd1-d5981b9a79f8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@Zechereh

Copy link
Copy Markdown
Contributor Author

Note on why Pi CI shows no run here yet.

Every CI workflow in this repo filters on pull_request: branches: [main], which matches the base branch — hermes-ci.yml, langchain-ci.yml, google-adk-ci.yml and plugin-manifests-ci.yml all do. This PR's base is zach/pf-3852-pi-publish, so pi-ci.yml will first execute when the stack lands and this retargets main. Same reason #41 and #42 show only TruffleHog, while #40 (base main) picked up pi-versions-match.

Not a problem with the workflow, but it does mean GitHub hasn't executed it yet. I ran both jobs' exact commands locally instead:

$ node test/validate.mjs
  ok  manifest
  ok  mcp
  ok  skills (5 valid)
  ok  skills-vs-directTools (10 taught, all registered)
  ok  files
All checks passed (5).

$ export PI_CODING_AGENT_DIR="$(mktemp -d)"
$ pi install ./pi && pi install npm:pi-mcp-adapter
$ node -e "...loadPackageMcpConfigs..."
PASS: adapter resolved tiny-fish_pi__tinyfish with 10 direct tools

Worth a look at the first green Pi CI run once #40#42 merge, rather than assuming from this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant