Skip to content

feat(ci): add workspace integrity, prerelease guard, and export smoke gates#115

Closed
diberry wants to merge 3 commits intodevfrom
squad/ci-health-gates
Closed

feat(ci): add workspace integrity, prerelease guard, and export smoke gates#115
diberry wants to merge 3 commits intodevfrom
squad/ci-health-gates

Conversation

@diberry
Copy link
Copy Markdown
Owner

@diberry diberry commented Mar 29, 2026

Summary

Adds 3 new CI validation gates to \squad-ci.yml\ motivated by the PR bradygaster#640 prerelease version incident where npm silently resolved a stale published SDK instead of the local workspace copy.

Gate 1: \workspace-integrity\

  • Verifies lockfile has no stale registry entries for workspace packages
  • Catches: npm resolving published registry copy instead of local workspace symlink
  • Cost: Zero-install (reads \package-lock.json\ only)
  • Flag: \�ars.SQUAD_WORKSPACE_CHECK\ / label: \skip-workspace-check\

Gate 2: \prerelease-version-guard\

  • Blocks prerelease version suffixes (-build, -alpha, -beta, -rc) from merging to dev/main
  • Catches: Forgotten prerelease suffixes that break semver range resolution
  • Cost: Zero-install (reads \packages/*/package.json\ only)
  • Flag: \�ars.SQUAD_VERSION_CHECK\ / label: \skip-version-check\

Gate 3: \�xport-smoke-test\

  • Verifies all subpath exports resolve to built artifacts after SDK build
  • Catches: Missing \dist/\ files for declared exports (complements \�xports-map-check)
  • Cost: Lightweight install + SDK build (~30s), only runs when SDK files change
  • Flag: \�ars.SQUAD_EXPORT_SMOKE\ / label: \skip-export-smoke\

Design

  • All gates follow existing CI patterns: feature flags, skip labels, three-dot diff, ::error::\ annotations
  • Gates 1 & 2 are zero-install (fast, no
    pm ci\ needed)
  • Gate 3 only triggers on SDK source/config changes and does its own lightweight build

Closes #114

… gates

Adds 3 new CI validation gates motivated by the PR bradygaster#640 prerelease
version incident where npm silently resolved a stale registry SDK.

- workspace-integrity: verifies lockfile has no stale registry entries
  for workspace packages (zero-install, reads lockfile only)
- prerelease-version-guard: blocks prerelease version suffixes from
  merging to dev/main (zero-install, reads package.json only)
- export-smoke-test: verifies all subpath exports resolve to built
  artifacts after SDK build (lightweight install+build)

All gates follow existing patterns: feature flags (vars.SQUAD_*),
skip labels, three-dot diff for change detection, ::error:: annotations.

Closes #114

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

📣 PAO Review: PR #115 CI Gates


1. YAML Comments Quality ✅

The header blocks for each gate are excellent:

Minor suggestion: In the feature flag inline comments, you explain how the logic works ([ "" = "false" ] evaluates to false), which is developer-friendly. Consistent across all 3 gates. 👍


2. Error Message UX ✅

Strengths:

  • All 3 gates follow a consistent error format:
    1. Problem statement (uppercase, ::error::)
    2. Blank line
    3. Specific violations (listed clearly)
    4. Blank line
    5. Fix instructions
    6. Skip alternative

Examples:

  • workspace-integrity: Explains the root cause ("version mismatch between workspace packages") and the fix ("ensure all workspace package version ranges match")
  • prerelease-version-guard: Lists each violating package with its version and path — very scannable
  • export-smoke-test: Shows which export failed AND the exact path issue

Minor note: The "skip" instructions vary slightly:

  • Gate 1/2: "To skip: add the skip-{gate} label"
  • Gate 3: Same ✅

All are good. One observation: developers encountering these gates might not know the feature flag vars exist—consider mentioning them in error output (optional; not blocking).


3. PR Description ✅

Strengths:

Quality: Excellent for a DevRel/CI gate PR. Clear audience is both maintainers (understanding cost) and future contributors (how to skip if needed).


4. Documentation Impact ⚠️

Current state: No changes to README, CONTRIBUTING, or SECURITY docs in this PR.

Questions for the maintainer:

  1. CONTRIBUTING.md — Should document:

    • The 3 new gates and when they run (only on PR events)
    • Skip labels when gates are too strict (e.g., hotfix where prerelease is temporary)
    • How to use feature flags to disable gates repo-wide or org-wide
  2. README — Does the "CI Gates" section need updating? Currently unclear if this deserves a mention in the main README or stays in CONTRIBUTING.

  3. Incident postmortem — Should there be a doc or issue linking back to PR feat(sdk): StorageProvider abstraction — complete migration + example providers bradygaster/squad#640 for context?

Recommendation:

  • ✅ Merge this PR as-is if the gates are exploratory/trial
  • ⚠️ If these gates are going to be enforced long-term, a follow-up PR to CONTRIBUTING.md would help new contributors understand the skip labels

Summary

Overall: Ready to merge.

  • Code is well-written, error UX is developer-friendly, intent is clear
  • YAML comments will help future maintainers
  • Error messages explain the what and the how to fix
  • PR description is thorough

Optional follow-up:

  • Add a line to CONTRIBUTING.md documenting the new skip labels (not a blocker for this PR)

@diberry 🚀

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

🧪 FIDO Quality Review — PR #115: CI Gates (workspace-integrity, prerelease-version-guard, export-smoke-test)

✅ Gate 1: workspace-integrity

Correctness: ✓ Gate checks both registry-resolved entries (https:// URLs) and non-workspace links. Correctly catches the PR bradygaster#640 scenario.

Edge cases & concerns:

  • ✓ Handles nested workspace packages correctly
  • ✓ Skips non-workspace @bradygaster/squad-* packages
  • ⚠️ False positive risk: Will trigger if a direct dependency (not workspace) is intentionally updated. Mitigation: skip-workspace-check label exists.
  • ⚠️ Silent pass risk: If package-lock.json has stale entries but npm.ci isn't run, they won't be detected. This is expected behavior (gate trusts the lockfile).

Error message: ✓ Clear — links directly to PR bradygaster#640, explains version mismatch, actionable fix (npm install). Good.

Testability: ✓ Can be tested locally — manually edit a workspace dependency to a registry URL, then run the gate script.


✅ Gate 2: prerelease-version-guard

Correctness: ✓ Regex /-/\ catches any hyphen-suffixed version. Matches specified patterns (-build, -alpha, -beta, -rc).

Edge cases & concerns:

  • ✓ Correctly handles \

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

🔧 EECOM Review: CI Gates for PR #115

✅ Summary

This PR adds 3 well-designed gates addressing the PR bradygaster#640 prerelease incident. All three follow solid patterns with good clarity, documentation, and skip mechanisms. Minor findings below.


1️⃣ Workspace Integrity Gate — ✅ STRONG DESIGN

Does npm ls approach catch the real bug?
Yes, directly. The gate parses \package-lock.json\ looking for workspace packages with:

The logic is precisely targeted: it won't false-positive on legitimate registry packages, only on @bradygaster/squad-*\ which must always be workspace links.

Cost benefit: Zero-install (no
pm ci), so negligible runtime. ✅


2️⃣ Prerelease Version Guard — ✅ SOLID

Does it catch the right thing?
Yes. Scans all \packages/*/package.json\ files for version strings matching /-/\ (regex confirms prerelease suffix). This prevents the root cause — leaving \

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

🏗️ Flight Review

Architecture Fit ✅

All 3 gates belong in squad-ci.yml — this is the right place for workspace & build integrity checks that run on every PR. They're lightweight, focused, and directly address the PR bradygaster#640 incident root causes. Not candidates for separate workflows.

Scope Assessment ✅

Correctly scoped. Each gate has a single, well-defined responsibility:

  • workspace-integrity: Detects version mismatch scenarios
  • prerelease-version-guard: Enforces release-ready semver
  • export-smoke-test: Validates build artifacts exist

The PR adds exactly what's needed — no over-engineering, no unnecessary gates. Closes issue #114 cleanly.

Dependencies & Ordering ✅

Good design here. Gates have no hard dependencies:

  • Gates 1 & 2 run in parallel (both zero-install, ~2s each)
  • Gate 3 runs independently with smart change detection
  • Each can skip independently without blocking others
  • Three-dot diff usage is correct — handles merge commits properly

Parallelization is optimal. No sequencing constraints needed.

Feature Flag/Skip Label Consistency ✅

Pattern-perfect. All three gates follow the exact template already established in changelog-gate and exports-map-check:

  • Feature flag check (undefined = enabled, set to 'false' to disable)
  • Skip label check (only if flag.skip == 'false')
  • Bash logic: [ "${{ vars.VAR }}" = "false" ] — consistent
  • GitHub Actions annotations: ::error:: format matches existing gates
  • Label names follow convention: skip-{gate-name}

All good here.

Coverage Assessment ✅

Gaps identified — minimal but worth noting:

  1. Label management: No automation to create/document the new skip labels. Consider a sync-squad-labels.yml update to ensure skip-workspace-check, skip-version-check, skip-export-smoke exist in repo + org variables. This is not blocking but would reduce friction.

  2. Error messages are excellent — they explain the problem, link to PR feat(sdk): StorageProvider abstraction — complete migration + example providers bradygaster/squad#640, and provide actionable fixes. Good UX.

  3. Test coverage for the gates themselves: These are shell scripts in YAML — no obvious way to unit test them before CI. Consider:

    • A quick smoke test of the Node.js logic (e.g., npm run test should include a test that verifies gate logic doesn't have syntax errors)
    • Or at least a linting pass to catch obvious Node.js issues
  4. Documentation: No update to CONTRIBUTING.md or a CI gates README explaining when/why to use each skip label. This could live in docs/ci-gates.md or in the workflow file comments (already done!). The comments in the workflow are excellent.

Overall Verdict 🟢 APPROVE

This is a high-quality addition. The PR:

Minor recommendations (not blockers):

  • Update sync-squad-labels.yml to document the new skip labels
  • Consider a quick syntax check or smoke test for gate logic in CI
  • Optional: Add label/gate documentation to CONTRIBUTING.md

Ready to merge. This PR significantly hardens CI against workspace incidents.

Copilot bot added 2 commits March 29, 2026 08:43
- Add Skip Labels Reference comment block listing all available skip labels (PAO, Flight)
- Add local testing instructions to each health gate (Flight, FIDO)
- Document change-detection regex patterns for future maintainers (FIDO)
- Enhance export smoke test with dynamic import() validation (EECOM)
- Add test PR creation hints to gate comments (FIDO)

Closes #115

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove duplicate cache-dependency-path in export-smoke-test setup-node step
- Remove false-positive else-if branch that flagged workspace packages lacking link:true
  (npm lockfile v3 workspace entries under node_modules/ use resolved:file: not link:true)

Closes #115

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diberry pushed a commit that referenced this pull request Mar 29, 2026
Resets 0.9.1-build.N versions to 0.9.1 in SDK and CLI package.json.
Prerelease suffixes cause npm to silently resolve stale registry
packages instead of local workspace links (semver prerelease behavior).

This unblocks the new prerelease-version-guard CI gate (PR #115).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diberry pushed a commit that referenced this pull request Mar 29, 2026
Resets 0.9.1-build.N versions to 0.9.1 in SDK and CLI package.json.
Prerelease suffixes cause npm to silently resolve stale registry
packages instead of local workspace links (semver prerelease behavior).

This unblocks the new prerelease-version-guard CI gate (PR #115).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diberry pushed a commit that referenced this pull request Mar 29, 2026
… gates

Adds 3 CI health gates to squad-ci.yml.

Closes #115

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diberry added a commit to bradygaster/squad that referenced this pull request Mar 29, 2026
… gates

Adds 3 CI health gates. Fork PR: diberry#115. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

Merged upstream as bradygaster#691

@diberry diberry closed this Mar 29, 2026
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