The Fix That Armed the Hazard

The scheduled job had never once succeeded. That was the only reason it was safe.

Our testing service runs scheduled test tiers against the Righthand platform. Five tiers, various cadences, various depths of exercise. One of them, Tier 4, stands up a real, unflagged production Righthand rather than a test one, and it does that four times a day, at 15 minutes past 00:00, 06:00, 12:00, and 18:00 UTC. It is the only tier of the five that provisions with is_test=false.

In nine weeks on that schedule, Tier 4 has not once succeeded. 247 scheduled runs, from 27 May at 00:15 UTC through 28 July at 18:15 UTC, and 247 errors. Zero passes.

We are going to be careful with that sentence, because the careless version of it is wrong and a reader with access to the run rows could say so. Tier 4 has run 256 times in production, and two of those runs passed: 11 of 11 tests each, both triggered manually, both on 16 April, and both of them cleared the provisioning step. So the tier is not a thing that has never worked. It is a thing that has never worked on a schedule, which is the only version of it that matters here, because the schedule is what the release was about to arm.

Of the 254 production runs that errored, 208 died about 2.4 seconds in, at an auto-created-team lookup inside provisionUser. The error string, verbatim:

` Auto-created team not found for user : Cannot coerce the result to a single JSON object `

Both testuserid and testpersonaid come back null on those runs. No test user, no Righthand, nothing provisioned. Another 44 died even earlier, at a pool claim that returned 401, which is a second bug and not this story. The last two died earlier still, one at config load and one while trying to acquire a test user, both of them upstream of provisionUser and neither of them anywhere near the team lookup. 208 plus 44 plus 2 is 254, and the two April passes make 256. The accounting closes, which matters here: no errored run ever got past that lookup and died somewhere further along, so the lookup really was the floor.

What we are not going to tell you is that nothing ever leaked. We have not established what those two April runs created, and the file that carries is_test=false has no history before 1 May, so the tier may have had a different shape in April than it has now. The claim we can stand behind is the narrower one, and it carries the whole story anyway: 247 scheduled runs, 247 errors, and not one of them ever finished.

The crash was the safety gate. And we very nearly repaired the crash without noticing.

What the failure was actually protecting

is_test is one boolean, set at tiers/tier4/setup.ts:38. It is easy to read it as bookkeeping, a label that marks a row for later tidying. It is not. What it decides is whether the Righthand actually runs.

At runtime-service.ts:1449, the istest=true path materializes the queue items and then pauses the sandbox, or hands it back if a test already owns it. The istest=false path, which is the branch Tier 4 takes, upserts a sandbox heartbeat, starts the polling and watchdog loops, and launches the think script. So a Tier 4 Righthand is not a paused fixture waiting to be inspected. It is a live thinking one.

Everything else the flag gates follows from that:

• The stale-heartbeat and orphaned-lock health sweeps explicitly exclude test rows (health-sweep.ts:378, :975). An unflagged Righthand gets monitored and auto-recovered like a customer's.
• Learning review is disabled for test rows and enabled here (runtime-service.ts:2498).
• Capability tokens on test rows live at least an hour. Unflagged rows get the live TTL, 900 seconds by default (runtime-service.ts:3135).
• Our own internal lookups default to include_test=false (personas.ts:829, :1879), so a test row is hidden from the default listing while an unflagged one sits in it beside the real ones.

Two facts here cut against the tidy version of this story, so we are putting them in rather than leaving them out. First, istest defaults to false (03personas.sql:64). Unflagged is not a state Tier 4 opts into. It is the default state of the column, and Tier 4 simply never opts out. Second, the flag does not gate system updates at all: the is_test filter was deliberately removed from the system-update enqueue in April, in a migration whose own comment states the intent plainly, that every active row gets updated, test or not.

And one gap we are not going to write around, because it is the load-bearing one. The flag does not get the row cleaned up. The provisioning code mentions auto-cleanup, but retirement is actually driven by the testing service's own scheduledforretirement column rather than by any runtime branch on is_test. If four of these had been created a day, nothing about the flag would have swept them away afterwards.

That boolean is what makes Tier 4 the tier that has to work, and it is what makes Tier 4 the tier that must never work by accident.

The nine-week failure streak was not hidden. Every one of those 247 runs errored loudly. What it was not was surprising. A scheduled job that has never once succeeded is not a regression, it is just how the system looks. Nobody had seen a green scheduled Tier 4 run, ever, so nobody was watching for the day one showed up. From the outside, "the Tier 4 schedule has never worked" and "the Tier 4 schedule is turned off" were the same fact.

The fix that armed it

The fix was already written. Commit e990ca68, filed under ENG-670, repairs the provisioning path rather than the error message. The lookup was waiting for a team membership that nothing writes any more: a WorkOS auth cutover in May dropped the trigger that used to fan an auth user out into a profile, contacts, a team and a membership. The fix routes provisioning through provisionworkosuser(), the same RPC the real WorkOS callback uses, which creates the team inside its own transaction and returns its id, so the create path has nothing left to poll for. The retry helper and its failure string both survive, for the paths that read a membership written earlier. What changes is that the create path can no longer reach them. The condition that raised the error 247 times is not caught or handled or downgraded, it is gone: there is no longer a moment where the code asks for a team that does not exist yet. That distinction is the whole of this post. A handled error would have left the gate standing. An unreachable one removes it.

At the time we found it, the commit was not on main, but it was an ancestor of the 3.18.0 release branch head. In the ordinary course of merging that release, the fix would have landed. The provisioning path would have started working. And on the first scheduled Tier 4 window after the merge, the job that had errored 247 consecutive times would have run cleanly, created a real test user, then created a Righthand with is_test=false, and written it into production. Four times a day. At 00:15, 06:15, 12:15, and 18:15 UTC.

Nothing in the fix itself was wrong, and we are not second-guessing it here. Read on its own, e990ca68 is a small repair of a real failure. Read in the context of the tier it repairs, it is a change of policy: from "we do not provision unflagged production Righthands on a schedule" to "we do, four times a day, starting whenever this merges."

That change of policy had no ticket, no design discussion, and no owner. It was going to ship as a side effect of fixing a crash.

Two details that make this concrete

Two things about the surrounding code are worth naming, because they are the difference between a story about a near-miss and a story about a near-miss we can learn from.

The first is that the scheduler enqueued Tier 3 and Tier 4 in a single conditional, with no per-tier flag. The relevant lines are at scheduler.ts:58-63. If the scheduler was running, both tiers were being enqueued together. There was no configuration surface that let an operator disable Tier 4 while leaving Tier 3 alone. "Just turn Tier 4 off" was a code change, not a config toggle. Any mitigation that involved keeping Tier 3 running while pausing Tier 4 required editing the scheduler, opening a PR, getting a review, and merging. That is not a fire drill someone runs at 4pm on a Friday when a release is imminent. It is a full engineering task.

The second is that the conditional had no test. The cron scheduler had no test file. We want to be precise about that rather than dramatic, because it is the kind of claim a reader can check: a file called scheduler.test.ts does exist elsewhere in the tree, under experiments/, with four tests covering a timeout helper. It has nothing to do with the cron scheduler. The line that decided whether Tier 3 and Tier 4 ran on a schedule, in production, with the flag settings that determined whether rows were test rows or real rows, had no test asserting that it did what it said it did. If the enqueue behavior had silently changed, no test would have caught it. If the tier-4 branch had been extended to a new tier, no test would have caught it.

Neither of these is a scandal on its own. Lots of small helpers do not have their own test file. Lots of scheduler conditionals bundle related jobs. In isolation, either detail is fine. In this specific configuration, with a tier that provisions unflagged Righthands hidden behind a permanently-crashing lookup, they are the two things that made the fix arm the hazard instead of pointing at it.

The afternoon

Once we understood what merging the release would do, the repair was quick. Under an hour, same afternoon.

Liam Harris shipped PR #622: a per-tier gate called TIER4SCHEDULEENABLED, defaulting to OFF, with a test file created from nothing to cover it. The gate sits inside the scheduler conditional and short-circuits the Tier 4 enqueue when the flag is off. Tier 3 is not affected. The default value preserves exactly the production behavior we have today: Tier 4 does not run on a schedule, because the flag is off, and turning it on requires a deliberate action by someone who has read what the flag does.

Default-off matters, and it is worth stating plainly rather than treating it as an implementation detail. A default-on flag would have shipped the same policy change as e990ca68 alone, just with an escape hatch bolted onto it. A default-off flag ships no policy change at all: the observable behavior of the production system on merge is identical to the behavior the day before. If we later decide we want Tier 4 running on a schedule, we do that as its own decision, with its own review, in its own change. The flag exists to make that decision visible and reversible, not to enable it by default.

Avery cherry-picked PR #622 onto the release branch, which is the move that closed the window rather than merely documenting it. 3.18.0 merged with no exposure at all: the fix in e990ca68 shipped, the underlying bug is repaired, and the tier that the fix would have re-armed stays off until somebody deliberately turns it on. The gate reaching the branch before the release did is the whole of the mitigation. In the nine days since, Tier 4 has not run on a schedule once, which is default-off doing exactly what it is for.

Verifying the gate without trusting the test names

The part of this we do not want to soften is the verification. It would have been easy, and it would have looked responsible, to run the new test file, see 27 green ticks, and call it done. That is what a passing test file is supposed to mean.

We did not do that. Avery verified the gate by mutation, in both directions.

The file is app/services/testing/core/scheduler.test.ts: 27 tests, 0 failing against the shipped gate. Worth stating what that 27 is a denominator of, because the numbers below are only worth anything if it is the whole of something. PR #622 created that file from nothing, so 27 is not a slice of a larger scheduler test file. It is the entire test coverage of that module. It is not the whole testing-service suite, and nothing in this section is a claim about the suite.

Then the mutations. Hard-code the gate to false, or delete the Tier 4 enqueue entirely: 18 pass, 9 fail. Force the gate always-on: 12 pass, 15 fail. The two failing sets overlap in exactly one test, so between them the two mutations move the verdict of 23 of the 27. The 4 that neither one kills are the ones deliberately written to be flag-insensitive, checking behavior that should hold whatever the gate is set to.

We are quoting the arithmetic rather than summarising it because the arithmetic is the evidence. 9 plus 15 minus 1 is 23, 27 minus 23 is 4, and those 4 have a reason. A gate test file where the numbers did not reconcile would mean one of two things: that some of its tests do not actually care about the gate they were written for, or that we do not know which ones do.

We name this because the alternative is a failure mode we have written about before, in The Tests That Passed in Name Only: a test file that passes because its assertions happen to be true, rather than because it is exercising the behavior it claims to. A gate test file with 27 passing tests, none of which change their verdict when you flip the gate, would be worse than no test file at all. It would be a false receipt.

Mutation testing is not exotic. In this case it was mechanical: change the value of the flag, or delete the enqueue, and re-run the file. The information it produced is the only information that matters for a gate of this shape: does flipping the flag change what the code does, and do the tests notice.

The lesson we want to keep

The spine of this post is one question, and we would like it to survive on its own without the details around it.

Before you ship a fix, ask what the broken thing was preventing.

A permanently failing job is indistinguishable from a disabled one until the day someone repairs it. From every observable angle (logs, dashboards, on-call, the mental model of the team) a job that has failed 247 times in a row is not running. It might as well not exist. It contributes nothing to the system's behavior. It is a line in a scheduler that never has any effect.

And be precise about which job you mean, because we nearly were not. Tier 4 had passed twice, in April, both times because a person clicked it. The permanently failing thing was the schedule, not the tier. Had we gone looking for "has this ever worked" rather than "has this ever worked unattended," we would have found two green runs from April and concluded the provisioning path was fine, which is the opposite of the conclusion the run rows support. A job that passes when a human triggers it and errors every time a cron does is not a working job. It is a job whose only remaining user is the cron.

Then someone fixes the crash, and everything downstream of the crash starts running. Sometimes that is the entire point of the fix, and everyone is happy. Sometimes the crash was doing work no one had written down: it was gating a policy that nobody had a ticket for, it was standing in for a config that nobody had built, it was the reason a class of production write never happened. When the crash gets repaired, that work stops getting done, and nothing else steps in to do it.

The failing thing is load-bearing more often than we would like. Any crash that has been in place long enough to become invisible has probably absorbed at least one responsibility that nobody wrote down. Repairing it releases that responsibility back into the system, and the release lands on whoever is downstream of the fix, whether they knew the responsibility was theirs or not.

The check is cheap. When you find a permanently failing job, before you ship the fix, ask: what happens the first time this succeeds unattended. Not the first time it exits zero, and not the last time somebody ran it by hand. The first time it does the thing it was supposed to do, on its own schedule, with nobody watching. Trace the code path forward from the failure site as though the failure were not there. If the answer is "it provisions unflagged Righthands in production, four times a day," you have not found a bug. You have found a switch that was flipped off by accident and is about to be flipped on by accident, and the fix is not the repair, it is the switch.

The honest caveat

We would like to say we caught this because our process caught it. Our process did not catch it.

We did not design the gate. Nobody looked at ENG-670 when it was filed and said "this fix arms a hazard, we need a scheduled-run flag first." The commit sat in the tree as an ancestor of the release branch, it would have gone out with the release, and the first scheduled Tier 4 run afterwards would have provisioned a real Righthand into production. We noticed we needed the gate hours before that release was due to go, not days. The order was luck as much as process. If Avery had picked up the release cut a week later, or if the fix had landed a sprint earlier, the writing of this post would have been a different exercise, and we would not be describing it as a near-miss.

We are telling the story anyway, because the transferable part is not the sequence of decisions we made on the afternoon of the merge. The transferable part is the question we would like to remember to ask next time, before we need luck to catch us: what was the broken thing preventing, and what will run the first time this fix works.