build: build dist/ at release time instead of committing it to main - #842
Open
xdustinface wants to merge 1 commit into
Open
build: build dist/ at release time instead of committing it to main#842xdustinface wants to merge 1 commit into
dist/ at release time instead of committing it to main#842xdustinface wants to merge 1 commit into
Conversation
Reverses the approach from [#810](#810). Instead of keeping `dist/` checked into `main` and verifying it on every push with `check-dist.yml`, the release workflow now rebuilds `dist/` when a `vX.Y.Z` tag is pushed, commits the artifacts, and force-updates the tag to point at that build commit. Exact-version pins like `manki-review/manki@v5.1.2` still resolve to a tree containing `dist/`, but `main` stays source-only. Release workflow changes in `.github/workflows/release.yml`: - Tag trigger tightened from `v*` to `v[0-9]+.[0-9]+.[0-9]+` so only proper semver releases fire. - Adds a `concurrency` group keyed on the tag ref to prevent racing release runs. - New `Check whether tag already has built dist/` step makes reruns idempotent: if the tag commit already carries `dist/`, the `setup-node`, `npm ci`, `npm run build`, and tag-rewrite steps are skipped. - When `dist/` is missing, build it, commit as `release: build artifacts for $TAG`, and force-push the rewritten annotated tag to that commit. - Floating major tag (`vN`) is updated via the fully-qualified `refs/tags/$MAJOR` ref to stay in sync. - GitHub Release step uses the resolved `steps.tag.outputs.tag` instead of re-deriving from `$GITHUB_REF`. Repo hygiene: - `.gitignore` now ignores `dist/` so locally-built artifacts don't get staged. - `.gitattributes` collapses to a single `dist/** -diff linguist-generated=true` glob. - `check-dist.yml` removed (no committed `dist/` to verify against). - Existing `dist/` artifacts deleted from `main`.
|
Manki — Review complete Planner (31s) Review — 4 findings Judge — 4 kept · 0 dropped (13s) Review metadataConfig:
Judge decisions:
Timing:
|
![manki-review[bot]](https://avatars.githubusercontent.com/in/3200573?s=60&v=4){kind=small}
There was a problem hiding this comment.
Force-pushing the release tag will retrigger the workflow on every release — real CI waste worth fixing. The other three are speculative edge cases that don't bite today.
📊 4 findings (1 warning, 2 suggestion, 1 nitpick) · 88265 lines · 194s
Manki context
{
"meta": {
"prNumber": 842,
"commitSha": "a6076a159fe386557f7c17e43d8de9a1199077be",
"round": 1,
"timestamp": "2026-05-23T13:53:40.519Z",
"mankiVersion": "5.2.0",
"cap": {
"priorRoundCount": 0,
"maxAutoRounds": 5,
"skipCap": false,
"forceReview": false,
"bypassReason": "within_cap"
},
"trigger": {
"event": "pull_request:opened",
"sender": "xdustinface"
}
},
"config": {
"reviewLevel": "large",
"memoryEnabled": true,
"reviewPasses": 1
},
"diff": {
"lines": 88265,
"additions": 48,
"deletions": 88217,
"filesReviewed": 4,
"fileTypes": {
".gitattributes": 1,
".yml": 2,
".gitignore": 1
},
"excludedFiles": [
{
"path": "dist/index.js",
"matchedPattern": "dist/**"
},
{
"path": "dist/index.js.map",
"matchedPattern": "dist/**"
},
{
"path": "dist/licenses.txt",
"matchedPattern": "dist/**"
},
{
"path": "dist/sourcemap-register.js",
"matchedPattern": "dist/**"
}
],
"oversizedHandled": false,
"perFile": [
{
"path": ".gitattributes",
"additions": 1,
"deletions": 2,
"changeType": "modified"
},
{
"path": ".github/workflows/check-dist.yml",
"additions": 0,
"deletions": 51,
"changeType": "deleted"
},
{
"path": ".github/workflows/release.yml",
"additions": 46,
"deletions": 10,
"changeType": "modified"
},
{
"path": ".gitignore",
"additions": 1,
"deletions": 0,
"changeType": "modified"
}
]
},
"models": {
"planner": "claude-haiku-4-5",
"reviewer": "claude-sonnet-4-6",
"judge": "claude-opus-4-7",
"dedup": "claude-haiku-4-5"
},
"planner": {
"source": "planner",
"used": true,
"coreAgentInjections": [],
"priorRoundEffortDowngrades": [],
"teamSize": 2,
"reviewerEffort": "medium",
"judgeEffort": "medium",
"prType": "build",
"durationMs": 31064,
"language": "javascript",
"context": "GitHub Actions action",
"agents": [
{
"name": "Architecture & Design",
"effort": "medium"
},
{
"name": "Dependencies & Integration",
"effort": "high"
}
]
},
"reviewers": {
"agents": [
"Architecture & Design",
"Dependencies & Integration"
],
"agentMetrics": [
{
"name": "Architecture & Design",
"findingsRaw": 1,
"findingsKept": 1,
"durationMs": 97599,
"status": "success",
"responseLength": 1702,
"inputTokens": 3,
"outputTokens": 5312,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Dependencies & Integration",
"findingsRaw": 3,
"findingsKept": 3,
"durationMs": 143730,
"status": "success",
"responseLength": 3093,
"inputTokens": 3,
"outputTokens": 7856,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
}
]
},
"judge": {
"summary": "Force-pushing the release tag will retrigger the workflow on every release — real CI waste worth fixing. The other three are speculative edge cases that don't bite today.",
"confidenceDistribution": {
"high": 3,
"medium": 1,
"low": 0
},
"severityChanges": 4,
"mergedDuplicates": 0,
"durationMs": 13467,
"retryCount": 0,
"verdictReason": "novel_suggestion",
"defensiveHardeningCount": 1,
"verdictTrace": {
"survivingBlockers": [],
"novelWarnings": [
{
"file": ".github/workflows/release.yml",
"title": "Force-pushing the rewritten tag triggers an unintended second workflow run",
"fingerprint": ".github/workflows/release.yml:57:57:Force-pushing-the-rewritten-tag-triggers-an-unintended-second-workflow-run"
}
],
"unresolvedPriors": []
},
"openThreadsState": "empty",
"openThreadCount": 0,
"resolvedThreadIdCount": 0,
"interRoundDiffState": "unknown",
"interRoundDiffTruncated": false
},
"dedup": {
"staticDropped": 0,
"llmDropped": 0
},
"memory": {
"loadStatus": "loaded"
},
"findings": {
"count": 4,
"severityCounts": {
"blocker": 0,
"warning": 1,
"suggestion": 2,
"nitpick": 1
},
"entries": [
{
"fingerprint": {
"file": ".github/workflows/release.yml",
"lineStart": 57,
"lineEnd": 57,
"slug": "Force-pushing-the-rewritten-tag-triggers-an-unintended-second-workflow-run"
},
"severity": "warning",
"specialist": "Architecture & Design",
"suggestedFix": "# Add as the first step in the job, before 'Resolve tag info':\n- name: Skip bot-triggered reruns\n if: github.actor == 'github-actions[bot]'\n run: |\n echo \"Workflow re-triggered by bot force-push — skipping.\"\n exit 0\n\n# Caveat: \u0060exit 0\u0060 inside a step does not stop subsequent steps;\n# use a jo",
"title": "Force-pushing the rewritten tag triggers an unintended second workflow run",
"judgeNotes": "Impact: Medium (wasted CI, confusing duplicate runs, redundant force-pushes of major tag and release edit), Likelihood: Certain (happens on every release). Concrete and actionable with a clear job-level \u0060if:\u0060 fix.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": ".github/workflows/release.yml",
"lineStart": 1,
"lineEnd": 1,
"slug": "Removing-dist--from-main-breaks--main-and-branch-SHA-consumers-immediately"
},
"severity": "suggestion",
"specialist": "Dependencies & Integration",
"suggestedFix": "Add a note to README / USAGE docs stating that @main is no longer a valid ref and consumers must pin to a release tag (e.g., @v5 or @v5.1.2). Optionally leave a stub dist/index.js on main that prints an actionable error message.",
"title": "Removing dist/ from main breaks @main and branch-SHA consumers immediately",
"judgeNotes": "This is a documented intentional change (PR explicitly states main stays source-only); @main pinning to a JS action was never supported practice. Worth a docs note but not a behavioral defect.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": ".github/workflows/release.yml",
"lineStart": 68,
"lineEnd": 68,
"slug": "MAJOR-tag-variable-not-guarded-against-empty-string-before-git-tag"
},
"severity": "nitpick",
"specialist": "Dependencies & Integration",
"suggestedFix": "Add at the top of the 'Update floating major tag' run block:\n\u0060\u0060\u0060bash\n[ -n \"$MAJOR\" ] || { echo \"::error::Could not extract major version from tag $TAG\"; exit 1; }\n\u0060\u0060\u0060",
"title": "MAJOR tag variable not guarded against empty string before git tag",
"judgeNotes": "Impact: Low (opaque error vs clear error), Likelihood: Unlikely (requires changing trigger pattern, which is currently strictly \u0060v[0-9]+.[0-9]+.[0-9]+\u0060). Defensive guard against a hypothetical future change.",
"judgeConfidence": "high",
"reachability": "hypothetical",
"reachabilityReasoning": "The workflow trigger pattern guarantees the tag matches \u0060^v[0-9]+\u0060, so the grep will always produce a non-empty major; no current trigger path produces an empty MAJOR.",
"tags": [
"defensive-hardening"
],
"originalSeverity": "suggestion"
},
{
"fingerprint": {
"file": ".github/workflows/release.yml",
"lineStart": 72,
"lineEnd": 72,
"slug": "Release-rerun-on-existing-release-does-not-regenerate-notes"
},
"severity": "suggestion",
"specialist": "Dependencies & Integration",
"suggestedFix": "Replace the edit branch with a delete-and-recreate, or add \u0060--generate-notes\u0060 to the edit command:\n\u0060\u0060\u0060bash\ngh release edit \"$TAG\" --title \"$TAG\" --latest --generate-notes\n\u0060\u0060\u0060\nCaveat: \u0060--generate-notes\u0060 on edit will overwrite any manually authored release body, so this trade-off should be documented.",
"title": "Release rerun on existing release does not regenerate notes",
"judgeNotes": "Impact: Low (cosmetic — empty notes on a partial-failure rerun), Likelihood: Possible but rare (requires initial creation to partially fail after release object existed). Trade-off with overwriting manual edits makes the fix non-obvious; reasonable as-is.",
"judgeConfidence": "medium",
"reachability": "reachable"
}
]
},
"usage": {
"inputTokens": 20,
"outputTokens": 17063,
"totalTokens": 17083,
"perStage": {
"planner": {
"inputTokens": 9,
"outputTokens": 3191,
"totalTokens": 3200
},
"reviewer": {
"inputTokens": 6,
"outputTokens": 13168,
"totalTokens": 13174
},
"judge": {
"inputTokens": 5,
"outputTokens": 704,
"totalTokens": 709
}
}
},
"verdict": "REQUEST_CHANGES",
"recap": {
"priorRoundCount": 0,
"reclassifiedPriorCount": 0
}
}Reviewed commit a6076a1
| run: | | ||
| TAG="${{ steps.tag.outputs.tag }}" | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" |
There was a problem hiding this comment.
🔴
The 'Commit dist/ and rewrite tag to the release commit' step ends with git push origin "refs/tags/$TAG" --force, which pushes a ref matching the workflow's own v[0-9]+.[0-9]+.[0-9]+ trigger. GitHub Actions fires a new run for that push, so every release will automatically spawn a second run. The second run will find has_dist=true and skip the build, but it still executes 'Update floating major tag' (redundant force-push of refs/tags/$MAJOR) and 'Create GitHub Release' (which falls into the edit-existing branch). This is wasteful CI consumption and creates confusing duplicate runs in the Actions history; if CI queues are slow the second run could also race against external consumers of the just-published release. Fix by short-circuiting at the job level when the event is bot-authored: add a first step that checks github.actor == 'github-actions[bot]' and exits early (or set a job-level if: condition using the pusher identity), ensuring only the human-initiated tag push does real work.
Suggested fix
Suggested change
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| # Add as the first step in the job, before 'Resolve tag info': | |
| - name: Skip bot-triggered reruns | |
| if: github.actor == 'github-actions[bot]' | |
| run: | | |
| echo "Workflow re-triggered by bot force-push — skipping." | |
| exit 0 | |
| # Caveat: `exit 0` inside a step does not stop subsequent steps; | |
| # use a job-level condition instead: | |
| jobs: | |
| release: | |
| runs-on: ubuntu-latest | |
| if: github.actor != 'github-actions[bot]' | |
| steps: | |
| ... |
AI context
{
"file": ".github/workflows/release.yml",
"line": 57,
"severity": "warning",
"confidence": "high",
"flaggedBy": [
"Architecture & Design"
],
"title": "Force-pushing the rewritten tag triggers an unintended second workflow run",
"fix": "# Add as the first step in the job, before 'Resolve tag info':\n- name: Skip bot-triggered reruns\n if: github.actor == 'github-actions[bot]'\n run: |\n echo \"Workflow re-triggered by bot force-push ",
"reachability": "reachable"
}| @@ -3,39 +3,75 @@ name: Release | |||
| on: | |||
There was a problem hiding this comment.
🔴 ✨ Suggestion: Removing dist/ from main breaks @main and branch-SHA consumers immediately
Deleting dist/index.js from main means any downstream workflow pinned to uses: manki-review/manki@main (or a commit SHA on main) will fail instantly after merge because action.yml references dist/index.js which no longer exists on that ref. The PR description acknowledges 'main stays source-only' but provides no migration path for @main consumers — a breaking change that is invisible until their CI fails. Consider adding a deprecation notice or at minimum calling this out in the PR description with an explicit statement that @main usage is unsupported going forward.
Suggested fix
Suggested change
| on: | |
| Add a note to README / USAGE docs stating that @main is no longer a valid ref and consumers must pin to a release tag (e.g., @v5 or @v5.1.2). Optionally leave a stub dist/index.js on main that prints an actionable error message. |
AI context
{
"file": ".github/workflows/release.yml",
"line": 1,
"severity": "suggestion",
"confidence": "high",
"flaggedBy": [
"Dependencies & Integration"
],
"title": "Removing dist/ from main breaks @main and branch-SHA consumers immediately",
"fix": "Add a note to README / USAGE docs stating that @main is no longer a valid ref and consumers must pin to a release tag (e.g., @v5 or @v5.1.2). Optionally leave a stub dist/index.js on main that prints ",
"reachability": "reachable"
}| TAG="${{ steps.tag.outputs.tag }}" | ||
| MAJOR="${{ steps.tag.outputs.major }}" | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" |
There was a problem hiding this comment.
🔴 📝 Nitpick: MAJOR tag variable not guarded against empty string before git tag
[defensive hardening — capped from suggestion]
In the 'Update floating major tag' step, MAJOR is taken from steps.tag.outputs.major which is produced by grep -oE '^v[0-9]+'. If this output is ever empty (e.g., if workflow_dispatch is added later, or the trigger pattern is relaxed), git tag -fa "" "$TAG" fails with an opaque git error rather than a helpful diagnostic, and the subsequent force-push would attempt to push an empty-named ref. A defensive guard at the top of the step would make the failure mode obvious and prevent confusing partial state.
Suggested fix
Suggested change
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| Add at the top of the 'Update floating major tag' run block: | |
| ```bash | |
| [ -n "$MAJOR" ] || { echo "::error::Could not extract major version from tag $TAG"; exit 1; } | |
| ``` |
AI context
{
"file": ".github/workflows/release.yml",
"line": 68,
"severity": "nitpick",
"confidence": "high",
"flaggedBy": [
"Dependencies & Integration"
],
"title": "MAJOR tag variable not guarded against empty string before git tag",
"fix": "Add at the top of the 'Update floating major tag' run block:\n```bash\n[ -n \"$MAJOR\" ] || { echo \"::error::Could not extract major version from tag $TAG\"; exit 1; }\n```",
"tags": [
"defensive-hardening"
],
"reachability": "hypothetical",
"reachabilityReasoning": "The workflow trigger pattern guarantees the tag matches `^v[0-9]+`, so the grep will always produce a non-empty major; no current trigger path produces an empty MAJOR.",
"originalSeverity": "suggestion"
}| git tag -fa "$MAJOR" "$TAG" -m "Update $MAJOR tag to $TAG" | ||
| git push origin "refs/tags/$MAJOR" --force | ||
|
|
||
| - name: Create GitHub Release |
There was a problem hiding this comment.
🟠 ✨ Suggestion: Release rerun on existing release does not regenerate notes
When gh release view "$TAG" succeeds (idempotency rerun path), the step runs gh release edit "$TAG" --title "$TAG" --latest which updates the title and latest flag but does not regenerate or update the release notes body. If the initial release creation failed mid-way (e.g., --generate-notes timed out) and the release was created with an empty body, a rerun will not fix the notes. Operators expecting a rerun to produce a complete release will be surprised. The fix is to conditionally pass --generate-notes on edit when the body is empty, or to always recreate rather than edit.
Suggested fix
Suggested change
| - name: Create GitHub Release | |
| Replace the edit branch with a delete-and-recreate, or add `--generate-notes` to the edit command: | |
| ```bash | |
| gh release edit "$TAG" --title "$TAG" --latest --generate-notes | |
| ``` | |
| Caveat: `--generate-notes` on edit will overwrite any manually authored release body, so this trade-off should be documented. |
AI context
{
"file": ".github/workflows/release.yml",
"line": 72,
"severity": "suggestion",
"confidence": "medium",
"flaggedBy": [
"Dependencies & Integration"
],
"title": "Release rerun on existing release does not regenerate notes",
"fix": "Replace the edit branch with a delete-and-recreate, or add `--generate-notes` to the edit command:\n```bash\ngh release edit \"$TAG\" --title \"$TAG\" --latest --generate-notes\n```\nCaveat: `--generate-notes",
"reachability": "reachable"
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverses the approach from #810. Instead of keeping
dist/checked intomainand verifying it on every push withcheck-dist.yml, the release workflow now rebuildsdist/when avX.Y.Ztag is pushed, commits the artifacts, and force-updates the tag to point at that build commit. Exact-version pins likemanki-review/manki@v5.1.2still resolve to a tree containingdist/, butmainstays source-only.Release workflow changes (
.github/workflows/release.yml)v*tov[0-9]+.[0-9]+.[0-9]+so only proper semver releases fire.concurrencygroup keyed on the tag ref to prevent racing release runs.Check whether tag already has built dist/step makes reruns idempotent: if the tag commit already carriesdist/, thesetup-node,npm ci,npm run build, and tag-rewrite steps are skipped.dist/is missing, build it, commit asrelease: build artifacts for $TAG, and force-push the rewritten annotated tag to that commit.vN) is updated via the fully-qualifiedrefs/tags/$MAJORref to stay in sync.steps.tag.outputs.taginstead of re-deriving from$GITHUB_REF.Repo hygiene
.gitignorenow ignoresdist/so locally-built artifacts don't get staged..gitattributescollapses to a singledist/** -diff linguist-generated=trueglob.check-dist.ymlremoved (no committeddist/to verify against).dist/artifacts deleted frommain.Test plan
check-distjob anymore, so just the standard checks).vX.Y.Ztag and confirm:dist/is rebuilt, committed, the tag is rewritten to that commit, the floatingvNtag is updated, and a GitHub Release is created.has_distshort-circuit skips the build/commit steps.manki-review/manki@vX.Y.Z(exact-version pin) still resolves to a tree containingdist/.