Making the lifecycle-desync failure class testable (a non-cooperative agent double)

[point-source]

I've filed a number of lifecycle/desync bugs over the past few months, and after the most recent one (#3358) I went looking for what they have in common rather than filing an eleventh. I think there's a single missing test primitive behind most of them, and it's small enough to be worth proposing on its own.

Writing this as a discussion rather than an issue because the second half is a direction, not a defect, and I'd rather ask than assert. I'm reading entirely from the outside — no visibility into the roadmap, what's already planned, or constraints I can't see from a checkout — so please discount accordingly.

First, the thing that made me stop and re-think

My starting assumption was that this area was under-tested. That's wrong, and the numbers aren't close:

	Files	Lines
internal/orchestrator	80	47,589
internal/orchestrator tests	183	82,077
internal/integration	17	5,046

A 1.7:1 test-to-code ratio on the package in question, go test -race across all packages in CI, an architecture-lint workflow, and a dedicated startup_order_test.go pinning four ordering invariants. The comments explaining why a given ordering matters — publishReadiness, ticker-vs-time.After in the sweep warner, the CREATED-session skip in resetAgentContext — are more thorough than most codebases I read.

So "add more tests" is both wrong and not the argument. Effort isn't the missing ingredient, which is what made me want to work out what actually is.

The shape these bugs share

Nine issues I've filed, plus the startup variant in #3358:

Issue	The counterparty that didn't cooperate
#1585	write-back never delivered; rows stuck at starting, pid/port 0
#1597	same, still present two releases later
#1631	state write silently no-ops; board and task disagree
#1890	follow-ups queue behind detached child work
#1947	activity record and tool-ownership map never released on teardown
#2823	turn-completion lost; session permanently bricked
#3149	cancel raced a live turn
#3210	pending completion drained without clearing the flag
#3358	agent ignored the cancel; ResetSession never returned
#3358 (startup)	agent.boot_ready published with no subscriber yet

Every one is the same sentence with different nouns: a state transition that happens only if a specific in-memory signal reaches a specific listener at a specific moment — and when it doesn't, nothing reconciles it, so something waits forever or until a long timeout.

Why the existing tests can't catch this class

Three reasons, none of them about coverage:

The test doubles are unconditionally cooperative. SimulatedAgentManagerClient can express launch delay, launch failure, and fail-after-N. But StopAgent unconditionally closes the stop channel, sets status, and publishes AgentStopped:

close(execution.stopCh)
execution.status = v1.AgentStatusStopped
s.publishAgentEvent(events.AgentStopped, execution)
return nil

There's no way to construct an agent that ignores a cancel, returns late, or never returns — which is the precondition for most of the table above.

-race is blind here by construction. It detects unsynchronized access to shared memory. A dropped event, a lost update through SQLite, a subscriber that isn't registered yet — none of those involve concurrent memory access. -race passing across every package is genuinely reassuring about a different class of bug.

The ordering invariants stop at the boundary. startup_order_test.go pins four, so the category is clearly already recognised and invested in. What isn't pinned is intra-Service.Start ordering, which is exactly where the #3358 startup variant lives.

The concrete ask

#3358 ends with a regression test I proposed:

start a turn against an agent that ignores cancel, fire an on_children_completed transition into a step with reset_agent_context, and assert (a) the reset does not block…

That test can't be written today. Not because it's hard, but because the primitive doesn't exist. I think the primitive is ~50–100 lines on the existing simulated client:

func (s *SimulatedAgentManagerClient) SetCancelBehavior(b CancelBehavior) // honor | ignore | delay(d) | error
func (s *SimulatedAgentManagerClient) SetResetBehavior(b ResetBehavior)   // honor | block | error
func (s *SimulatedAgentManagerClient) SetDropEvents(subjects ...string)

That one addition turns #3149, #3210, #3358 and #2823 from "reproduced by hand on a live board" into ordinary table tests. I'm happy to write it and the four regression tests if that'd be useful — it's the part of this I'm most confident is worth doing regardless of what you think of the rest.

Four more that are each an afternoon or less:

1. One test in the existing startup_order_test.go: the watcher is subscribed before any sweep that can block on watcher-driven state.
2. lifecycleSweepStuckWarningInterval's warner is a one-shot time.NewTimer, so a stalled sweep logs once at 60s and then goes silent. A ticker turns a dead-looking log into a progress signal.
3. no live agent execution for context reset logs at Debug. At the default level, "the reset ran and short-circuited" and "the reset hung" are indistinguishable — that line cost me a confidently wrong first diagnosis of my own bug.
4. Bound waitForSessionReady when it's reached from startup recovery. AgentLaunchTimeout is a fair budget for a user-initiated launch and an expensive one for a sweep that holds readiness.

The longer-term direction, if useful — expand

This part is speculative and I hold it loosely.

The thing that struck me is that the pattern that would fix this class already exists in the codebase — it's just applied to a different layer.

The task lifecycle tokens (manual_move_lifecycle_pending → work → manual_move_lifecycle_completed → clear pending) are a durable, crash-safe, idempotent, resumable intent record. They survive restarts. That design is why my most recent incident self-healed instead of stranding the task, and it's genuinely good.

Session state has no equivalent. It's ~46 direct write sites across 15 files, 8 states, no central transition validator (the only canTransition in the backend is in internal/plugins), coordinated by an in-memory bus with no persistence, replay, or ack. Truth is effectively split across four places that must agree: the in-memory execution, task_sessions.state, executors_running, and the provider's own ACP session — the last of which can only be inferred, which is precisely what #3358 gets wrong.

Holding that together currently takes ~34 lock/guard identifiers on Service and 33 reconcile*/recover*/sweep*/reap* entrypoints. My read is that the reconciler count is the system compensating at boot for transitions it couldn't guarantee at runtime — and the #3358 startup variant is one reconciler deadlocking against the absence of the subsystem that repairs another.

Two incremental steps that don't require a rewrite:

• A session-state transition validator. One func transition(from, to State) error with a table of legal edges; route the 46 sites through it one at a time. Doesn't fix ownership, but makes illegal transitions loud instead of silent, and gives a migration path.
• Extend the durable-intent pattern to session transitions. Under it, a dropped event becomes a delay rather than a permanent wedge — the difference between these bugs being annoying and being unrecoverable.

I'm deliberately not proposing single-writer-per-session or an actor model. It's probably the endgame, it's not a reasonable ask, and it would bring its own crop of bugs.

What I'm actually asking

Mostly: does this framing match how you see it from the inside? You have context I don't, and it's entirely possible the counterparty-cooperation gap is known and already queued behind something more important.

If the framing does hold, I'd like to contribute the non-cooperative agent double and the regression tests for the four issues it covers. That seems useful whether or not anything downstream of it happens.

Thanks for building this — the durable-intent design in the task lifecycle is a large part of why these failures have been recoverable rather than destructive, and it's not an accident.