fix(resync): classify terminal refusals instead of retrying them - #440
Merged
schickling merged 1 commit intoSep 4, 2026
Conversation
`emit_resync` collapsed every publication error into one retryable failure, so a refusal the catalog will keep giving was re-armed on the carrier's class window forever — 500 ms for an immediate carrier. Eligibility is resolved under the shared catalog-authoring lock and a refusal follows from the declaration, not from timing, so each attempt re-resolved the whole catalog to reach the same answer and threw it away. On the affected host that was ~2010 attempts per seat and a continuous stream of new shared lock acquisitions. A refusal is now classified by what could admit it later. `resolve_stream` returns a typed refusal, so the classification comes from the reason rather than from matching a message. A recipient that is declared but not running parks its reservation: attempted once, then it captures nothing and schedules nothing. The reservation is retained beyond the subscription, which the next refresh drops, and re-arms with its exact reserved bytes when a refresh carries that recipient again — which happens only while it is running. Dropping it at the refusal would lose a resync the agent should see on resume. An ambiguous recipient, a recipient owned by another host, and a recipient that does not declare the stream are permanently refused: the reservation is dropped after one diagnostic and the carrier baseline advances, so the same transition is not captured again on the next observation. Everything else stays retryable, including an absent declaration — a declaration being replaced by rename is briefly absent, and dropping its reservation would lose a resync nothing was wrong with. This is the secondary hardening for #431. It removes the CPU burn and the unbounded retry; the pass-completion property it used to deny is fixed separately and independently. Refs #431 agent-identity: dev3.compoundingtech.st2.resync-lock.worker agent-persona: worker agent-supervisor: dev3.compoundingtech-lead agent-tool: OMP agent-tool-version: 18.1.2 agent-runtime: OMP 18.1.2 tooling-profile: dotfiles@7534055
schickling-assistant
marked this pull request as ready for review
September 4, 2026 10:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
emit_resynccollapsed every publication error into one retryable failure:The caller treats
falseas uniformly retryable, so it re-arms the reservation on the carrier'sclass window — 500 ms for an immediate carrier — forever.
Several refusals are not transient. Eligibility is resolved under the shared catalog-authoring lock
precisely so that a suspension edit and a publication cannot interleave, which means a refusal
follows from the declaration rather than from timing. Retrying one is pure cost:
emit_admittedtakes the shared lock and re-resolves the entire catalog through
discover_strictbefore itreaches the refusal, then discards the work. On the affected host in #431 that was ~2010 attempts
for each of three seats, against 4 for a seat whose task had already exited.
Goal
A refused publication is classified by what could admit it later, and only a refusal something
could still admit is retried.
Decisions
resolve_streamreturns a typed refusal, so the classification comes from the reason ratherthan from matching a message. Every existing diagnostic keeps its exact text — several tests
assert on those strings, and they are what an operator greps for.
admit the publication later. A not-running recipient can resume; ambiguity, a foreign owning
host, and an undeclared stream cannot be resolved from the publisher's side.
a resync the agent should see on resume. It is attempted once, then captures nothing and
schedules nothing.
subscription entirely, so a reservation held only inside the subscription would not survive to
the resume. It is retained per
(recipient, binding)alongside the existing occurrence-sequencefloor, which is the same worker-lifetime, bounded-cardinality retention.
carries is running by construction, so that is exactly the moment a parked reservation may be
attempted again. The worker needs no new view of desired state.
"re-capture the same transition on the next observation and refuse again", which is the loop with
extra steps.
absent, and a catalog mid-edit is a transient state; dropping a reservation there would lose a
resync nothing was wrong with. This is a deliberate exclusion from the permanent set, and the
mutation matrix below covers it.
Verification
Fail-before — the assertions describe state this change introduces (a parked subscription and
its retained reservation), so fail-before is shown by mutating the classification away rather than
by checking out the parent commit.
C1_suspended_refusal_untypedrestores exactly the previousbehaviour — the refusal reaches
emit_resyncunclassified and takes the retry path — and threetests fail. The behavioural assertion inside them, "attempted exactly once", is the pre-change
symptom directly: before this change the reservation is re-captured and handed off on every window.
Mutation — one shipped clause removed at a time, no survivors, no unrun mutants:
a_not_running_recipient_parks…+ 2a_permanently_refused…,refusals_are_classified…a_not_running_recipient_parks…a_permanently_refused…a_not_running_recipient_parks…a_not_running_recipient_parks…a_not_running_recipient_parks…a_parked_reservation_re_arms…refusals_are_classified…+ 5Two of these took a second pass and it is worth recording why, because both first attempts were
mutants that could not have failed. One left the park body attached to the mutated match arm, so it
still parked; the other did not compile, which the harness reported as UNRAN rather than as a
survivor. The third, "mutation wakeup does not skip a parked subscription", was a genuine survivor:
the test only observed the publication count, which the flush-path skip already keeps at one. It
now asserts the property the clause actually carries — a parked subscription schedules nothing —
checked separately at each wakeup source.
Suite —
cargo test -p st2 --lib: 684 passed, 0 failed. The exact Nix-check test selection(
--lib --bins --test discovery --test codex_hooks --test hooks --test run --test driver_expansion,--test-threads=1) is green, every targetok.--test resync,--test event_e2e,--test resync_notify_chain,--test reconcilegreen, including the existingtests that assert the refusal message text.
Complexity
One two-variant classification, one boolean on a subscription, and one retained-reservation map
that mirrors the existing occurrence-sequence retention next to it. No new dependency, no new
thread, no change to the publication path's locking or ordering.
Concerns
intended, and it is strictly better than the previous behaviour, where the same subscription was
attempted twice a second and its reservation was dropped by the next refresh anyway. It does mean
the reservation's delivery is bounded by the resume, not by a deadline.
observed, like the occurrence-sequence floor beside them. A supervisor restart discards them,
which is the same horizon
RESYNC-T03already sets for carrier state.no agent foundstays retryable on purpose. If a declaration is genuinely gone rather thanmid-rename, its reservation is retried until the refresh drops the subscription. That is the
pre-existing behaviour and this PR does not change it.
that property; whether the pass stall of Terminal resync refusal is retried at 500 ms forever, re-resolving the catalog under the shared authoring lock and starving all writers #431 is fully explained remains open there.
Friction & bottlenecks
canonical megarepo worktree whose branch identity is pinned, so the local branch switching
gh stackexpects is refused. The result on GitHub is an ordinary stacked PR.touched here.
Follow-ups
References
Refs #431. Stacked on #437.
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile