A deploy that an admin had rejected could go out anyway if someone pressed Retry on it. I found that during an audit of Railyard’s control plane and fixed it this morning. The bug was small. The reason it existed was not, and the fix changed how every deploy in the system starts.

The rule Retry forgot

Railyard has a setting called deploy approval. Turn it on for an app and a deploy started by someone below admin doesn’t run. It waits in a pending_approval state until an admin approves or rejects it. Teams turn it on for production apps so that every deploy gets a second pair of eyes.

A rejected deploy is stored as failed, with an error that says who rejected it. Here is what the Retry action did with it:

def retry
  unless @deploy.failed?
    return redirect_to(deploy_path(@deploy), alert: "Only failed deploys can be retried.")
  end
  if application.deploys.active.exists?
    return redirect_to(deploy_path(@deploy), alert: "A deploy is already in progress.")
  end

  attrs = { status: :queued, triggered_by: current_user, trigger: "retry", message: @deploy.message }
  attrs[:target_ref] = @deploy.target_ref if @deploy.trigger == "ref"
  new_deploy = application.deploys.create!(attrs)
  DeployJob.run(new_deploy)
  redirect_to deploy_path(new_deploy)
end

The rejected deploy passes the first check because it is failed. Nothing is running, so it passes the second. Then the action builds a new deploy with the same target ref, marks it queued, and hands it to the job. The ref the admin turned down goes out.

Retry was written to answer one question: can this failed deploy run again? It checked what that question needs. The approval rule belonged to a different question, “may this person start a deploy of this app right now?”, and that question was answered somewhere else. Retry never asked it.

Nineteen ways in

In late September I read the control plane end to end the way I’d review someone else’s code, writing down anything that would hurt as the system grew. One entry: “start a deploy” was implemented 19 times. There were 19 calls to deploys.create! and 18 calls to DeployJob.run, spread across controllers, service objects and models.

The rules themselves are short. The app must not be archived. No other deploy may be running or waiting for approval. The team must be under its hourly deploy cap. If the app requires approval and the person isn’t an admin, the deploy waits, and admins get a notification and a webhook. On top of those, a couple of softer checks (environment variables the first deploy needs, the app’s deploy window) can be overridden by a person who clicks “Deploy anyway”.

The model already had a method that enforced all of it. The dashboard’s Deploy button and the Canvas deploy action called it. The API’s deploy endpoint had copied each rule inline, correctly at the time it was written. Retry, scale and maintenance mode, in both the dashboard and the API, created deploy records themselves. And an older model method, used by templates, the Canvas builder, functions and imports, checked only that nothing else was running before it queued a deploy.

So the model had a strict way to start a deploy and a lax one, and the lax one had more callers. Which rules applied depended on which button you pressed. Each path had been added for a feature, by me, thinking about that feature, and each copied whatever subset of the rules came to mind that day. The Retry bug was the visible case of a general one.

Where a rule should live

Rails gives you two natural homes for a check like “this app needs approval”: the controller action handling the request, or the model that owns the data. Putting it in the action reads well when there is one action. It stops holding the moment a second way in appears, because a rule in a controller only applies to requests that pass through that controller.

Deploy approval is a fact about an application and its deploys. It has to hold whether the request comes from a browser, an API token, the MCP endpoint, or a background job importing an app from another platform. The only code every one of those reaches is the model. This is the core of the DHH style of Rails I follow: controllers translate HTTP into a call on the domain and translate the answer back into HTTP, and the domain objects carry the behavior. In domain-driven design terms, Application is the aggregate root for its deploys. You don’t create a deploy by inserting a row. You ask the application for one, and the application decides.

A model that owns every rule gets large, and that’s where concerns come in. A Rails concern is a module, with ActiveSupport::Concern handling the included and class-method plumbing, that groups one role of a model into its own file. Application includes Application::Deploying the same way it includes concerns for its GitHub integration and its environment variables. Everything about starting a deploy lives in that one file, inside the model’s namespace, and nothing in it is meant to be called from outside except the entry methods. A concern used this way is a chapter of the model. It is not a place to share helpers between unrelated classes, and it doesn’t replace a real object when a concept deserves one.

Here is the concern after the fix, trimmed to the entry method:

class Application
  module Deploying
    extend ActiveSupport::Concern

    Refused       = Class.new(StandardError)
    NeedsEnv      = Class.new(Refused)
    OutsideWindow = Class.new(Refused)

    def enqueue_deploy!(by: nil, admin: false, force: false, ignore_window: false, system: false, **attrs)
      raise Refused, "#{name} is archived, restore it first." if archived?
      raise Refused, "A deploy is already in progress for #{name}." if deploys.active.exists?
      raise Refused, "A deploy for #{name} is waiting for approval." if deploys.awaiting_approval.exists?
      raise OutsideWindow, "#{name} is outside its deploy window." unless system || ignore_window || in_deploy_window?
      check_needed_env!(force) unless system
      TeamLimits.for(team).check_deploy_rate! unless system

      gated  = !system && deploy_approval_required? && !admin
      deploy = deploys.create!(status: gated ? :pending_approval : :queued, triggered_by: by, **attrs)
      gated ? announce_awaiting_approval(deploy) : DeployJob.run(deploy)
      deploy
    rescue TeamLimits::Exceeded => e
      raise Refused, e.message
    rescue ActiveRecord::RecordNotUnique
      raise Refused, "A deploy is already in progress for #{name}."
    end
  end
end

Look at what a caller is allowed to say. It says who is asking and whether they are an admin. It can pass force and ignore_window, which map to buttons a person clicks. It cannot say “skip approval” or “skip the rate cap”. Those are decided inside. The one exception is system: true, for redeploys Railyard starts by itself, and even a system redeploy still refuses an archived app.

Refusal is an exception, and every subclass of Refused carries a message written for a person. That keeps the contract plain: the method either returns a deploy, queued or pending, or it raises something a controller can show.

The rescue ActiveRecord::RecordNotUnique line is the other half of the design. The deploys table has a partial unique index on application_id where the status is queued or running. Two requests that both pass the “nothing is running” check in the same instant can’t both insert a row, and the loser gets the same “already in progress” refusal as everyone else. The model states the rule for people reading the code. The database enforces it under concurrency.

Controllers get shorter

The fix was mostly deletions in controllers. Retry now reads:

def retry
  return redirect_to(deploy_path(@deploy), alert: "Only failed deploys can be retried.") unless @deploy.failed?

  attrs = { trigger: "retry", message: @deploy.message }
  attrs[:target_ref] = @deploy.target_ref if @deploy.trigger == "ref"
  new_deploy = application.enqueue_deploy!(by: current_user, admin: current_membership.admin?, force: true, **attrs)

  notice = new_deploy.pending_approval? ? "Retry created, an admin must approve it before it runs." : "Retrying deploy."
  redirect_to deploy_path(new_deploy), notice: notice
rescue Application::Deploying::Refused => e
  redirect_to deploy_path(@deploy), alert: e.message
end

The “is something running” check is gone from the controller, because the model does it. Retrying a rejected deploy now creates a new pending deploy, and an admin sees it again.

The older lax method became deploy!, a thin wrapper that calls enqueue_deploy! and returns nil on refusal, which is what its callers already expected. The MCP tool calls enqueue_deploy! directly and returns the refusal message as its text. Every entry point rescues the same Refused error and renders it in its own transport: a flash alert in the dashboard, a JSON error with a 4xx status in the API, a plain string for MCP. Git pushes to an archived app are now ignored at the webhook, with a reason in the response.

Why one door is easier

A question about the rules now has one place to be answered. “When does a deploy wait for approval?” used to mean reading 19 call sites and working out which ones a given user could reach. Now it means reading one method. Review gets simpler the same way: a new feature that starts a deploy should contain a call to enqueue_deploy! and nothing else, and a bare deploys.create! in a controller stands out in a diff.

It also changes the shape of bugs. If enqueue_deploy! has a mistake, every entry point has the same mistake. That sounds worse, and it is much easier to find and fix once. Nineteen slightly different implementations give you nineteen slightly different bugs, and you find them one at a time, usually from a user.

The parity test

A refactor like this only stays done if something stops the next feature from adding a twentieth door with its own rules. I wanted a test that fails if any entry point disagrees with the others.

The shape is a table. Rows are doors: each one a lambda that starts a deploy the way that entry point does, through the real route, controller and authentication. Columns are rules: each one a test that runs every door and checks what landed in the deploys table.

def doors
  {
    "dashboard deploy" => -> { post application_deploys_url(@app) },
    "scale"            => -> { patch scale_application_url(@app), params: { replicas: { worker: (@replicas += 1).to_s } } },
    "retry a rejected deploy" => -> {
      rejected = @app.deploys.create!(status: :failed, trigger: "ref", target_ref: "unapproved", error: "Rejected by Owner")
      post retry_deploy_url(rejected)
    },
    "API maintenance"  => -> { post maintenance_api_application_url(@app), headers: @auth },
    "MCP deploy_app"   => -> { post api_mcp_url, headers: @auth, as: :json, params: mcp_call("deploy_app", app: @app.name) },
    "template / builder / import" => -> { @app.deploy!(triggered_by: users(:deployer), trigger: "template") }
    # eleven doors in all
  }
end

test "an app that requires approval waits for an admin, whatever the door" do
  sign_in_as users(:deployer)
  @app.update_columns(deploy_approval_required: true)
  doors.each do |door, knock|
    reset!
    assert_equal ["pending_approval"], new_deploys(&knock).map(&:status), "#{door} skipped the approval gate"
  end
end

Three helpers make the table cheap to read. new_deploys stubs DeployJob.run, records the existing deploy ids, runs the block, and returns whatever new non-failed deploys appeared. reset! clears active deploys and maintenance mode between doors, so one door’s deploy can’t make the next door look refused. mcp_call builds the JSON-RPC envelope. The test never asks how a door is implemented. It only asks what ended up in the table, which is what a user would see.

If you write one of these, a few choices matter. Drive each door through its real transport (an HTTP request with a session or an API token) rather than calling the controller’s internals, or the test will happily pass for a door that misses a rule in a before_action. Name the doors in plain words, because the name is the failure message: “retry a rejected deploy skipped the approval gate” tells you exactly what broke without opening a debugger. And add positive tests next to the refusals. Mine check that an admin’s retry and scale of a gated app queue at once, and that a system redeploy skips the approval gate and the rate cap but still refuses an archived app. Without those, a door that refused everything would pass every rule.

There are three rule tests (archived, approval, rate cap) and two positive ones. Eleven doors against three rules (twelve for the archived rule, which also covers git push) is over thirty checks in a file under a hundred lines. Adding a door is one line in the hash, and it is checked against every rule at once. Adding a rule is one test, and it covers every door. I ran the table against the old code before the fix: every door that went around the strict method failed at least one row.

Retry on a rejected deploy now lands in pending_approval, and the test that says so runs on every commit.