Skip to content

feat: improve TypedCommand parity and executor restart#6

Merged
simonovic86 merged 2 commits intomainfrom
claude/xenodochial-northcutt
Mar 10, 2026
Merged

feat: improve TypedCommand parity and executor restart#6
simonovic86 merged 2 commits intomainfrom
claude/xenodochial-northcutt

Conversation

@simonovic86
Copy link
Owner

Changes

TypedCommand Enhancements

  • Add TypedExpiredFunc and WithExpired option for handling deadline expirations
  • Add WithTags option to set default tags on typed commands
  • Make HandleTyped return *Executor for method chaining
  • Implement Expirable interface on TypedCommand

Option Validation

  • Clamp negative retry values to 0 in WithRetries, Retries, and WithDefaultRetries
  • Ignore non-positive durations in Period, Deadline, WithPeriod, WithDeadline, and WithDefaultTimeout
  • Add comprehensive test coverage in new options_test.go

Executor Restart Support

  • Enable executor restart by reinitializing runtime state on Start() after Stop()
  • Use Overwrite for barrier command registration to allow re-registration
  • Add TestExecutor_RestartAfterStop test case

Documentation

  • Add quick start example to package documentation
  • Enhance Instance.Get and Instance.Set documentation with JSON serialization notes
  • Add inline comments about option behavior and validation

Testing

  • Add 45 new test cases for restart functionality and option validation
  • Add 147 new test cases for typed command features

simonovic86 and others added 2 commits March 10, 2026 21:17
…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>
@simonovic86 simonovic86 merged commit 3331185 into main Mar 10, 2026
3 checks passed
@simonovic86 simonovic86 deleted the claude/xenodochial-northcutt branch March 10, 2026 20:21
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +132 to +136
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{}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

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