Skip to content

build: build dist/ at release time instead of committing it to main #842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xdustinface wants to merge 1 commit into
base: main
Choose a base branch
from
Open

build: build dist/ at release time instead of committing it to main #842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
dist/* linguist-generated=true
dist/index.js* -diff
dist/** -diff linguist-generated=true
51 changes: 0 additions & 51 deletions .github/workflows/check-dist.yml
View file
Open in desktop

This file was deleted.

56 changes: 46 additions & 10 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,75 @@ name: Release
on:

@manki-review manki-review Bot May 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 ✨ 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"
}

push:
tags:
- 'v*'
- 'v[0-9]+.[0-9]+.[0-9]+'

permissions:
contents: write

concurrency:
group: release-${{ github.ref }}
cancel-in-progress: false

jobs:
release:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Resolve tag info
id: tag
run: |
TAG="${GITHUB_REF#refs/tags/}"
MAJOR=$(echo "$TAG" | grep -oE '^v[0-9]+')
echo "tag=$TAG" >> $GITHUB_OUTPUT
echo "major=$MAJOR" >> $GITHUB_OUTPUT

- name: Check whether tag already has built dist/
id: check
run: |
if git ls-tree -r HEAD --name-only | grep -q '^dist/'; then
echo "has_dist=true" >> $GITHUB_OUTPUT
else
echo "has_dist=false" >> $GITHUB_OUTPUT
fi

- uses: actions/setup-node@v4
if: steps.check.outputs.has_dist == 'false'
with:
node-version: '24'
cache: 'npm'

- run: npm ci
- run: npm run build
- if: steps.check.outputs.has_dist == 'false'
run: npm ci

- name: Update floating major tag
run: |
TAG="${GITHUB_REF#refs/tags/}"
MAJOR=$(echo "$TAG" | grep -oE '^v[0-9]+')
- if: steps.check.outputs.has_dist == 'false'
run: npm run build

- name: Commit dist/ and rewrite tag to the release commit
if: steps.check.outputs.has_dist == 'false'
run: |
TAG="${{ steps.tag.outputs.tag }}"
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"

@manki-review manki-review Bot May 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 ⚠️ Warning: Force-pushing the rewritten tag triggers an unintended second workflow run

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"
}

git add -f dist/
git commit -m "release: build artifacts for $TAG"
git tag -fa "$TAG" -m "$TAG"
git push origin "refs/tags/$TAG" --force

git tag -fa "$MAJOR" -m "Update $MAJOR tag to $TAG"
git push origin "$MAJOR" --force
- name: Update floating major tag
run: |
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"

@manki-review manki-review Bot May 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 📝 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

@manki-review manki-review Bot May 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 ✨ 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"
}

run: |
TAG="${GITHUB_REF#refs/tags/}"
TAG="${{ steps.tag.outputs.tag }}"
if gh release view "$TAG" &>/dev/null; then
echo "Release $TAG already exists — updating"
gh release edit "$TAG" --title "$TAG" --latest
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
node_modules/
dist/
*.js.map
*.d.ts.map
.idea/
Expand Down
Loading
Loading