Honor Remove on a supervisor that has not started

[youdie006]

Add has an explicit not-yet-running branch (v4/supervisor.go:274) that registers the service directly instead of going through the control channel, and its doc comment says:

If the supervisor has not been started yet, the service will be started when the supervisor is. [...] The returned ServiceID may be passed to the Remove method of the Supervisor to terminate the service.

Remove (v4/supervisor.go:704) and RemoveAndWait (:726) have no matching branch. Both go straight to sendControl, which returns ErrSupervisorNotStarted and mutates nothing.

s := suture.NewSimple("Top")
id := s.Add(svc)
err := s.Remove(id)        // "supervisor not started yet"
s.ServeBackground(ctx)     // svc starts anyway
len(s.Services())          // 1

step1 after Add            : len(services)=1 restartQueue=[0]
step2 Remove returned      : supervisor not started yet
step3 after ServeBackground: REMOVED SERVICE WAS STARTED
step3 Services()           : 1 entries

Doing the same thing after Serve works correctly, so the gap is specifically the pre-start state that Add goes out of its way to support.

The fix

Delete the service directly while the supervisor is still notRunning, mirroring Add's branch. Remove already maps the sibling ErrSupervisorNotRunning to nil at :709, so "a removal that needed no work returns nil" is the shape this file already uses.

I checked the rest of the state that Add's pre-start branch writes. It also appends to restartQueue, and I deliberately left that alone: both readers (:345 and :425) do namedService, present := s.services[id] and skip when absent, then reset the queue, so a stale id there is inert. Deleting from services is sufficient.

Behaviour change

Remove / RemoveAndWait on a never-started supervisor now return nil instead of ErrSupervisorNotStarted. No existing test row changes -- ErrSupervisorNotStarted is asserted nowhere in the suite (its only references are the two sendControl sites), and the full suite is green unmodified.

Verification

go test -count=1 ./... in v4 -- ok, with and without the change. go vet ./... clean. gofmt -l . lists doc.go errors_after_13.go errors_before_13.go service.go both before and after; supervisor.go and suture_test.go are not among them, so the count is unchanged at 4.

One table test added to suture_test.go, covering both entry points. I mutation-checked each call site on its own rather than reverting the pair together:

mutation	result
revert only the Remove call site	fails TestRemoveBeforeServe/Remove alone
revert only the RemoveAndWait call site	fails TestRemoveBeforeServe/RemoveAndWait alone
drop the state != notRunning guard from the helper	fails TestRemoveAndWait, then the suite times out

The third is the one that matters: without the guard the helper would delete from services while the supervisor is running and racing the control loop, so the guard is load-bearing rather than decoration.

A note on how I got there, since it nearly cost the test its value: my first version of the table did not re-bind the loop variable. go.mod declares go 1.9, so the subtests closed over the same tt and both ran the second case -- and the "revert only the Remove site" mutation passed. go vet caught it (loop variable tt captured by func literal). With tt := tt added, each site fails independently as shown above.

Two things I want to flag rather than leave you to find

• go test -race fails on unmodified master too, in TestShim, TestFailures, TestRemoveAndWait and others, at supervisor.go:566/574. That is pre-existing and unrelated to these lines -- I mention it only so the failures are not attributed to this change. There is no CI in the repo and pre-commit runs plain go test.
• During one mutation run the full suite reported a failure I could not reproduce; three clean re-runs of the identical tree were ok, and the change itself is green on every run. I have no explanation for it, so I would rather say so than let it pass unmentioned.

What I did not change

restartQueue (stale ids are inert, as above), cancellations (never populated before runService), Services() which still returns nil before start, and the documented Add-after-terminate no-op.

---

Disclosure: AI-assisted. I found and prepared this with an AI assistant, and I ran and verified everything above myself.