Skip to content

Merge v4-beta to v4 (2026-05-29)#174

Merged
PaulNewling merged 5 commits into
v4from
merge-2026-05-29
May 29, 2026
Merged

Merge v4-beta to v4 (2026-05-29)#174
PaulNewling merged 5 commits into
v4from
merge-2026-05-29

Conversation

@PaulNewling
Copy link
Copy Markdown
Collaborator

@PaulNewling PaulNewling commented May 29, 2026

Merge v4-beta into v4

Canary tested:
platforma-open/sequence-properties#14 - Not over strict
platforma-open/sequence-properties#15 - Still fail when module missing

Greptile Summary

This PR refines the catalog-bump coverage check so that only runtime consumers (dependencies and peerDependencies) are required to add a changeset entry when a pnpm-workspace.yaml catalog key changes. Previously the check also flagged packages that consumed the entry solely in devDependencies (e.g. build tools like block-tools or tengo-builder), generating false positives for UI-only or unrelated changes.

  • check-coverage.sh: narrows the jq filter from four dependency sections to dependencies + peerDependencies only, with a new inline comment and updated header note explaining the rationale.
  • coverage.bats + pkg-dev fixture: adds a new test package consuming is-string only as a devDependency and a matching test case that asserts it is not flagged when the catalog entry bumps.
  • action.yaml: description updated to document the runtime-only scoping.

Confidence Score: 4/5

Safe to merge; the core devDependency exclusion is correct and well-tested, with one unaddressed edge case around optionalDependencies.

The devDependency exclusion is intentional, clearly motivated, and backed by a new test. The only concern is that optionalDependencies was also silently dropped from the catalog-consumer scan — those entries appear in the published package.json and affect consumers at runtime, so removing them from the check could cause the script to miss legitimate coverage gaps.

actions/changeset/check-coverage/check-coverage.sh — specifically the jq filter on line 200 and whether the optionalDependencies exclusion is intentional.

Important Files Changed

Filename Overview
actions/changeset/check-coverage/check-coverage.sh Narrows catalog-consumer detection to dependencies and peerDependencies only, dropping devDependencies and optionalDependencies; optionalDependencies exclusion is unexplained and potentially unintended
actions/changeset/check-coverage/test/coverage.bats Adds a test case covering the new devDependency exclusion behaviour; test logic is correct and well-commented
actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-dev/package.json New test fixture package declaring is-string only in devDependencies; correctly models the build-tool devDep scenario
actions/changeset/check-coverage/action.yaml Description updated to document the runtime-only scoping of catalog-bump detection
actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-dev/index.js Trivial fixture JS file; comment correctly explains its purpose

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pnpm-workspace.yaml changed?] -->|No| Z[Skip catalog check]
    A -->|Yes| B[Extract changed catalog keys via yq structural parse]
    B --> C{Any keys changed?}
    C -->|No| Z
    C -->|Yes| D[For each catalog key / For each workspace package]
    D --> E{Package private?}
    E -->|Yes| D
    E -->|No| F[Read package.json]
    F --> G{Key in dependencies or peerDependencies with catalog: value?}
    G -->|No — devDeps and optionalDeps now excluded| D
    G -->|Yes| H[Add to required_bump_set]
    H --> D
    D --> I[Compare required vs bumped set]
    I --> J{Any missing?}
    J -->|No| K[Exit 0 ✓]
    J -->|Yes| L[Exit 1 — list missing packages]
Loading

Comments Outside Diff (1)

  1. actions/changeset/check-coverage/check-coverage.sh, line 199-204 (link)

    P2 optionalDependencies silently dropped from the coverage check. The stated rationale — build-tool bumps in devDependencies leave the published artifact unchanged — does not apply to optionalDependencies. Those entries are emitted into the published package.json and installed by consumers at runtime, so a catalog bump there does change what the package ships. Before this PR the check caught them; now it silently skips them with no explanation in the comment or the header note.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: actions/changeset/check-coverage/check-coverage.sh
    Line: 199-204
    
    Comment:
    `optionalDependencies` silently dropped from the coverage check. The stated rationale — build-tool bumps in `devDependencies` leave the published artifact unchanged — does not apply to `optionalDependencies`. Those entries are emitted into the published `package.json` and installed by consumers at runtime, so a catalog bump there does change what the package ships. Before this PR the check caught them; now it silently skips them with no explanation in the comment or the header note.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
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
actions/changeset/check-coverage/check-coverage.sh:199-204
`optionalDependencies` silently dropped from the coverage check. The stated rationale — build-tool bumps in `devDependencies` leave the published artifact unchanged — does not apply to `optionalDependencies`. Those entries are emitted into the published `package.json` and installed by consumers at runtime, so a catalog bump there does change what the package ships. Before this PR the check caught them; now it silently skips them with no explanation in the comment or the header note.

```suggestion
        if jq -e --arg k "${key}" '
            [.dependencies?, .peerDependencies?, .optionalDependencies?]
            | map(select(. != null) | to_entries) | add // []
            | map(select(.key == $k and ((.value // "") | startswith("catalog:"))))
            | length > 0
          ' "${pj}" >/dev/null 2>&1; then
```

Reviews (1): Last reviewed commit: "Merge v4-beta into v4" | Re-trigger Greptile

Section 2b required a changeset for every workspace package consuming a
bumped catalog key, scanning all dependency sections. That flagged
packages whose only "change" was a build-tool catalog bump in a
devDependency (e.g. @platforma-sdk/block-tools in a block model,
@platforma-sdk/tengo-builder in a workflow) even though the package's own
files were untouched and `pnpm changeset` does not flag them.

Restrict the catalog-consumer scan to runtime sections (dependencies,
peerDependencies). A runtime catalog dep bump still requires a release;
build-only tooling no longer produces false positives.

Adds a bats case + pkg-dev fixture covering the devDependency path.
Section 2b required a changeset for every workspace package consuming a
bumped catalog key, scanning all dependency sections. That flagged
packages whose only "change" was a build-tool catalog bump in a
devDependency (e.g. @platforma-sdk/block-tools in a block model,
@platforma-sdk/tengo-builder in a workflow) even though the package's own
files were untouched and `pnpm changeset` does not flag them.

Restrict the catalog-consumer scan to runtime sections (dependencies,
peerDependencies). A runtime catalog dep bump still requires a release;
build-only tooling no longer produces false positives.

Adds a bats case + pkg-dev fixture covering the devDependency path.
@PaulNewling PaulNewling merged commit f317484 into v4 May 29, 2026
2 checks passed
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