Skip to content

Commit 3d8aab8

Browse files
committed
merge: PR #321 - feat: add distribution pipeline checks across skill workflow
2 parents 2f50d85 + 024fc5f commit 3d8aab8

4 files changed

Lines changed: 59 additions & 2 deletions

File tree

office-hours/SKILL.md.tmpl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,8 @@ Before proposing solutions, challenge the premises:
334334
1. **Is this the right problem?** Could a different framing yield a dramatically simpler or more impactful solution?
335335
2. **What happens if we do nothing?** Real pain point or hypothetical one?
336336
3. **What existing code already partially solves this?** Map existing patterns, utilities, and flows that could be reused.
337-
4. **Startup mode only:** Synthesize the diagnostic evidence from Phase 2A. Does it support this direction? Where are the gaps?
337+
4. **If the deliverable is a new artifact** (CLI binary, library, package, container image, mobile app): **how will users get it?** Code without distribution is code nobody can use. The design must include a distribution channel (GitHub Releases, package manager, container registry, app store) and CI/CD pipeline — or explicitly defer it.
338+
5. **Startup mode only:** Synthesize the diagnostic evidence from Phase 2A. Does it support this direction? Where are the gaps?
338339

339340
Output premises as clear statements the user must agree with before proceeding:
340341
```
@@ -474,6 +475,11 @@ Supersedes: {prior filename — omit this line if first design on this branch}
474475
## Success Criteria
475476
{measurable criteria from Phase 2A}
476477
478+
## Distribution Plan
479+
{how users get the deliverable — binary download, package manager, container image, web service, etc.}
480+
{CI/CD pipeline for building and publishing — GitHub Actions, manual release, auto-deploy on merge?}
481+
{omit this section if the deliverable is a web service with existing deployment pipeline}
482+
477483
## Dependencies
478484
{blockers, prerequisites, related work}
479485
@@ -526,6 +532,10 @@ Supersedes: {prior filename — omit this line if first design on this branch}
526532
## Success Criteria
527533
{what "done" looks like}
528534
535+
## Distribution Plan
536+
{how users get the deliverable — binary download, package manager, container image, web service, etc.}
537+
{CI/CD pipeline for building and publishing — or "existing deployment pipeline covers this"}
538+
529539
## Next Steps
530540
{concrete build tasks — what to implement first, second, third}
531541

plan-eng-review/SKILL.md.tmpl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ Before reviewing anything, answer these questions:
9494

9595
5. **Completeness check:** Is the plan doing the complete version or a shortcut? With AI-assisted coding, the cost of completeness (100% test coverage, full edge case handling, complete error paths) is 10-100x cheaper than with a human team. If the plan proposes a shortcut that saves human-hours but only saves minutes with CC+gstack, recommend the complete version. Boil the lake.
9696

97+
6. **Distribution check:** If the plan introduces a new artifact type (CLI binary, library package, container image, mobile app), does it include the build/publish pipeline? Code without distribution is code nobody can use. Check:
98+
- Is there a CI/CD workflow for building and publishing the artifact?
99+
- Are target platforms defined (linux/darwin/windows, amd64/arm64)?
100+
- How will users download or install it (GitHub Releases, package manager, container registry)?
101+
If the plan defers distribution, flag it explicitly in the "NOT in scope" section — don't let it silently drop.
102+
97103
If the complexity check triggers (8+ files or 2+ new classes/services), proactively recommend scope reduction via AskUserQuestion — explain what's overbuilt, propose a minimal version that achieves the core goal, and ask whether to reduce or proceed as-is. If the complexity check does not trigger, present your Step 0 findings and proceed directly to Section 1.
98104

99105
Always work through the full interactive review: one section at a time (Architecture → Code Quality → Tests → Performance) with at most 8 top issues per section.
@@ -111,6 +117,7 @@ Evaluate:
111117
* Security architecture (auth, data access, API boundaries).
112118
* Whether key flows deserve ASCII diagrams in the plan or in code comments.
113119
* For each new codepath or integration point, describe one realistic production failure scenario and whether the plan accounts for it.
120+
* **Distribution architecture:** If this introduces a new artifact (binary, package, container), how does it get built, published, and updated? Is the CI/CD pipeline part of the plan or deferred?
114121

115122
**STOP.** For each issue found in this section, call AskUserQuestion individually. One issue per call. Present options, state your recommendation, explain WHY. Do NOT batch multiple issues into one AskUserQuestion. Only proceed to the next section after ALL issues in this section are resolved.
116123

review/checklist.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,18 @@ To do this: use Grep to find all references to the sibling values (e.g., grep fo
125125
- Small utility additions (<5KB gzipped)
126126
- Server-side-only dependencies
127127

128+
#### Distribution & CI/CD Pipeline
129+
- CI/CD workflow changes (`.github/workflows/`): verify build tool versions match project requirements, artifact names/paths are correct, secrets use `${{ secrets.X }}` not hardcoded values
130+
- New artifact types (CLI binary, library, package): verify a publish/release workflow exists and targets correct platforms
131+
- Cross-platform builds: verify CI matrix covers all target OS/arch combinations, or documents which are untested
132+
- Version tag format consistency: `v1.2.3` vs `1.2.3` — must match across VERSION file, git tags, and publish scripts
133+
- Publish step idempotency: re-running the publish workflow should not fail (e.g., `gh release delete` before `gh release create`)
134+
135+
**DO NOT flag:**
136+
- Web services with existing auto-deploy pipelines (Docker build + K8s deploy)
137+
- Internal tools not distributed outside the team
138+
- Test-only CI changes (adding test steps, not publish steps)
139+
128140
---
129141

130142
## Severity Classification
@@ -141,7 +153,8 @@ CRITICAL (highest severity): INFORMATIONAL (lower severity):
141153
├─ Time Window Safety
142154
├─ Type Coercion at Boundaries
143155
├─ View/Frontend
144-
└─ Performance & Bundle Impact
156+
├─ Performance & Bundle Impact
157+
└─ Distribution & CI/CD Pipeline
145158
146159
All findings are actioned via Fix-First Review. Severity determines
147160
presentation order and classification of AUTO-FIX vs ASK — critical

ship/SKILL.md.tmpl

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,33 @@ If the Eng Review is NOT "CLEAR":
8383

8484
---
8585

86+
## Step 1.5: Distribution Pipeline Check
87+
88+
If the diff introduces a new standalone artifact (CLI binary, library package, tool) — not a web
89+
service with existing deployment — verify that a distribution pipeline exists.
90+
91+
1. Check if the diff adds a new `cmd/` directory, `main.go`, or `bin/` entry point:
92+
```bash
93+
git diff origin/<base> --name-only | grep -E '(cmd/.*/main\.go|bin/|Cargo\.toml|setup\.py|package\.json)' | head -5
94+
```
95+
96+
2. If new artifact detected, check for a release workflow:
97+
```bash
98+
ls .github/workflows/ 2>/dev/null | grep -iE 'release|publish|dist'
99+
```
100+
101+
3. **If no release pipeline exists and a new artifact was added:** Use AskUserQuestion:
102+
- "This PR adds a new binary/tool but there's no CI/CD pipeline to build and publish it.
103+
Users won't be able to download the artifact after merge."
104+
- A) Add a release workflow now (GitHub Actions cross-platform build + GitHub Releases)
105+
- B) Defer — add to TODOS.md
106+
- C) Not needed — this is internal/web-only, existing deployment covers it
107+
108+
4. **If release pipeline exists:** Continue silently.
109+
5. **If no new artifact detected:** Skip silently.
110+
111+
---
112+
86113
## Step 2: Merge the base branch (BEFORE tests)
87114

88115
Fetch and merge the base branch into the feature branch so tests run against the merged state:

0 commit comments

Comments
 (0)