Skip to content

docs(skill): add versioning policy — prevents prerelease version incidents#117

Closed
diberry wants to merge 1 commit intodevfrom
squad/versioning-policy
Closed

docs(skill): add versioning policy — prevents prerelease version incidents#117
diberry wants to merge 1 commit intodevfrom
squad/versioning-policy

Conversation

@diberry
Copy link
Copy Markdown
Owner

@diberry diberry commented Mar 29, 2026

Versioning Policy Skill

Adds .squad/skills/versioning-policy/SKILL.md — a comprehensive versioning policy for the Squad monorepo.

Why this is needed

The repo had no documented versioning policy, which caused two incidents:

Codifies semver versioning rules for SDK and CLI packages:
- MAJOR.MINOR.PATCH only on dev/main (no prerelease suffixes)
- bump-build.mjs -build.N versions are local-only, never committed
- SDK + CLI versions must stay in sync (workspace resolution footgun)
- Surgeon owns version bumps; other agents hands-off
- prerelease-version-guard CI gate enforces the policy

Motivated by PR bradygaster#640 (prerelease broke workspace resolution) and
PR #116 (prerelease leak on release branch).

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

diberry commented Mar 29, 2026

🔧 EECOM CODE REVIEW

PR #117 - versioning-policy skill documentation

Review Summary

APPROVED — Technical accuracy verified on npm workspace semver footgun, bump-build.mjs behavior, and SKIP_BUILD_BUMP workaround.

Detailed Findings

1. npm Workspace Semver Footgun

The documentation correctly describes the critical semver vulnerability (§4):

  • Accurately states that >=0.9.0 does NOT match

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

📣 PAO DevRel Review — PR #117

Overall Assessment

Clarity & Readability: Excellent
Contributor Onboarding: Strong
Skill Format Compliance: Meets template standards


Strengths

1. Excellent Context & Clarity

The skill document excels at explaining why the policy exists. The incident reference (PR bradygaster#640) is particularly valuable — it grounds the abstract rules in a concrete failure scenario that new contributors can understand. A contributor reading this will immediately grasp why "prerelease versions break workspace resolution" is not just a rule, but a learned lesson.

2. Accessible Incident Narrative

The PR bradygaster#640 explanation (§8) is genuinely helpful:

  • Clearly shows the problem (workspace dependency failed to match prerelease)
  • Explains the semver footgun (§4) thoroughly with concrete examples
  • Traces how four symptom-fixing PRs masked the root cause
  • Makes the rationale undeniable

3. Strong Template Compliance

  • ✅ YAML front matter: correct fields (name, description, domain, confidence, source)
  • ✅ Context section: sets up the domain and problem space
  • ✅ Numbered rules (8 sections) with clear subsections
  • ✅ Quick Reference table for easy scanning
  • ✅ Examples and anti-patterns (though anti-patterns are implicit in the flow)
  • ✅ Both practical instruction and conceptual grounding

4. Practical, Actionable Rules

The ownership matrix (§5) is clear:

Agent May modify version?
Surgeon ✅ Yes
Others ❌ No (unless fixing leak)

The lifecycle diagram (§6) is a nice visual anchor for understanding version state transitions.


Minor Suggestions for Improvement

1. Typo in SKILL.md, Line 25–26

The `scripts/bump-build.mjs` script creates `-build.N` versions (e.g., `0.9.1-build.4`) for **local development testing
g only**.

→ Should be: **local development testing only** (stray line break and g)

2. Line 30–31 Formatting Issue

Both `@bradygaster/squad-sdk` and `@bradygaster/squad-cli` **MUST have the same version** at all times. The root `packa
ge.json` version must also match.

→ Should be: package.json (stray line break in word)

3. Line 33–34 Line Break Issue

`bump-build.mjs` enforces this by updating all three `package.json` files in lockstep (root + `packages/squad-sdk` + `p
packages/squad-cli`).

→ Should be: packages/squad-cli (stray p and line break)

4. Inconsistent Line Break in §4

Lines 52–53 have similar wrapping artifacts. Suggests the markdown may have been reformatted or pasted with word-wrap issues.

5. Consider Adding Anti-Patterns Section

The document is strong on what to do, but an explicit anti-patterns section would reinforce lessons:

  • ❌ Committing -build.N versions to branches
  • ❌ Modifying versions outside of Surgeon ownership (except leak fixes)
  • ❌ Diverging SDK/CLI versions
  • ❌ Relying on the skip-version-check label for regular development

6. Decision File Could Link Back to Skill

The decision file references the skill ("Full policy documented in .squad/skills/versioning-policy/SKILL.md"), which is good. Consider also linking from the SKILL.md header back to the decision for cross-referencing.


New Contributor Perspective

A new Squad member reading this would:

  1. ✅ Understand that prerelease versions are forbidden on release branches (clear rules)
  2. ✅ Learn why they're forbidden (PR feat(sdk): StorageProvider abstraction — complete migration + example providers bradygaster/squad#640 incident + semver footnote)
  3. ✅ Know who to contact for version bumps (Surgeon)
  4. ✅ Identify the escape hatch (fix leaks without approval)
  5. ✅ Understand the CI enforcement gate

The skill is contributor-ready with minor formatting cleanup.


Recommendation

Merge with minor fixes:

  • Fix the three line-break/wrapping issues (lines 25–26, 30–31, 33–34)
  • Optionally add an explicit anti-patterns section for reinforcement
  • Consider a back-reference from SKILL.md to the decision file

The core policy is well-reasoned, clearly explained, and will prevent similar incidents. The investment in narrative and incident context is exactly what makes skills effective for onboarding and compliance.


PAO signing off — nicely done, Flight. ✨

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

⚙️ Booster (CI/CD Engineer) Review: PR #117 — Versioning Policy Documentation

Summary

The versioning policy SKILL is well-written and comprehensively documents the constraints that the CI gates in PR #115 enforce. I found the documentation to be accurate and complete — no critical gaps between the policy and CI enforcement.

Alignment with CI Gates (PR #115)

Fully Aligned. The policy correctly references and explains all three version-related gates:

  1. prerelease-version-guard → Policy §5 & §7 precisely match the CI job:

    • Uses regex /-/\ to detect prerelease suffixes (matches the CI gate)
    • Correctly states \skip-version-check\ label is Surgeon-only (gate respects this)
    • Explains the escape hatch: any agent may revert a prerelease leak
  2. workspace-integrity → Policy §4 (npm Workspace Semver Footgun) explains the root cause:

    • Semver spec: >=0.9.0\ does NOT match \

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

🧪 FIDO Quality Review — PR #117 Versioning Policy Skill

Summary

This PR documents critical versioning policies that prevented the PR bradygaster#640 workspace resolution incident. The skill is well-structured and addresses the semver footgun comprehensively. Overall: Strong foundation with actionability gap.

Completeness vs. PR bradygaster#640 Incident ✅

PR bradygaster#640 failure modes covered:

  • ✅ Prerelease versions breaking workspace resolution (§4 explains the semver footgun in detail)
  • ✅ Root cause: >=0.9.0 does NOT match 0.9.1-build.4 per spec
  • ✅ Silent fallback to stale registry version
  • ✅ Prevention: CI gate + version ownership centralization
  • ✅ Escape hatch for accidental prerelease leaks (any agent can revert)

Additional incidents addressed:

Accuracy of Semver Claims ✅

Verified against code:

  • ✅ Format claim: MAJOR.MINOR.PATCH (SKILL.md §1) — bump-build.mjs enforces this
  • ✅ Prerelease format: -build.N is valid semver (npm accepts it, scripts produce it)
  • ✅ Sync requirement (§3): bump-build.mjs updates all 3 package.json files atomically
  • ✅ CI skip logic (§2): Script checks SKIP_BUILD_BUMP=1 and CI=true
  • ✅ Workspace dependency range: CLI depends on SDK via >=0.9.0 (verified in code)

Critical concern: The skill claims prerelease-version-guard CI gate exists (§7), but I could not find the implementation in squad-ci.yml. This gate is documented but NOT yet implemented in the workflow. This is a gap.

Actionability ⚠️

Clear and actionable:

  • ✅ Version format rule is explicit and falsifiable
  • ✅ Ownership model is unambiguous (Surgeon only; escape hatch defined)
  • ✅ Quick reference table (§9) is team-friendly

Gap: Missing CI enforcement mechanism

  • The skill documents the prerelease-version-guard CI gate (§7) as if it's active
  • But the gate is NOT currently enforced in the squad-ci.yml workflow
  • Result: Agents following the skill may expect automation that doesn't exist yet
  • Recommendation: Either (a) implement the gate, or (b) revise §7 to clarify status

Critical Questions

  1. Is the prerelease-version-guard gate implemented? If not, §7 should clarify "planned" vs. "active"
  2. Is skip-version-check label created? The skill references it but I found no label definition
  3. Who enforces the "Surgeon-only" ownership rule? Currently human discipline. Consider CODEOWNERS or branch protection rules

Recommendation

Approve with note: This skill successfully documents the root cause and prevention strategy for the PR bradygaster#640 incident. It is actionable for the team once the CI gate is in place or documentation is clarified.

Before merge, address:

  1. Clarify in PR description whether prerelease-version-guard CI gate is implemented or planned
  2. Confirm skip-version-check label exists (or will be created as a follow-up)
  3. Optional: Link to the CI gate implementation (if it's a separate PR) or add a TODO to squad backlog

FIDO confidence: 🟢 Good fit — Skill accurately reflects semver spec and incident RCA. Actionability depends on confirming CI gate status.

diberry pushed a commit that referenced this pull request Mar 29, 2026
Documents versioning rules to prevent prerelease version incidents.

Closes #117

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
Documents versioning rules. Fork PR: diberry#117. 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#692

@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