This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

Staxa is a multi-tenant deployment platform I am building solo under Stackforge Labs. The backend is a single Go binary (staxad) using the chi router, with about 60 API endpoints, running on K3s on a Hetzner CAX21 ARM64 server that costs around $11/month. Each tenant gets an isolated Kubernetes namespace with their own app container, a PostgreSQL 16 or MySQL 8 database, a subdomain with automatic SSL, and resource quotas. Container builds run through Buildah, and the frontend is Next.js (App Router) with shadcn/ui and Clerk for auth.

Bug Fix or Performance Improvement

The symptom: POST /api/v1/tenants/{id}/deployments/{depId}/rollback accepted a deployment ID in the URL path and then completely ignored it. Whatever version you asked for, you got the most recent successful deployment instead.

The route was wired up correctly in internal/api/router.go:149:

r.Post("/tenants/{id}/deployments/{depId}/rollback", srv.handleRollbackDeployment)

Enter fullscreen mode Exit fullscreen mode

But handleRollbackDeployment never called chi.URLParam(r, "depId"). It read {id} for the tenant and stopped there.

How I found it: I was auditing my published API docs against the actual handlers, endpoint by endpoint. When I got to the rollback entry I went to write down what {depId} did, went to the handler to confirm, and found nothing reading it. The docs described an ID that the code never looked at.

The worst part is that it returned 202 Accepted and then performed a real, successful rollback. Just not the one you asked for. There was no error to notice, no failed request in any log. The frontend had been passing the deployment ID into the URL since it was written (src/lib/api.ts), so the UI always believed the parameter was honored.

Root cause: the handler created a rollback deployment row with no reference to any target, and the worker independently decided what to restore. In internal/worker/pipeline.go, runRollbackPipeline did this:

prevDep, err := cfg.Store.GetPreviousDeployment(ctx, tenant.ID)
if err != nil {
    return failPipeline(ctx, cfg, dep, "deploy",
        fmt.Errorf("no previous successful deployment to roll back to: %w", err))
}

Enter fullscreen mode Exit fullscreen mode

GetPreviousDeployment was a single query: most recent deployment for this tenant where status='success' AND is_current=FALSE, ordered by version descending, limit 1. It is a perfectly good "roll back one version" implementation. It just is not "roll back to the version the user named," and nothing in between the HTTP layer and the worker was carrying that information. (That query has since changed again, for reasons in the last section.)

Why it mattered: rollback is the button you press when production is broken. If v9 is bad you might roll back to v8, find that also bad, and go for v5. On Staxa you would click v5 and get v8 again, three times in a row, while your tenant stayed down. Meanwhile the deployment history would show three successful rollbacks.

The row it wrote was misleading too. It hardcoded ImageTag: "rollback", a placeholder that is not a real image reference, so the deployment history could not tell you what had actually been deployed even after the fact.

Then fixing it exposed a second, older bug underneath. More on that below.

Code

Three PRs on staxa-api-v2:

The handler, before and after

internal/api/deployments.go. Before:

func (s *Server) handleRollbackDeployment(w http.ResponseWriter, r *http.Request) {
    provider := middleware.GetProvider(r.Context())
    tenantID := chi.URLParam(r, "id")
    // depId is never read

    if err := s.assertTenantOwnership(r, provider.ID, tenantID); err != nil {
        pkgapi.ErrNotFound(w, r, "tenant not found")
        return
    }

    version, err := s.store.GetNextDeploymentVersion(r.Context(), tenantID, "")
    if err != nil {
        pkgapi.ErrInternal(w, r)
        return
    }

    d := &store.Deployment{
        ID:         id.Generate("dep"),
        TenantID:   tenantID,
        ProviderID: provider.ID,
        DeployType: "rollback",
        Trigger:    "api",
        Status:     "pending",
        ImageTag:   "rollback",
        Version:    version,
        Metadata:   map[string]any{},
        CreatedAt:  time.Now(),
    }
    // ... create + enqueue
}

Enter fullscreen mode Exit fullscreen mode

After:

func (s *Server) handleRollbackDeployment(w http.ResponseWriter, r *http.Request) {
    provider := middleware.GetProvider(r.Context())
    tenantID := chi.URLParam(r, "id")
    depID := chi.URLParam(r, "depId")

    if err := s.assertTenantOwnership(r, provider.ID, tenantID); err != nil {
        pkgapi.ErrNotFound(w, r, "tenant not found")
        return
    }

    target, err := s.store.GetDeployment(r.Context(), depID)
    if err != nil || target.TenantID != tenantID {
        pkgapi.ErrNotFound(w, r, "deployment not found")
        return
    }
    if err := validateRollbackTarget(target); err != nil {
        pkgapi.ErrUnprocessable(w, r, err.Error())
        return
    }

    version, err := s.store.GetNextDeploymentVersion(r.Context(), tenantID, "")
    if err != nil {
        pkgapi.ErrInternal(w, r)
        return
    }

    // The new deployment carries the target's image and source so the row is an
    // honest record of what ends up running; the pipeline reads the target back
    // out of metadata to decide what to deploy.
    d := &store.Deployment{
        ID:         id.Generate("dep"),
        TenantID:   tenantID,
        ProviderID: provider.ID,
        ImageTag:   target.ImageTag,
        SourceRef:  target.SourceRef,
        SourceType: target.SourceType,
        DeployType: "rollback",
        Trigger:    "api",
        Status:     "pending",
        Version:    version,
        Metadata: map[string]any{
            "rollback_to":         target.ID,
            "rollback_to_version": target.Version,
        },
        CreatedAt: time.Now(),
    }
    // ... create + enqueue
}

Enter fullscreen mode Exit fullscreen mode

The validation rules are their own function so both the handler and the worker can apply them:

// validateRollbackTarget reports whether a deployment can be rolled back to.
// Only a deployment that finished successfully has an image known to run, and
// only a tenant-level one can be restored by the rollback pipeline (that
// pipeline redeploys the tenant's primary workload, so restoring one service's
// image through it would deploy that image tenant-wide).
func validateRollbackTarget(d *store.Deployment) error {
    if d.Status != "success" {
        return errors.New("can only roll back to a deployment that completed successfully")
    }
    if d.ServiceID != "" {
        return errors.New("cannot roll back to a service-scoped deployment, redeploy the service instead")
    }
    // "pending" and "rollback" are placeholders written at create time; a
    // deployment carrying one never had a real image assigned to it.
    switch d.ImageTag {
    case "", "pending", "rollback":
        return errors.New("deployment has no image to roll back to")
    }
    return nil
}

Enter fullscreen mode Exit fullscreen mode

The worker

internal/worker/pipeline.go. The GetPreviousDeployment call became:

// resolveRollbackTarget returns the deployment whose image this rollback should
// restore. The API records the caller's chosen target under "rollback_to" in the
// rollback deployment's metadata; jobs enqueued before that field existed carry
// no target, so those fall back to the most recent successful deployment.
func resolveRollbackTarget(ctx context.Context, cfg WorkerConfig, dep *store.Deployment, tenantID string) (*store.Deployment, error) {
    targetID, _ := dep.Metadata["rollback_to"].(string)
    if targetID == "" {
        prev, err := cfg.Store.GetPreviousDeployment(ctx, tenantID)
        if err != nil {
            return nil, fmt.Errorf("no previous successful deployment to roll back to: %w", err)
        }
        return prev, nil
    }

    target, err := cfg.Store.GetDeployment(ctx, targetID)
    if err != nil {
        return nil, fmt.Errorf("rollback target %s not found: %w", targetID, err)
    }
    // Re-check what the handler checked. The target could have been deleted or
    // rewritten between enqueue and execution, and deploying the wrong image is
    // worse than failing the rollback.
    if target.TenantID != tenantID {
        return nil, fmt.Errorf("rollback target %s belongs to another tenant", targetID)
    }
    // Mirrors validateRollbackTarget: this pipeline deploys the result as the
    // tenant's primary workload, so a service-scoped row would be pushed
    // tenant-wide and then marked current in the service's is_current scope.
    if target.ServiceID != "" {
        return nil, fmt.Errorf("rollback target %s is service-scoped and cannot be restored tenant-wide", targetID)
    }
    switch target.ImageTag {
    case "", "pending", "rollback":
        return nil, fmt.Errorf("rollback target %s has no image to restore", targetID)
    }
    return target, nil
}

Enter fullscreen mode Exit fullscreen mode

That ServiceID check is a late addition. It came out of code review, and the story behind it is below.

The schema bug underneath (PR #22)

internal/store/migrations/004_deployments.sql:28, untouched since I wrote it:

CREATE UNIQUE INDEX idx_deployments_current ON deployments(tenant_id, is_current) WHERE is_current = TRUE;

Enter fullscreen mode Exit fullscreen mode

At most one current deployment per tenant. Migration 027 replaces it:

DROP INDEX IF EXISTS idx_deployments_current;

CREATE UNIQUE INDEX idx_deployments_current
    ON deployments (tenant_id, COALESCE(service_id, ''))
    WHERE is_current = TRUE;

Enter fullscreen mode Exit fullscreen mode

And SetCurrentDeployment in internal/store/deployments.go went from clearing the flag across the whole tenant:

// before
tx.Exec(ctx, `UPDATE deployments SET is_current=FALSE WHERE tenant_id=$1 AND is_current=TRUE`, tenantID)
tx.Exec(ctx, `UPDATE deployments SET is_current=TRUE WHERE id=$1`, deploymentID)

Enter fullscreen mode Exit fullscreen mode

to deriving its scope from the target row, in one statement:

// after
_, err := s.pool.Exec(ctx, `
    UPDATE deployments d SET is_current = (d.id = $2)
    FROM deployments t
    WHERE t.id = $2
      AND d.tenant_id = $1
      AND COALESCE(d.service_id, '') = COALESCE(t.service_id, '')
      AND (d.is_current OR d.id = $2)`,
    tenantID, deploymentID,
)

Enter fullscreen mode Exit fullscreen mode

The signature is unchanged, so all five existing call sites got the correct scoping without edits.

My Improvements

Diagnosing it

The diagnosis was trivial once I looked. What is worth writing down is why I had not looked for four months. Rollback worked. I had tested it by deploying twice, clicking rollback, and watching the previous version come back up. With exactly two deployments, "the target you named" and "the most recent successful one" are the same row. My manual test could not distinguish the correct behavior from the bug, and I never went back with three versions.

Tracing it took two greps: grep -rn "depId" internal/api/ showed handleGetDeployment reading it and handleRollbackDeployment not, and reading runRollbackPipeline showed where the decision was actually being made.

How to get the target to the worker

Jobs go through a Redis list as queue.Job structs. The obvious move was adding a TargetDeploymentID field to queue.Job.

I put it in the deployment's Metadata map instead, for three reasons. Jobs are transient and the deployments table is not, so the metadata answers "what did that rollback restore?" months later, when the job is long gone. It survives a requeue, since the pipeline re-reads the deployment row by ID and the target comes back with it. And it is already serialized into the API response, so the caller sees rollback_to and rollback_to_version without me adding response fields.

The cost is that it is a map[string]any, so the read is an unchecked type assertion (dep.Metadata["rollback_to"].(string)) instead of a typed field. I decided the durability was worth losing the compile-time check, and resolveRollbackTarget treats a missing or non-string value as "no target given" and falls back, which is also the migration path for jobs already sitting in Redis when the binary is replaced.

Validating twice on purpose

validateRollbackTarget runs in the handler, and resolveRollbackTarget re-checks tenant ownership and the image tag in the worker. That is deliberate duplication. Between the enqueue and the execution the target row can be deleted or rewritten, and jobs can retry minutes later. Deploying the wrong image is worse than failing the rollback, so the worker does not trust the ID it was handed.

The bug the fix exposed

Rejecting service-scoped targets was correct for the pipeline as written, since runRollbackPipeline calls Orchestrator.DeployApp, which redeploys the tenant's primary workload. Restoring one service's image through it would push that image tenant-wide.

But it meant multi-service tenants now had no rollback at all, because every deployment after tenant creation is service-scoped. Before my fix the same call "worked" only by accident: GetPreviousDeployment filtered on tenant_id alone, so it could pick up a service-scoped row and deploy that service's image as the entire tenant. I had turned a silent corruption into a loud 422, which is an improvement, but it is still a regression from the user's side.

Chasing why a service row could surface there at all led to the index above. is_current was tenant-scoped at the schema level, left over from before Staxa supported multiple services per tenant. Migration 011 had already made the sibling version index service-aware, so this one was just missed.

The masking was almost funny: runServiceScopedRedeploy never called SetCurrentDeployment at all, so no service deployment was ever marked current, so the unique index was never violated and nobody noticed it was wrong. Two bugs cancelling out. Adding the missing call without fixing the index would have started throwing SQLSTATE 23505 on the second service deploy.

I confirmed the tests actually catch it by deleting migration 027 and re-running them:

--- FAIL: TestDeploymentsCurrentPerService/two_services_in_one_tenant_can_both_be_current
    Error: Received unexpected error:
    ERROR: duplicate key value violates unique constraint "idx_deployments_current" (SQLSTATE 23505)

Enter fullscreen mode Exit fullscreen mode

That check is the one I would skip if I were rushing, and it is the one that proves the test is testing anything.

What code review caught that I missed

I had fixed the index and written the service-scoped GetPreviousServiceDeployment, and I considered the scoping problem closed. A reviewer pointed out that I had made the new function service-safe and left its older sibling alone, and that the two now interacted badly.

GetPreviousDeployment still filtered on tenant_id, status='success' and is_current=FALSE, with nothing excluding service rows. Marking service deployments current, which my fix had just started doing, removes only each service's currently running row from candidacy. Every superseded service row remains a candidate. Worse, version is a per-(tenant, service) counter from GetNextDeploymentVersion, so ORDER BY version DESC was ranking independent sequences against each other: a service's v7 outranks a tenant-level v3.

So the exact failure I thought I had closed was still reachable. runRollbackPipeline would deploy that service's image as the whole tenant, then call SetCurrentDeployment, which after migration 027 marks it current in the service's scope, leaving the tenant scope with no current deployment at all.

The fix is one clause:

WHERE tenant_id=$1 AND service_id IS NULL AND status='success' AND is_current=FALSE

Enter fullscreen mode Exit fullscreen mode

Both problems collapse into it. The filter excludes service rows outright, and restricting to one scope makes the version ordering compare a single counter again, so the mixed-sequence problem disappears rather than needing its own fix.

The exposure was narrower than it first looked, and I want to be accurate about that: validateRollbackTarget in the handler already rejected service-scoped targets, and after this fix the API always writes rollback_to, so the only path that reached the bad query was the metadata-less fallback for jobs enqueued before that field existed. I fixed it rather than filing it, because the fallback is permanent code and the failure mode is silent. Reviewing my own diff also showed resolveRollbackTarget re-checking tenant and image tag under a comment reading "re-check what the handler checked" while skipping scope, which was the one check that mattered here. That is the ServiceID guard in the code above.

The lesson I took is narrower than "get your code reviewed." I had written the new function carefully and never re-read the old one next to it, because in my head the work was "add service scoping" rather than "make this table's queries agree about scope." Adding a dimension to a data model is not done when the new code handles it. It is done when the code that predates it has been checked against it.

The design problem underneath that

Once rollback restored the image you asked for, a different question surfaced: which configuration does it restore? It was deploying the old image with today's environment variables. If a bad env change caused the incident, rolling back did not undo it.

I went with Octopus Deploy's release model, since that matches how I think about deploys: a deployment is an immutable snapshot of its inputs, and rollback means deploying that snapshot again. That is PR #24, with refresh_config: true as the explicit escape hatch, mirroring Octopus's "update variables" action.

The interesting tradeoff was retention. Snapshots duplicate secret material, so keeping them forever means rotating a leaked secret never purges every copy. My first version kept the last three snapshots and fell back to current config beyond that.

The better answer came from noticing that the bound only ever existed because of secrets. Ports, resource sizes, health check config and non-secret env are small and not sensitive. My env_vars table already had an is_secret column, so I split the snapshot: config and the names of the secret variables are kept for the life of the deployment, and only the secret values are bounded. A rollback past the window restores everything else and resolves the secret names against their current values.

That also turned out to be more correct than replaying, not just cheaper. If you rotate an API key between v1 and v9, restoring v1's key breaks the app. For rotated credentials, resolving to current is the behavior you actually want.

One case stays genuinely lossy and now fails loudly: a variable that was secret then and does not exist now can be filled from neither side, so the rollback fails naming the keys rather than starting the app without values it needs.

Verifying

Unit tests with a mock store for the handler (target not found, target belonging to another tenant, unsuccessful target, placeholder image tag, happy path asserting metadata and image tag) and the pipeline (metadata target wins over GetPreviousDeployment, and a bad target deploys nothing at all, which I assert explicitly since "fails" and "fails after deploying the wrong thing" are very different outcomes).

Three existing tests broke when I made the change, all with 404s, because they only set the id URL param and never depId. That was a good sign: they had been passing against a handler that ignored the parameter.

The schema work needed real Postgres, since a mock store cannot catch a unique index regression. That went into the testcontainers-go suite behind -tags integration: two services in one tenant both holding a current deployment, promoting one service leaving the other untouched, tenant-level promotion not disturbing service rows, and GetPreviousServiceDeployment ignoring other services and the running deployment.

The scoping fix from review got TestGetPreviousDeploymentIgnoresServiceScopes, and I ran the same check on it by reverting only the SQL clause. It fails on the returned row's scope:

Error: Should be empty, but was svc_9697128d7951c74c24b2c46b
Messages: must not return a service-scoped row

Enter fullscreen mode Exit fullscreen mode

The setup deliberately deploys the service enough times to run its version counter past the tenant's, so the mixed-sequence ordering is what is under test rather than just the presence of service rows. A test that only created one service deployment would pass for the wrong reason.

What I would do differently

Two things.

First, my manual test was shaped so it could not fail. Two deployments made the correct behavior and the bug identical. Now when I test anything that selects one item from a history, I use at least three and pick the middle one, because off-by-one and ignore-the-parameter bugs both hide at n=2.

Second, I found this by reading my own API docs against my own code. That audit found several other drifts the same afternoon (a response envelope documented with the wrong shape, an endpoint documented as replacing a set of environment variables when the SQL is INSERT ... ON CONFLICT DO UPDATE and never deletes anything). Writing docs from the handler rather than from memory is a decent bug-finding technique on its own, and I would rather do it on a schedule than stumble into it.

Third, when I add a dimension to a data model I now grep for every query against that table instead of trusting that I have thought of them. The review catch above cost one clause to fix and would have cost nothing to find, if I had listed the callers rather than reasoning about the two functions I happened to be looking at.

The broader lesson is about silent success. None of these bugs produced an error. The original returned 202 every time, the schema bug was masked by a second bug that kept it from ever firing, and the query scoping issue would have left a tenant with no current deployment without anything logging that. The ones that hurt are not the ones that throw. They are the ones that confidently do the wrong thing and report that it went fine.