Skip to content

Module-reload window is caller-visible: pre-forward retryable codes aren't absorbed by the shared restart-burst timer #255

Description

@iceteaSA

Small one, and it falls directly out of v0.52.0's transport-resilience work — the shared retry timer covers the phase after a restart but not the reload window itself.

Field observation

Restarting the aft module under v0.52.0, an operator with an in-flight bash call saw, in order:

module_id 'aft' is reloading                                            ×3 attempts
supervised but not available (state=running, enabled=true, live=false)  ×1
<live>

Each surfaced to the caller as a failure. The last shape is the daemon having spawned the process but not yet received HELLO.

Why the new timer doesn't apply

subc-transport.test.ts:470 ("unknown_channel retries share a 100ms restart-burst delay") pins the coalescing behaviour precisely:

expect(client.routeOpens).toHaveLength(2);
expect(clock.scheduledWorkCount).toBe(1);   // one shared floor timer

That keys on unknown_channel — a stale route with the daemon already back up. module_reloading and the not-yet-live supervision state both occur before a route can be opened, so they never reach that path. The burst test is correct and passing; this is a window outside its scope, not a regression in it.

Searching packages/aft-bridge/src, module_reloading appears only in __tests__/error-contract.test.ts — nothing in the request path acts on it. So every attempt during the reload is surfaced verbatim.

The asymmetry that makes this cheap to fix

Your own retry classification already separates these two cases, and #203 encoded it:

condition disposition safe to retry?
module_reloading (per-corr, before forward) provably not executed yes
GOODBYE mid-request, no terminal frame outcome-unknown no — double-execution hazard

error-contract.test.ts:206 states it outright: "module_reloading is proven-not-forwarded and retryable; it must not be [treated as outcome-unknown]." So the safe half of the distinction is already established and tested — it just isn't acted on in the transport.

Absorbing the pre-forward codes behind the existing shared floor timer would make a deliberate module restart invisible to callers, using machinery that already exists rather than new retry logic. The GOODBYE path must keep its current behaviour untouched — surfacing outcome-unknown to the caller is the correct and safety-critical answer there, and nothing here should soften it.

One design question I don't have the standing to answer: whether the supervision state state=running, enabled=true, live=false carries the same pre-forward guarantee as module_reloading. It looks like it should — the process exists but has not completed HELLO, so nothing can have been forwarded — but that's a daemon-side invariant rather than something I can establish from the plugin, and it should not be assumed into the retryable set without your confirmation.

Severity

Cosmetic per incident, mildly annoying in aggregate: on our fleet the module gets restarted several times a week, and each restart currently costs whichever sessions are mid-call a handful of visible failures on top of the one in-flight call that legitimately gets GOODBYE'd.

Credit for the observation goes to a sibling operator seat that drives the restarts — I had read "transport resilience" in the v0.52.0 notes and assumed it covered the whole restart window, and would not have gone looking without their report that the inside view was louder than expected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions