Repository navigation
ci: discover the test matrix with pgpm ls; smoke-test templates; template DX cleanup - #53
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟠 1 high 💬 Inline comments (1)
PR #53 adds One high-severity defect was found in the new discovery logic.
The Reviewed commit: 3dc8080 |
There was a problem hiding this comment.
Adds a smoke-test CI workflow and converts the pgpm workspace test job to dynamic package discovery, gating the test matrix on a has_packages output.
Key findings
- 🟠 Empty
packages/abortsdiscoverstep — ci.yml:21
| packages=$(ls -d packages/*/ 2>/dev/null | xargs -n1 basename 2>/dev/null | jq -Rsc 'split("\n")[:-1]') | ||
| echo "packages=${packages:-[]}" >> "$GITHUB_OUTPUT" | ||
| echo "has_packages=$([ "${packages:-[]}" != "[]" ] && echo true || echo false)" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🟠 bug · high
Empty packages/ aborts discover step
The packages=$(ls -d packages/*/ ...) pipeline aborts the step whenever packages/ is missing or empty (pgpm/workspace/.github/workflows/ci.yml:21): 2>/dev/null hides the ls error but not its non-zero exit code, and under GitHub Actions' default bash -e -o pipefail runner shell that failed command substitution terminates the step before the packages=[] fallback and has_packages guard on the following lines run. Any freshly generated workspace (which ships no packages/ directory) therefore gets a red discover job, and the has_packages output added to make the empty case harmless is unreachable exactly when it is needed.
📋 Prompt for AI Agents
In pgpm/workspace/.github/workflows/ci.yml line 21, the packages=$(ls -d packages/*/ 2>/dev/null | xargs -n1 basename 2>/dev/null | jq -Rsc 'split("\n")[:-1]') assignment aborts the discover step under bash -e -o pipefail when packages/ is missing or empty, because ls returns non-zero. Append || packages='[]' to that assignment (keeping lines 22-23 unchanged) so an empty packages/ yields packages=[] and has_packages=false, letting the downstream if: needs.discover.outputs.has_packages == 'true' guard skip the test job instead of failing the whole run.
pgpm ls; smoke-test templates on PRs
… of pinning an inventory
pgpm ls; smoke-test templates on PRspgpm ls; smoke-test templates; template DX cleanup
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Summary
Fixes the generated workspace CI and a set of DX defects found by scaffolding the templates with published
pgpm@5.33.0.Generated CI (
pgpm/workspace/.github/workflows/ci.yml)matrix: package: [package-a, package-b]is replaced by adiscoverjob:packages=$(pnpm exec pgpm ls --paths --json)(pgpm lslanded in pgpm 5.33.0; the template now requirespgpm ^5.33.0). Paths are workspace-relative, so the test step iscd ./${{ matrix.package }} && pnpm test; the test job is skipped when the list is[].PGPM_VERSION: 5.30.6+npm install -g pgpm: both jobs use the workspace's own pgpm viapnpm exec pgpm, so discovery, tune, and admin-users all run the same CLI the workspace pins.Supply-chain policy — no more pinned inventory
A fresh scaffold's
pnpm installfailed withERR_PNPM_NO_MATURE_MATCHING_VERSION graphile-storage-registry@0.2.0: the template exempted first-party names via@constructive-io/pnpm-policy@0.3.2's inventory, which predates that package. Both workspace templates now drop the two pinned inventory devDependencies and list first-party names directly, using thepackages:key frompnpm-policy@0.6.0(constructive-io/dev-utils#131):pnpm-workspace.yamlis regenerated with the publishedpnpm-policy@0.6.0and committed;pnpm-policyis bumped to^0.6.0. Upstream Graphile packages not published by us (@dataplan/*,@graphile/lru,@graphile-contrib/*) are no longer exempt.Other template fixes
cd packages/your-module→cd "$(pnpm exec pgpm ls --paths | head -1)".lintis check-only (eslint .);--fixmoved tolint:fix. Rootpnpm lintno longer rewrites files.versionscript no longer runsgit add -A.Repo CI (
.github/workflows/ci.yml)New
smokejob on PRs:pgpm init workspace+pgpm initwith the published pgpm, thenpnpm install --lockfile-only— deliberately without a release-age override, so a template that pins something its own policy quarantines fails here instead of on the user's machine.Link to Devin session: https://app.devin.ai/sessions/dd4055c50bcf4b1e8e604211038aaa82
Open in Devin Desktop: https://app.devin.ai/desktop/session/dd4055c50bcf4b1e8e604211038aaa82?variant=devin
Requested by: @pyramation