Skip to content

feat(vessel): add Handle.OnTerminate lifecycle hook#63

Merged
lIang70 merged 1 commit into
mainfrom
feat/vessel-on-terminate
May 7, 2026
Merged

feat(vessel): add Handle.OnTerminate lifecycle hook#63
lIang70 merged 1 commit into
mainfrom
feat/vessel-on-terminate

Conversation

@lIang70

@lIang70 lIang70 commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a synchronous completion-hook API on `vessel.Handle` so multiple consumers can observe run termination with deterministic ordering — no more racy `<-h.Done()` watcher goroutines per consumer.

Why

PR #62 surfaced a real ordering gap on `/v1/vessels/{id}/call`: `/call` returned when `h.Wait` resolved, but `runRegistry`'s background goroutine watching `h.Done()` had no ordering guarantee against `Wait`. Result: `/metrics` and `/v1/runs` could miss a just-completed run, breaking the scrape contract.

The architectural fix is to give `Handle` a proper lifecycle-event hook API instead of patching one consumer at a time.

Contract

  • Hooks fire synchronously, in registration order, BEFORE `Wait` returns and BEFORE `Done` is closed.
  • Any consumer of `Wait`/`Done` is guaranteed to see a state where every hook has executed.
  • Hooks must be fast and non-blocking — they delay every `Wait` caller.
  • Hooks may panic; isolated per-hook via `recover` so one buggy hook can't block siblings.
  • Late registration is safe: registering after termination invokes `fn` synchronously with the cached terminal state.

Scope

  • `vessel/handle.go`: `Handle.OnTerminate(fn)` + `hookMu`/`onTerminate`/`terminated` fields; `deliver` runs hooks under panic isolation before `close(done)`.
  • `vessel/handle_test.go`: 8 tests covering ordering, fan-out, panic isolation, late-register fast path, nil-fn no-op, concurrent registration race.

Backwards compatible — pure addition.

Follow-up

PR #62 will pick up `vessel v0.1.0-rc.2` (this PR + tag) and migrate `runRegistry` off its own goroutine, then revert the test-side polling hack added as a stopgap.

Test plan

  • `go test -race -count=10 -run TestHandle ./vessel/` green
  • `go test -race -count=1 ./vessel/...` green
  • CI green

Made with Cursor

Introduce a synchronous completion-hook API on vessel.Handle so
multiple consumers can observe run termination with consistent
ordering, without each spawning its own <-h.Done() watcher
goroutine (the racy pattern that motivated this).

Contract:

  - Hooks fire synchronously, in registration order, BEFORE Wait
    returns and BEFORE Done is closed. Any consumer of Wait/Done
    is guaranteed to see a state where every hook has executed.
  - Hooks must be fast and non-blocking (they delay every Wait
    caller by their wall-clock cost).
  - Hooks may panic; the panic is recovered per-hook so one buggy
    hook cannot block siblings or leave the run hanging.
  - Late registration is safe: registering after termination
    invokes fn synchronously with the cached terminal state.

This is purely additive — Handle stays backwards compatible. The
intended initial consumer is cmd/vesseld's runRegistry, which
will migrate off its own background goroutine in a follow-up PR
and gain a deterministic "/call returns => registry recorded"
contract for /metrics and /v1/runs scrapes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lIang70
lIang70 merged commit a4e10a4 into main May 7, 2026
8 checks passed
@lIang70
lIang70 deleted the feat/vessel-on-terminate branch May 7, 2026 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant