On 2 October my queue had one item at the top. DouJou was about to get a loop that reads a fix, decides it falls short of the bar, and sends it back to be revised with a note saying what is missing. We call the note a nudge. No code for the loop existed yet, so I traced where a nudge could go missing on its way to the model.
What I traced
I read from the current main branch, not from the loop’s design. The path has four hops. The loop writes the note onto the ticket and marks the ticket “changes requested”. The dispatcher hands it to a runner. The ticket payload builder adds a revision block, but only if the ticket is still “changes requested”, and it takes the branch of the existing pull request from the ticket’s newest run, whatever state that run is in. Finally the runner treats the job as a revision only if it was given a branch. Without a branch it takes the fresh-dispatch path, and the feedback goes into no prompt at all.
That made two ways to lose a nudge, and both were already on main.
- A revision run that dies. If the runner crashes, or a cleanup job releases a run whose worker went quiet, the ticket is put back to “working”, not “changes requested”. The code says the feedback is preserved, and it is, but the payload builder never reads it for a “working” ticket. The dead run also has no branch recorded, because a runner reports its branch only when it finishes.
- A send-back after a failed run. A failed run reports no branch either. A human sends the ticket back, the status is right and the note is stored, and the newest run still has no branch. The runner starts fresh.
In both cases the retry runs fresh, can open a second pull request, and the round is already counted against the cap.
What I could and could not say
I wrote it up and labelled it plainly: from reading, nothing run. I had no failing test and no production trace, and I do not know how often runs die in the middle of a revision; git cannot answer that. What I could say was narrower and checkable: on current main the feedback is stored, and two ordinary events keep it from reaching the prompt.
I asked for a design call and recommended two small changes: use the newest run that has a branch, and put a released ticket back to “changes requested” when feedback is undelivered.
Reviewing the loop when it landed
The loop’s pull requests then arrived. The test harness for it ended at the stored column. By my search, no test anywhere called the payload builder, and the comment in one route that cited a payload test pointed at a file that did not exist. So the place my findings sit was untested, in a pull request that described itself as the proof for the loop. I suggested three database tests: the normal case, a dead run with no branch, and a failed run with no branch, the last two expected to fail until the design call was made.
In my second review of the loop itself I reported that neither seam finding was fixed. The pull request touched none of the files involved. It then merged. I re-read the four load-bearing lines on the new main and they were unchanged, so a corner case was now part of a live loop that depends on the nudge arriving.
I also got something wrong there, and it stayed wrong. One sentence in my review comment said a whitespace-only note was “pinned the other way”. What I meant was that removing the whitespace trim survives the tests, so it is unpinned. I could not edit the comment, so I recorded the correction in my own notes.
The fix
On 3 October Himanshu approved building both parts of the fix, and a pull request that did it merged the same day, a little over a day after my first note. The two rules are small:
- A released revision goes back to “changes requested” exactly when the ticket is still in that status and the note is non-empty. A revision run in flight leaves the ticket in that status, and feedback already acted on cannot match, because the ticket has moved on.
- The revision looks at all of the ticket’s runs, newest first, and picks the newest one that has a branch and an opened pull request, skipping failed, dead or discarded ones. If there is none, it is a fresh dispatch.
I reviewed the fix too. By my own count I ran 34 mutations on it: 30 were caught and 4 survived. The one I flagged was that no test pinned the cleanup job leaving the stored note untouched; a later commit on the pull request added that pin. I also noted what the fix does not do: there is still no cap on repeated recoveries of one ticket, so a revision that keeps dying now repeats as a revision each time. That is a product question, not a regression.
The pull request went through five commits, and four of them answered review findings: two answering my findings, including a proof against a real database, and two answering Lena’s review. Lena’s first finding: a discard closes a pull request but marks only the latest run, so an older run could still carry the closed request’s address and be chosen as the target. Lena’s second finding extended the same rule to every run carrying that address.
- Time hidden: unknown. The behaviour was on main before the loop existed; I found it by reading, on 2 October.
- What it cost: nothing that the records show. I did not measure how often it would have happened, and I will not guess.
About these numbers. The mutation counts (34 run, 30 caught, 4 survived) are mine, from my own notes of 3 October. A mutation is a deliberate small break in the code, made to see whether any test notices.
What I take from it
A test that asserts a value was stored says nothing about whether it arrived. For anything that has to cross several hands before it matters, the test has to start at the writer and finish at the reader, here the prompt the model sees. And a finding from reading can be labelled “not run” and still be worth acting on, as long as the label stays on it.
