Skip to content

docs(skills): require final app builds#11

Merged
kherembourg merged 2 commits into
mainfrom
fix/build-verification-instructions
May 28, 2026
Merged

docs(skills): require final app builds#11
kherembourg merged 2 commits into
mainfrom
fix/build-verification-instructions

Conversation

@kherembourg

@kherembourg kherembourg commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add Completion Build Gate instructions to purchasely-integrate, purchasely-review, and purchasely-debug
  • Require fixing failed user-app builds and rerunning build/tests before reporting success when these skills are used in mobile app projects
  • Extend the snippet guard to enforce the build-gate section across all skills

Test Plan

  • node scripts/guard-known-bad-snippets.mjs
  • npx --yes skills add . --list
  • GitHub CI: Plugin structure & JSON, Lint Markdown

Notes

  • This repo is an AI plugin, not a mobile app, so there is no app build to run here.

@kherembourg kherembourg force-pushed the fix/build-verification-instructions branch from f736196 to 8e52107 Compare May 28, 2026 10:27
@kherembourg kherembourg requested a review from Copilot May 28, 2026 10:29

Copilot AI 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.

Pull request overview

This PR adds final app build requirements to the Purchasely skills and updates the snippet guard to enforce those instructions for the current skill set.

Changes:

  • Added ## Completion Build Gate guidance to integrate, review, and debug skills.
  • Updated integration verification steps to require build/test reruns before success.
  • Added changelog coverage and fixed the validate workflow’s renamed integrate skill path.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/guard-known-bad-snippets.mjs Enforces presence and wording of the Completion Build Gate in Purchasely skills.
purchasely/skills/purchasely-review/SKILL.md Adds final build gate instructions for review completion and auto-fix flows.
purchasely/skills/purchasely-integrate/SKILL.md Adds build gate instructions and updates final verification steps.
purchasely/skills/purchasely-debug/SKILL.md Adds build gate instructions for debug fixes and diagnosis-only tasks.
CHANGELOG.md Documents the new final app build requirement under Unreleased.
.github/workflows/validate.yml Updates the hard-coded integrate skill path to the renamed directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kherembourg

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented May 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a "Completion Build Gate" instruction section to the purchasely-debug, purchasely-integrate, and purchasely-review skills, requiring agents to build the user's app and fix any failures before reporting success. It also hardens the guard script to enforce the gate's presence, and fixes a stale CI path for purchasely-integrate/SKILL.md.

  • Skill docs: All three SKILL.md files gain a consistent ## Completion Build Gate section; purchasely-integrate additionally renumbers its post-integration checklist to surface the build step first.
  • Guard script: A new pre-walk loop in guard-known-bad-snippets.mjs checks each skill file for the heading and the fix-and-rerun pattern; readFileSync calls in the new loop are not wrapped in try-catch, so a missing file produces an uncaught exception rather than a structured failure entry.
  • CI fix: validate.yml corrects the hardcoded path from purchasely/skills/integrate/SKILL.md to purchasely/skills/purchasely-integrate/SKILL.md.

Confidence Score: 4/5

Safe to merge; all documentation and CI changes are well-scoped and the one code-level gap is non-blocking.

The guard script's new loop calls fs.readFileSync without a try-catch, so renaming or deleting any of the three skill files would cause the guard to crash with a raw Node.js exception rather than emitting the structured failure message the rest of the script produces. This is a minor fragility rather than a present failure, since all three files exist and pass today.

scripts/guard-known-bad-snippets.mjs — the new readFileSync loop lacks error handling

Important Files Changed

Filename Overview
scripts/guard-known-bad-snippets.mjs Adds a pre-walk loop that checks each skill file for the build-gate section; missing error handling around readFileSync could surface as an uncaught exception instead of a structured failure message.
.github/workflows/validate.yml Corrects a stale path in the 'Check plugin folder links' step from purchasely/skills/integrate/SKILL.md to purchasely/skills/purchasely-integrate/SKILL.md.
purchasely/skills/purchasely-debug/SKILL.md Adds the 'Completion Build Gate' section between the PLYError step and Guidelines; wording is consistent with the other skills and satisfies the guard regex.
purchasely/skills/purchasely-integrate/SKILL.md Adds the 'Completion Build Gate' section and renumbers the post-integration verification checklist to make the gate item #1; platform command hints are a useful addition.
purchasely/skills/purchasely-review/SKILL.md Appends the 'Completion Build Gate' section at the end of the file; correctly scopes the fix obligation to Purchasely-related failures only.
CHANGELOG.md Adds a 'Changed' entry under [Unreleased] describing the build gate requirement; format is consistent with existing entries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Skill invoked] --> B[Perform task]
    B --> C{Code changed?}
    C -- Yes --> D[Run Completion Build Gate]
    C -- No --> E{Build available?}
    E -- Yes --> D
    E -- No --> F[Report diagnosis-only outcome]
    D --> G{Build succeeds?}
    G -- No --> H[Fix error and rerun]
    H --> G
    G -- Yes --> I[Include commands and outcomes in response]
    I --> J[Report success]

    subgraph guard [guard-known-bad-snippets.mjs]
        K[Read each SKILL.md] --> L{Contains Completion Build Gate?}
        L -- No --> M[failures.push]
        L -- Yes --> N{Matches fix-and-rerun regex?}
        N -- No --> O[failures.push]
        N -- Yes --> P[Pass]
    end
Loading

Fix All in Claude Code Fix All in Cursor Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
scripts/guard-known-bad-snippets.mjs:36-37
`fs.readFileSync` is called without error handling in the new loop. If any file listed in `requiredBuildGateSkillFiles` is missing or renamed, the script throws an uncaught `ENOENT` exception and dumps a Node.js stack trace instead of pushing a structured entry into `failures[]` and exiting via the normal `process.exit(1)` path. This makes CI output harder to parse and breaks the script's consistent failure-reporting contract.

```suggestion
for (const rel of requiredBuildGateSkillFiles) {
  let content;
  try {
    content = fs.readFileSync(path.join(root, rel), 'utf8');
  } catch {
    failures.push(`${rel} could not be read (file missing or unreadable)`);
    continue;
  }
```

Reviews (1): Last reviewed commit: "docs(skills): require final app builds" | Re-trigger Greptile

Comment thread scripts/guard-known-bad-snippets.mjs Outdated
@kherembourg kherembourg force-pushed the fix/build-verification-instructions branch from 8e52107 to 1dfbd97 Compare May 28, 2026 13:49
@kherembourg kherembourg merged commit e70add3 into main May 28, 2026
2 checks passed
@kherembourg kherembourg deleted the fix/build-verification-instructions branch May 28, 2026 14:07
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