Skip to content

Add GitHub Actions CI workflow - #29

Merged
Railly merged 2 commits into
mainfrom
afk/issue-28-add-github-actions-ci-workflow-lint-typecheck-re
Apr 27, 2026
Merged

Railly merged 2 commits into
mainfrom
afk/issue-28-add-github-actions-ci-workflow-lint-typecheck-re

Conversation

@Railly

@Railly Railly commented Apr 27, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a basic GitHub Actions CI workflow for pushes to main and pull requests
  • Install Bun 1.2.2, install dependencies with the frozen lockfile, run lint, typecheck, and registry build
  • Add the typecheck script and update CLAUDE.md development commands to Bun-first examples

Closes #28

Verification

  • bun install --frozen-lockfile --force
  • bun run typecheck
  • bun run build:registry
  • bun run test:run
  • bun run lint (currently fails on pre-existing tracked lint/format diagnostics outside this slice)

@vercel

vercel Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
elements Ready Ready Preview, Comment Apr 27, 2026 3:09am

@Railly

Railly commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Adversarial Review Report

Verdict: REVISE

  • FATAL: 0
  • MAJOR: 4
  • MINOR: 5

Issues

1. [MAJOR] logic

Location: .github/workflows/ci.yml
Problem: The CI workflow does not pin a Node.js version via actions/setup-node. While oven-sh/setup-bun@v2 installs Bun, tsc --noEmit and next-related tooling typically rely on a Node runtime being present. GitHub-hosted ubuntu-latest runners ship with a default Node, but it can drift over time and across runner image updates, causing nondeterministic CI. For a workflow whose stated goal is 'fresh git clone lands green', pinning Node alongside Bun is a normal hardening step.
Fix: Add an actions/setup-node@v4 step with a pinned node-version (e.g. 20.x) before the bun install step, or document why relying on the runner's default Node is acceptable.

2. [MAJOR] coverage

Location: .github/workflows/ci.yml (setup-bun step)
Problem: No caching is configured for Bun's install cache or for Next.js/registry build artifacts. The acceptance criteria do not require it, but every CI run will perform a cold bun install and a cold build:registry, which slows feedback and increases the chance that flaky network hiccups during install fail PRs unnecessarily. setup-bun@v2 supports a built-in cache option.
Fix: Either pass cache: true (or equivalent) to oven-sh/setup-bun@v2, or add an explicit actions/cache step keyed on bun.lockb. At minimum, note in the PR that caching was deliberately deferred.

3. [MAJOR] logic

Location: .github/workflows/ci.yml (top-level)
Problem: The workflow lacks a top-level permissions: block. GitHub's default GITHUB_TOKEN permissions vary by repo setting and have historically been overly broad. For a CI-only workflow that does no writes, explicit least-privilege (permissions: contents: read) is a standard hardening practice and is especially relevant since the issue explicitly mentions 'PRs from external contributors' — i.e., pull_request events from forks.
Fix: Add permissions:\n contents: read at the workflow root.

4. [MAJOR] logic

Location: .github/workflows/ci.yml (top-level)
Problem: No concurrency group is defined. On an active PR, every push will spawn a new CI run while previous in-flight runs continue, wasting minutes and potentially reporting stale results. This is a near-universal pattern for PR CI workflows.
Fix: Add a concurrency block, e.g. concurrency:\n group: ci-${{ github.ref }}\n cancel-in-progress: true.

5. [MINOR] coverage

Location: .github/workflows/ci.yml (on.pull_request)
Problem: The pull_request trigger has no event-type filter. The default is [opened, synchronize, reopened], which is usually fine, but worth being explicit if external-contributor PRs are a stated concern (e.g., should pull_request_target be considered, or explicitly rejected for security reasons?). The PR/issue does not discuss this trade-off.
Fix: Either leave defaults and add a one-line comment confirming the choice, or explicitly list event types. Explicitly note that pull_request_target was rejected (it would expose secrets to fork PRs).

6. [MINOR] logic

Location: .github/workflows/ci.yml (jobs.ci)
Problem: The job has no timeout-minutes. A hung step (network, infinite loop in registry build) will run for the GitHub default of 360 minutes (6 hours), burning Actions minutes.
Fix: Add timeout-minutes: 15 (or similar) at the job level.

7. [MINOR] logic

Location: .github/workflows/ci.yml (steps)
Problem: Step names are not provided (name: is omitted on every step). The Actions UI will show raw run commands, which is fine but noisier than necessary. For a workflow others will read when CI fails, named steps ('Install', 'Lint', 'Typecheck', 'Build registry') improve diagnosability.
Fix: Add a name: field to each step.

8. [MINOR] coverage

Location: CLAUDE.md (Development Commands section)
Problem: The acceptance criteria require updating CLAUDE.md's 'Development Commands' section, and the diff does so — but it drops the human-readable comment lines (# Start development server, etc.) entirely rather than translating them. A reader of the new CLAUDE.md sees a bare command list with no labels. The original intent of the section appears to be onboarding documentation, not a script reference.
Fix: Either preserve the comment labels above each command (e.g., # Start development server\nbun dev), or confirm with Hunter that the bare list is the intended final form.

9. [MINOR] logic

Location: Verification section
Problem: The verification plan ('We will verify by checking the Actions tab after merge') is post-merge. There is no pre-merge dry-run validation — e.g., act locally, or pushing to a throwaway branch first to confirm the workflow file syntax and that bun run typecheck actually passes against the current codebase. If tsc --noEmit surfaces existing latent type errors in main, this PR will turn CI red on day one and block all future PRs. The PR/research does not mention having run bun run typecheck locally to confirm a clean baseline.
Fix: Before merge, run bun install && bun run typecheck && bun run build:registry locally on a fresh clone to confirm the new CI will be green. Document the result in the PR.


Generated: 2026-04-27T02:15:47.986Z

@Railly

Railly commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Adversarial Review Report

Verdict: REVISE

  • FATAL: 0
  • MAJOR: 4
  • MINOR: 4

Issues

1. [MAJOR] logic

Location: .github/workflows/ci.yml — missing actions/setup-node step
Problem: The workflow does not pin a Node.js version. tsc --noEmit and next build-adjacent tooling rely on Node, and oven-sh/setup-bun@v2 does not install Node. CI will use whatever Node is preinstalled on the ubuntu-latest runner, which can drift over time and produce non-reproducible failures vs. local dev. The acceptance criteria don't explicitly require it, but a CI workflow whose stated goal is 'a fresh git clone lands green' should pin Node alongside Bun.
Fix: Add - uses: actions/setup-node@v4 with node-version matching the repo's .nvmrc or engines.node (or document why ubuntu-latest's default Node is acceptable). At minimum verify tsc runs against an intended Node version.

2. [MAJOR] logic

Location: .github/workflows/ci.yml — top-level keys
Problem: No concurrency group is defined. On active PRs, every push will spawn a new CI run while previous runs continue, wasting Actions minutes and slowing feedback. This is standard practice for any CI workflow that runs on pull_request.
Fix: Add concurrency: { group: ci-${{ github.ref }}, cancel-in-progress: true } so superseded runs are cancelled.

3. [MAJOR] coverage

Location: Verification report — no local run of bun run typecheck documented
Problem: The PR did not verify that bun run typecheck (i.e., tsc --noEmit) actually passes on the current main. The acceptance criteria added the script but the report shows no evidence the codebase is currently type-clean. If there are pre-existing TS errors, the very first CI run on main will be red, defeating the 'lands green' verification goal.
Fix: Run bun run typecheck locally on the branch before merge and confirm exit 0. If errors exist, either fix them in this PR or split into a precursor issue, otherwise CI will be broken on day one.

4. [MAJOR] logic

Location: .github/workflows/ci.yml — on.pull_request
Problem: pull_request trigger without filters means CI runs on draft PRs and on every label/assignee/title edit. For a workflow this lightweight it's borderline, but most repos scope to pull_request: { types: [opened, synchronize, reopened, ready_for_review] } to avoid runs on metadata-only events.
Fix: Either accept the noise explicitly or scope event types. Not strictly required by acceptance criteria but worth flagging.

5. [MINOR] logic

Location: .github/workflows/ci.yml — missing top-level permissions
Problem: No permissions: block is set. GitHub's hardening guidance recommends declaring least-privilege permissions explicitly (permissions: contents: read) rather than relying on the repo/org default, which may grant write tokens to the workflow.
Fix: Add permissions: contents: read at the workflow level.

6. [MINOR] logic

Location: .github/workflows/ci.yml — jobs.ci
Problem: No timeout-minutes on the job. A hung bun install or tsc can burn the default 360-minute Actions timeout. Cheap to add, prevents runaway billing.
Fix: Add timeout-minutes: 15 (or similar) to the job.

7. [MINOR] coverage

Location: CLAUDE.md diff — single-file scope of npm→bun change
Problem: The PR adds CLAUDE.md updates but didn't verify whether other files in the repo (README.md, CONTRIBUTING.md, docs) still reference npm run …. The stated rationale ('this repo is bun-first, the npm references are wrong') applies equally to those files. Out of scope per the issue, but worth noting as follow-up.
Fix: Grep the repo for npm run and either fix in this PR or open a follow-up issue.

8. [MINOR] logic

Location: Verification section
Problem: The verification report states 'we will verify by checking the Actions tab after merge,' but the workflow only triggers on push to main and on pull_request. The PR itself should trigger a CI run before merge — verification should happen on the PR, not after. If the report literally means post-merge verification, that's the wrong loop: a red main is what this workflow is meant to prevent.
Fix: Verify on the PR's Actions tab before merging, not after.


Generated: 2026-04-27T02:47:57.319Z

@Railly
Railly marked this pull request as ready for review April 27, 2026 03:07
@Railly
Railly merged commit 8b40eb2 into main Apr 27, 2026
3 of 4 checks passed
@Railly
Railly deleted the afk/issue-28-add-github-actions-ci-workflow-lint-typecheck-re branch April 27, 2026 03:07
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.

Add GitHub Actions CI workflow (lint + typecheck + registry build)

1 participant