feat: improve TypedCommand parity and executor restart#6
Conversation
…dation, executor restart - Add WithExpired[T] and WithTags[T] options to TypedCommand for feature parity with FuncCommand, including Expirable interface implementation - Change HandleTyped to return *Executor for method chaining consistency - Add input validation: clamp negative retries to 0, ignore non-positive durations in Period/Deadline/Timeout options - Make executor reusable after Stop() by reinitializing runtime state on restart (context, queue, timers, WaitGroup) - Use registry.Overwrite for barrier command to support re-registration - Document int→float64 JSON coercion on Instance.Get/Set godocs - Add Quick Start section to package documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mark as done: executor restart, TypedCommand Expirable/Tags support, HandleTyped return type, option bounds checks, Spec builders, terminology drift, JSON coercion docs, and graceful shutdown tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 026c5b582c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| e.ctx, e.cancel = context.WithCancel(context.Background()) | ||
| e.stopping.Store(false) | ||
| e.queue = make(chan *Instance, e.queueSize) | ||
| e.delayedTimers = make(map[string]*time.Timer) | ||
| e.wg = sync.WaitGroup{} |
There was a problem hiding this comment.
Prevent restart after timed-out shutdown
When Stop() times out (e.g., a handler ignores context), Start() still enters the restart branch and resets e.ctx, e.queue, and especially e.wg. Any old worker goroutine that exits later will call Done() on the reset waitgroup, which can trigger panic: sync: negative WaitGroup counter, and those old workers may also start consuming from the new queue/context. This makes restart unsafe precisely in the timeout path that operators are likely to hit during incident recovery.
Useful? React with 👍 / 👎.
Changes
TypedCommand Enhancements
TypedExpiredFuncandWithExpiredoption for handling deadline expirationsWithTagsoption to set default tags on typed commandsHandleTypedreturn*Executorfor method chainingExpirableinterface onTypedCommandOption Validation
WithRetries,Retries, andWithDefaultRetriesPeriod,Deadline,WithPeriod,WithDeadline, andWithDefaultTimeoutoptions_test.goExecutor Restart Support
Start()afterStop()Overwritefor barrier command registration to allow re-registrationTestExecutor_RestartAfterStoptest caseDocumentation
Instance.GetandInstance.Setdocumentation with JSON serialization notesTesting