Skip to content

feat(cli): add proof install-skill so docs stay copy-pasteable - #267

Merged
tonyketcham merged 5 commits into
mainfrom
toeknee/proof-install-skill-8475
Aug 24, 2026
Merged

feat(cli): add proof install-skill so docs stay copy-pasteable#267
tonyketcham merged 5 commits into
mainfrom
toeknee/proof-install-skill-8475

Conversation

@tonyketcham

Copy link
Copy Markdown
Collaborator

Summary of changes

End-user READMEs and the Proof setup skill previously asked people to substitute a git tag and package version, or they hardcoded a release. Those snippets rot on every publish.

This adds flatbread proof install-skill. One copy-paste command, npx --yes flatbread@latest proof install-skill, downloads the latest CLI, pins that exact flatbread version as a devDependency, and copies the Proof skill that shipped with it. @latest only chooses which CLI to run.

The installer detects npm, pnpm, Yarn, or Bun from packageManager or a single lockfile, skips this monorepo, and refuses conflicting lockfiles. Pack checks now reject version placeholders in skill docs.

Users copy one command. Maintainers still keep release.json in lockstep for pack identity.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • I added doc comments to any new public exports, and inline comments to any hard-to-understand areas
  • My changes generate no new console errors locally
  • If applicable, try to include a test that fails without this PR but passes with it

Does this introduce any non-backwards compatible changes?

  • Yes
  • No

Does this include any user config changes?

  • Yes
    • If so, I have updated the relevant areas of documentation
  • No

The documented install path changes from a versioned snippet to npx --yes flatbread@latest proof install-skill. Existing projects keep working.

Open in Web Open in Cursor 

End-user READMEs and the Proof setup skill previously asked people to
substitute a git tag and package version, or they hardcoded a release.
Those snippets rot on every publish.

Add `flatbread proof install-skill`. One copy-paste command,
`npx --yes flatbread@latest proof install-skill`, downloads the latest
CLI, pins that exact flatbread version as a devDependency, and copies
the Proof skill that shipped with it. `@latest` only chooses which CLI
to run.

The installer detects npm, pnpm, Yarn, or Bun from packageManager or a
single lockfile, skips this monorepo, and refuses conflicting
lockfiles. Pack checks now reject version placeholders in skill docs.

Users copy one command. Maintainers still keep release.json in lockstep
for pack identity.

Change-Id: I52a2ca4fc9f1b8504de6c769bc4021d96557922d

Co-authored-by: Erika Ruth Witt <erikawitt@users.noreply.github.com>
@tonyketcham
tonyketcham marked this pull request as ready for review August 24, 2026 00:26
CI sets FLATBREAD_CI=true. The bin used to look for dist under
cwd/node_modules/flatbread, so spawned install-skill tests in a temp
directory printed "Flatbread's CLI is not available" instead of JSON.

Check dist next to the bin file. Keep the spawn test on FLATBREAD_CI so
this cannot pass only on a local machine.

Change-Id: I8db6cb72827b35b2a16a3459385707e629a6d739

Co-authored-by: Tony <tonyketcham@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bb7a502. Configure here.

Comment thread packages/flatbread/src/cli/installSkill.ts
@mergify

mergify Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

REQUEST_CHANGES

flatbread proof install-skill is the right seam: one copy-paste command, pin the running CLI, copy the skill that shipped with it. The Yarn path is not.

Prior notes closed: none (first review). Skip resolve.

What must change
A Yarn 1 project (yarn.lock, or packageManager: yarn@1.22.22) plans yarn dlx after yarn add has already rewritten the manifest. Yarn Classic has no dlx. managerFromField keeps only the name, so yarn@1.x is treated as Berry. setup.md says Yarn works.

Smallest fix: keep yarn add --dev --exact for the package; run the skill step with npx --yes skills (or route Yarn 1 there). Add a planProofSkillInstall('yarn') test so Classic cannot land on yarn dlx again.

Domain verdicts

  • architecture — REQUEST_CHANGES (Yarn Classic vs documented Yarn)
  • quality — COMMENT (launcher table / duplicated @latest prose; not blocking)
  • proof-journal — APPROVE (no new record; unique signal already lives in CONTRIBUTING, CHANGELOG, pack check, and tests)
  • correctness — REQUEST_CHANGES (Yarn dlx; residual Windows bunx.cmd)

Held

  • Unpinned skills CLI is the same risk as the old npx skills add path. Not new.
  • Workspace skip by name === '@flatbread/monorepo' is good enough for this repo.
  • Defaulting to npm with no lockfile is the usual default.

Models
architecture / proof / correctness: inherit (cursor-grok-4.6-high-fast)
quality: composer-2.5

Open in Web View Automation 

Sent by Cursor Automation: Flatbread PR Review

Comment thread packages/flatbread/src/cli/installSkill.ts Outdated
Comment thread packages/flatbread/src/cli/installSkill.ts Outdated
Node 20.12+ raises EINVAL when execFile runs a .cmd shim without a
shell. The installer used to append .cmd and call execFile directly, so
proof install-skill could not run npm, pnpm, Yarn, or Bun on Windows.

Run those commands through ComSpec on Windows, quote arguments for
cmd.exe, and keep a direct execFile on Unix. Tests cover quoting and the
Windows argv shape.

Change-Id: I0ba1b5d334b7551f71df608028653742538d207e

Co-authored-by: Tony <tonyketcham@users.noreply.github.com>
Comment thread .agents/skills/proof/setup.md Outdated
Yarn Classic has no `dlx`. The installer already runs `yarn add` to pin
flatbread, then planned `yarn dlx skills`, which fails after rewriting
package.json.

Keep `yarn add --dev --exact` for the package. Copy the skill with
`npx --yes skills`, which exists wherever this CLI runs. Berry still
works. A plan test locks the Yarn path off `dlx`.

Change-Id: Icfc1d03051f0be074b0f6a57df1b558bba084156

Co-authored-by: Tony <tonyketcham@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

REQUEST_CHANGES

flatbread proof install-skill is the right seam, and the last two installer bugs are closed. The remaining block is the file that command copies.

Prior notes closed

  • Yarn Classic yarn dlx — fully addressed (npx --yes skills + plan test).
  • Windows bunx.cmd — fully addressed (cmd.exe / ComSpec, no .cmd suffix).

What must change
packages/proof/skills/proof/setup.md is the consumer contract. This PR added pnpm skills:sync to the dogfooding paragraph. After copy, a user-project agent can skip npm, look for a workspace flatbread binary, or run pnpm skills:sync. That matches Tony's comment.

Smallest fix: delete the opener (generated .agents/ projection) and the dogfooding paragraph from this single source. Keep maintainer steps in CONTRIBUTING.md, packages/proof/README.md, and the skipped_workspace CLI message. Extend verifyPackPayload to reject pnpm skills:sync and “dogfooding this monorepo” the same way it already rejects the monorepo CLI path. Do not add a second skill source or a pack-time rewrite.

Domain verdicts

  • architecture — REQUEST_CHANGES (shipped skill still teaches Flatbread-only workflow)
  • quality — COMMENT (same docs defect; other duplication is not blocking)
  • proof-journal — APPROVE (no new record; unique signal already lives in CONTRIBUTING, CHANGELOG, pack check, and tests)
  • correctness — COMMENT (Yarn/Windows closed; residual is the shipped-skill agent misread)

Held

  • Unpinned skills CLI is the same risk as the old npx skills add path. Not new.
  • Workspace skip by name === '@flatbread/monorepo' is good enough for this repo.
  • Defaulting to npm with no lockfile is the usual default.

Models
architecture / proof / correctness: inherit (latest Cursor Grok)
quality: composer-2.5

Open in Web View Automation 

Sent by Cursor Automation: Flatbread PR Review

Comment thread packages/proof/skills/proof/setup.md Outdated
Comment thread packages/proof/scripts/pack-skills.mjs
Cloud and fresh agents Read setup.md before any sync or install
script, so maintainer asides cannot live in the canonical payload.
Pack-check now rejects those phrases. Workspace skill edits stay on
AGENTS.md, CONTRIBUTING.md, and the install-skill skip path.

Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I0e947b1f8eeb7cfb2f9e2e5a76c2ca6f40ea9c3e

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

091f8b7 closes the last block. The file install-skill copies is consumer-only again.

Prior notes closed

  • Shipped setup.md taught workspace flatbread and pnpm skills:sync — fully addressed (those blocks left the canonical skill; maintainer steps live in AGENTS.md, CONTRIBUTING.md, and the skipped_workspace CLI message).
  • Pack-check allowed those phrases — fully addressed (forbiddenSkillSubstrings plus a wrap test).

Domain verdicts

  • architecture — APPROVE (one skill source; no pack-time rewrite; pack-check is the gate)
  • quality — COMMENT (dead version option and repeated @latest prose; not blocking)
  • proof-journal — COMMENT (Constraint scores 4/4 on load-order: agents Read setup.md before any pack or sync. Decision is 3/4: alternatives and reversal sit in canvas proof-skill-audience-split, not in the record. Siblings stay proposed. install-skill itself still needs no extra record.)
  • correctness — APPROVE (Yarn Classic, Windows cmd.exe, and CI bin path stay closed; both setup.md copies match)

Held

  • The pack denylist is four known phrases. A new aside that avoids them can still ship.
  • Parent Decision dec-distribute-the-effort-graph-as-a-versioned-agent--8as4sybr34zcqyqh still describes git-tag install. Update later; do not block this PR.
  • Unpinned skills CLI, workspace skip by package name, default npm with no lockfile, and Windows %VAR% expansion stay residual.

Models
architecture / proof / correctness: inherit (latest Cursor Grok)
quality: composer-2.5

Open in Web View Automation 

Sent by Cursor Automation: Flatbread PR Review

@tonyketcham
tonyketcham merged commit 7129229 into main Aug 24, 2026
22 checks passed
@tonyketcham
tonyketcham deleted the toeknee/proof-install-skill-8475 branch August 24, 2026 19:32
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.

2 participants