fix: correctly set the Mercure hub for the main worker request

[dunglas]

No description provided.

[Copilot]

Pull request overview

This PR fixes an issue where the Mercure hub was not correctly set for worker initialization requests. The solution refactors how request options (like Mercure hub and logger) are propagated from Caddy configuration to worker dummy contexts.

Key Changes:

• Introduced WithWorkerRequestOptions to allow passing request options to worker initialization
• Refactored worker struct to store requestOptions instead of just env, enabling flexible configuration
• Moved Mercure hub assignment to occur earlier in the Caddy module provisioning phase

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
worker.go	Replaced env field with requestOptions in worker struct; appends document root and prepared env to options during initialization
threadworker.go	Simplified worker script setup by passing all request options from worker to newDummyContext
options.go	Added WithWorkerRequestOptions function to support passing request options to worker initialization
context.go	Updated newDummyContext to use globalCtx for proper context propagation
caddy/workerconfig.go	Added unexported requestOptions field to store per-worker request configuration
caddy/module.go	Moved Mercure hub assignment earlier in Provision; sets worker requestOptions with logger and Mercure hub; includes minor code style improvements
caddy/app.go	Updated worker initialization to conditionally pass requestOptions (contains redundant logic that should be simplified)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The conditional check for w.requestOptions == nil is redundant because both branches append identical options. The code can be simplified by removing the if-else and always calling WithWorkerRequestOptions(w.requestOptions...) since it handles nil/empty slices correctly.

Suggested change

	if w.requestOptions == nil {
	workerOpts = append(workerOpts,
	frankenphp.WithWorkerEnv(w.Env),
	frankenphp.WithWorkerWatchMode(w.Watch),
	frankenphp.WithWorkerMaxFailures(w.MaxConsecutiveFailures),
	frankenphp.WithWorkerMaxThreads(w.MaxThreads),
	)
	} else {
	workerOpts = append(
	workerOpts,
	frankenphp.WithWorkerEnv(w.Env),
	frankenphp.WithWorkerWatchMode(w.Watch),
	frankenphp.WithWorkerMaxFailures(w.MaxConsecutiveFailures),
	frankenphp.WithWorkerMaxThreads(w.MaxThreads),
	frankenphp.WithWorkerRequestOptions(w.requestOptions...),
	)
	}
	workerOpts = append(
	workerOpts,
	frankenphp.WithWorkerEnv(w.Env),
	frankenphp.WithWorkerWatchMode(w.Watch),
	frankenphp.WithWorkerMaxFailures(w.MaxConsecutiveFailures),
	frankenphp.WithWorkerMaxThreads(w.MaxThreads),
	frankenphp.WithWorkerRequestOptions(w.requestOptions...),
	)

[henderkes]

quick phone based questions:

[henderkes]

where's this 7 coming from?

[dunglas]

That's the number of hardcoded parameters.

[henderkes]

Suggested change

	opts := make([]frankenphp.Option, 0, len(options)+len(f.Workers)+7)
	// we have 7 parameters
	opts := make([]frankenphp.Option, 0, len(options)+len(f.Workers)+7)

[dunglas]

It's obvious, isn't it?

[henderkes]

or this +4?

[henderkes]

Suggested change

	workerOpts := make([]frankenphp.WorkerOption, 0, len(w.requestOptions)+4)
	// we have 4 parameters
	workerOpts := make([]frankenphp.WorkerOption, 0, len(w.requestOptions)+4)