Skip to content
L-04Journal / ArticleEL +0.00 m
Cloud & DevOps11 min read

GitHub Actions in Production, Part 1: The Build Gate That Wasn't Guarding Anything

A CI workflow that ran on every push to the production branch, went green for months, and protected nothing — because it fired at the same moment as the deploys it was supposed to gate. On the difference between reporting and enforcement, and why the trigger matters more than the steps.

By Mohammed Mostafa · Published

GitHub ActionsCI/CDTypeScriptpnpmNode.jsDevOpsContinuous Integration

We had a workflow called test-build.yml. It ran on every push to live, installed dependencies with pnpm, ran pnpm build, and went green. It did that for months before I looked at it properly and realised it wasn't protecting anything at all.

Thirty lines. No test runner, no linter, no coverage threshold. Just a compile.

Introduction#

I'm starting the series with this one because it's the least impressive workflow I've written and the one I learned the most from. Everything I got wrong here was invisible. The pipeline was green, the deploys worked, nobody complained. The problem was structural, and structural problems don't announce themselves.

The short version: a build check that runs after the merge isn't a gate. It's a report. I spent time tuning the steps inside a workflow whose trigger made the steps irrelevant, which is a specific kind of wasted effort that I now watch for.

Why this workflow exists#

Everything in this stack is TypeScript on Node, and before there was any CI at all, the most common way we broke production wasn't a logic bug. It was a build error that only showed up on the server.

The shape of it was always the same. Someone renames a field on an interface. Their editor is fine with it because the dev server runs through a transpile-only path that strips types without checking them, so nothing complains locally. They push. The deploy script SSHes into the box, runs npm run build, and tsc dies on some file they never opened, three imports downstream of the thing they renamed.

Now you've got a half-deployed service. The git pull worked. node_modules is updated. The build output is stale or missing. And you found out about it because the deploy script printed a stack trace at eleven at night.

That's a bad failure mode, and not because the error is hard to fix. It's bad because you're not preventing a bad release anymore, you're recovering from a partial one. Different problem, much worse timing.

So the goal was narrow: prove tsc succeeds on a clean checkout with a fresh install, on a machine that isn't mine, before that same command runs on a box serving traffic.

Notice what's not in there. No unit tests. No linting. I scoped it to the failure I was actually having rather than the checks that would look good in a README, and I'd still defend that. Starting with the check that maps to your real incidents beats starting with the check that maps to your insecurity about not having enough checks.

What I won't defend is the name. test-build.yml, titled "Test Build TypeScript Project", running no tests. That's a small lie in a filename, and it costs you the first time someone opens the file expecting a test suite and finds a compile.

Architecture#

One job, one runner, four steps in a line. No matrix, no fan-out, nothing uploaded.

snippettext
push → live  ──┐
               ├──► ubuntu-latest ──► checkout ──► pnpm ──► node 20 (+cache)
workflow_dispatch ─┘                                            │
                                                                ▼
                                                    pnpm install ──► pnpm build
                                                                       │
                                                        pass ──────────┴────────── fail
                                                          │                          │
                                                   (deploy already                (red X on a
                                                    running in parallel)          commit that
                                                                                  already merged)

I drew the flaw into the diagram on purpose. This workflow triggers on push to live, which is the same event that fires the Docker build, the ECS deploy, the EC2 deploy, and the S3 backup. Six workflows, one event, all starting at the same instant. Nothing connects them.

So when the build fails, the deploy is already running. The gate and the thing it's supposedly gating are siblings. There's no parent-child relationship anywhere in the setup.

The obvious response is "make it a required check on pull requests," and that is the right answer, but I want to be honest that it isn't free. Branch protection means no direct pushes to live, every change goes through a PR, and every hotfix waits for CI to finish before you can merge it. On a two-person team pushing fixes to a live property, that friction is real, and plenty of teams look at it and reasonably decide speed matters more.

My actual mistake wasn't choosing speed. It was never making the choice. The friction never got weighed, it just never got confronted, and "we didn't think about it" is a worse position than either option.

Step-by-step explanation#

The trigger:

snippetyaml
on:
  push:
    branches: ["live"]
  workflow_dispatch:

workflow_dispatch is the underrated half of that. It puts a manual re-run button on the Actions tab, and the reason that matters is what it replaces. Without it, re-running a pipeline means pushing an empty commit. Every chore: retrigger CI sitting in your production branch history is a small permanent tax on git log and a landmine for git bisect.

Runner is ubuntu-latest. For a Node compile that's correct and I wouldn't change it. The floating tag means GitHub upgrades the image underneath me without asking, which is technically a supply chain surface, but this job holds no credentials and touches nothing, so I'll take the maintenance saving.

Then pnpm before Node, and the order is load-bearing:

snippetyaml
- uses: pnpm/action-setup@v4
  with: { version: 9 }

- uses: actions/setup-node@v4
  with:
    node-version: "20"
    cache: 'pnpm'

People get this backwards constantly. setup-node with cache: 'pnpm' shells out to pnpm store path to find the content-addressable store it's meant to be caching, so pnpm has to already be on PATH. Swap those two steps and you get Error: Unable to locate executable file: pnpm, which points at the Node setup step while the actual cause is the step below it. I've watched two different people lose twenty minutes to that error message.

What you get for the correct ordering is pnpm's global store cached against the lockfile hash. On a commit where dependencies haven't moved, install drops from tens of seconds to a couple, and because pnpm hard-links out of the store instead of copying files, the restore is cheap in a way that a lot of "cached" installs aren't.

Then the install itself:

snippetyaml
- run: pnpm install

This is the line I'd change first, and it's one flag. Bare pnpm install will happily rewrite pnpm-lock.yaml if the lockfile and package.json have drifted apart. In CI that's backwards. It means the pipeline quietly resolves a dependency tree that nobody committed, and the build you just validated isn't the build anyone else will get.

--frozen-lockfile fails loudly instead of silently fixing it.

Now, pnpm does default --frozen-lockfile to true when CI=true, and GitHub Actions sets that. So in practice this is safer than it reads. I still want it written down. Relying on an implicit environment-dependent default for something that determines whether your build is reproducible feels wrong, and the moment anyone copies this template into a CI system that doesn't set CI, the protection vanishes without a word.

The build:

snippetyaml
- run: pnpm build

Which runs tsc underneath, and that's the whole gate. Type checking earns its place here. It won't tell you the code does the right thing, but it catches an entire family of refactor breakage that tests routinely miss, and it does it across every file rather than only the paths someone bothered to write assertions for. Tests check the behaviour you thought of. The compiler checks every consumer of every symbol you touched.

Last, the permissions:

snippetyaml
permissions:
  contents: write
  id-token: write

This is too much and I'd fix it today. A job that checks out code and compiles it needs contents: read. contents: write hands the run's GITHUB_TOKEN the ability to push commits, move tags, and edit releases. id-token: write mints OIDC tokens for cloud federation that this workflow never does.

Neither is exploitable by itself. But the entire argument for least privilege in CI is that you don't get to know in advance which package in your dependency tree turns hostile, and permissions is about the cheapest control GitHub gives you. There's no reason to leave it open.

Interesting implementation details#

The thing I keep coming back to is that if you're only going to run one check on a TypeScript codebase, tsc is probably the right one. It needs no fixtures, no test database, no setup. It has close to zero false positive rate, its failures are unambiguous, and it's whole-program. That's a lot of coverage for pnpm build.

The other detail worth pulling out is the six-hour default job timeout, which I didn't know about for an embarrassingly long time. A pnpm install hanging against a wedged registry connection will sit there burning a runner slot for a full working day before GitHub kills it. timeout-minutes: 10 on a job that normally takes ninety seconds turns a silent resource leak into a fast obvious failure, and it costs one line.

Common mistakes#

The big one is treating "the build ran" as "the change is safe." A green compile says your types line up. It says nothing about whether the code does what it's supposed to. Naming the workflow "Test Build" actively invites that confusion, including from the person who wrote it, which in this case was me.

Second is the one this whole article is about: putting the gate downstream of the merge. A check on push: live is reporting. A check on pull_request with branch protection is enforcement. The YAML is nearly identical and the guarantee isn't remotely the same.

Then there's the setup-node ordering, where the error message misdirects you. Bare install in CI, where lockfile drift resolves silently instead of failing. And copying permissions blocks between workflows, which is how least privilege actually decays in practice. Nobody decides to over-permission a job. They paste a block from a workflow that needed it into one that doesn't, and nothing breaks, so nobody removes it.

One that's subtler: assuming a cache hit means a correct cache. cache: 'pnpm' keys on the lockfile hash, so if you're not enforcing the lockfile, you can end up restoring a store built from a resolution that no longer matches what you're about to install.

Lessons learned#

Design the trigger before you design the steps. I put real thought into the pnpm and Node ordering and zero thought into whether push: live was the right event, and the trigger made all of that careful work decorative. Now the first question I answer for any new workflow is what it's allowed to prevent, and everything else follows from that.

Scope to your real incidents, then be honest about what you scoped to. Building a compile-only gate because compile errors were the actual problem was fine engineering. Calling the result "test-build" wasn't. Names are the cheapest documentation you'll ever write, and vague ones are how a team ends up believing it has coverage it doesn't have.

The cheapest controls are the ones you skip. permissions: contents: read. timeout-minutes: 10. --frozen-lockfile. Three lines, no ongoing maintenance, real risk reduction. They get skipped because nothing visibly breaks without them, which is exactly the property that makes them worth adding deliberately rather than waiting to need them.

And a fast pipeline is a pipeline people actually use. The store cache isn't a micro-optimisation. Once CI takes more than a couple of minutes, people stop waiting for it before they merge, and a check nobody waits for has stopped being a check.

Production considerations#

The ordering problem has a middle path I didn't see at first. Running the gate in parallel with the deploys is fast and wrong. Chaining everything with needs: is correct and serialises your deploy behind a compile. What actually works is keeping this as a fast PR gate and letting the deploy workflows fire unconditionally on live, because if the merge was already blocked by the PR check, the code reaching live has been verified. You get enforcement without adding latency to the deploy path.

On secrets: this job doesn't touch any, which is why the over-broad permissions block is latent rather than active. That distinction is worth sitting with. It's fine today because of a property of the job that one added step could change, and nobody's going to re-audit the permissions block when they add that step.

Cost is real on private repos. Public repos get free minutes, private ones don't, and a six-hour ceiling on a job that fires on every push to your production branch is a billing exposure rather than a theoretical one.

Supply chain, finally. Every action here is pinned to a major version tag, and tags are mutable. A compromised or force-moved @v4 executes attacker code inside a job that currently has contents: write. Pinning to full commit SHAs with Dependabot handling the bumps removes that entirely and costs you nothing except uglier YAML.

Improvements#

Roughly in order of what I'd do first.

Move the trigger to pull_request and make it a required status check. That's the whole thing. It converts the workflow from observation into enforcement and everything below it is detail. Keep push: live alongside if you want a post-merge sanity check.

Add the checks the name already promises, as separate jobs so the failures are individually readable:

snippetyaml
jobs:
  verify:
    strategy:
      fail-fast: false
      matrix:
        task: [lint, typecheck, test]
    steps:
      # ... setup ...
      - run: pnpm ${{ matrix.task }}

fail-fast: false is the part people leave off. You want every failure from one run, not just whichever one lost the race.

Tighten the job contract:

snippetyaml
permissions:
  contents: read
timeout-minutes: 10
concurrency:
  group: ci-${{ github.ref }}
  cancel-in-progress: true

The concurrency block kills superseded runs when someone pushes three times in a row, which cuts queue contention and gets you feedback faster on the commit that actually matters.

After that: --frozen-lockfile explicitly, SHA-pin the actions with Dependabot configured for github-actions, and convert the whole thing to a workflow_call reusable workflow with node-version and pnpm-version inputs. That last one matters more than it sounds, because right now this job is copy-pasted across every Node service I run, and every copy diverges the moment someone fixes a bug in one of them and not the others.

Then rename it. ci.yml, or build.yml until it genuinely runs tests. The file is currently writing a cheque it can't cash.

Next: release.yml, which is thirty-two lines and is probably the highest-leverage thing in the whole library.

Why is a GitHub Actions check on push different from a check on pull_request?
A check triggered by push to your production branch runs after the merge has already happened, so it can only report a failure. A check triggered by pull_request, combined with branch protection, blocks the merge itself. The YAML is nearly identical; only the guarantee differs.
Why must pnpm/action-setup run before actions/setup-node?
actions/setup-node with cache set to pnpm shells out to `pnpm store path` to locate the store it caches, so the pnpm binary must already be on PATH. Reversing the two steps produces `Unable to locate executable file: pnpm`, an error that points at the Node step while the real cause is the step below it.
Should CI use pnpm install or pnpm install --frozen-lockfile?
Use --frozen-lockfile. Bare `pnpm install` rewrites pnpm-lock.yaml when it has drifted from package.json, so the pipeline validates a dependency tree nobody committed. pnpm defaults the flag to true when CI=true, but stating it explicitly keeps the guarantee when the workflow is copied elsewhere.
What permissions does a build-only GitHub Actions job need?
Only `contents: read`. Granting `contents: write` lets the run's GITHUB_TOKEN push commits, move tags, and edit releases, and `id-token: write` mints OIDC tokens for cloud federation a compile job never performs. Over-broad permissions blocks usually arrive by copy-paste rather than by decision.
What is the default timeout for a GitHub Actions job?
Six hours. A dependency install hanging against an unresponsive registry will hold a runner slot for a full working day before GitHub kills it. Setting timeout-minutes to roughly ten on a job that normally takes ninety seconds turns a silent resource leak into a fast, obvious failure.