refactor(controller): read env configuration in constructors, not package init

[Joibel]

Part 1 of 3 in a stack that retires util/logging's init logger (see #16705 for the endgame; follows up on #16693, which fixed the init logger's process-wide signal handler swallowing SIGTERM).

Motivation

The init logger exists to buffer log messages emitted before the main logger is configured — almost all of them package-level var/init() env-config reads. Moving those reads into constructors (where a real, correctly-formatted logger is already in context) removes the need for pre-main logging entirely, makes the config values testable with t.Setenv, and turns config echoes into ordinary constructor log lines in the operator's chosen format and level.

Modifications

Pure moves of where each env read happens — every variable keeps its exact name and default:

• WorkflowController gains constructor-read fields for INDEX_WORKFLOW_SEMAPHORE_KEYS, CACHE_GC_PERIOD, SEMAPHORE_NOTIFY_DELAY, CACHE_GC_AFTER_NOT_HIT_DURATION, HEALTHZ_AGE, MAX_OPERATION_TIME, and DEFAULT_REQUEUE_TIME; the package-level vars and init() blocks are deleted. The newController test helper mirrors each default explicitly.
• WorkflowSemaphoreKeysIndexFunc becomes an explicit factory parameter (enabled bool) and the package-level indexers var becomes newIndexers(semaphoreKeysEnabled bool); its tests now cover the disabled branch without env mutation.
• fixedItemIntervalRateLimiter carries the requeue time as a field; the exported, otherwise-unused GetRequeueTime() is removed (breaking only for downstream importers — nothing in-repo references it).
• Cron: CRON_SYNC_PERIOD moves into NewCronController. The init()-time RFC822 timezone-parse sanity check remains, but no longer involves the init logger: it now simply panics on failure (stderr, no special handling).

Deliberate behavior change: an unparseable duration value now fails at constructor time with a real message (previously it panicked at package-init via the init logger's unimplemented WithPanic).

Verification

go build ./..., golangci-lint (0 issues), and go test ./workflow/... -count=1 all pass (one pre-existing, unrelated environment failure in workflow/executor/osspecific reproduces identically on main). Each field's default is asserted equal between constructor and test helper by inspection; the semaphore-index factory gained a new disabled-branch test.

Documentation

Not needed: every env var keeps its documented name, default, and semantics; docs/environment-variables.md remains accurate.

AI

Claude Code planned and implemented this change under human direction, with per-task and whole-branch review passes; the author reviewed and takes responsibility for the result.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SqVAUnuPJZ7sQZCy44bBSE

Summary by CodeRabbit

• Improvements
  • Configuration for workflow scheduling, health checks, operation timeouts, cache cleanup, and semaphore handling is now applied consistently per controller instance.
  • Workflow queue rate limiting and cron synchronization use their configured intervals more reliably.
  • Cache cleanup and health diagnostics now honor controller-specific settings.
  • Semaphore indexing can be enabled or disabled through configuration.
  • Improved isolation and predictability when running multiple controller instances with different settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bc41c8cf-d00b-413b-9942-3a7faef023e0

📥 Commits

Reviewing files that changed from the base of the PR and between dc4452e and 4ad6e80.

📒 Files selected for processing (10)

• workflow/controller/agent.go
• workflow/controller/cache_gc.go
• workflow/controller/controller.go
• workflow/controller/controller_test.go
• workflow/controller/healthz.go
• workflow/controller/indexes/workflow_index.go
• workflow/controller/indexes/workflow_index_test.go
• workflow/controller/operator.go
• workflow/controller/rate_limiters.go
• workflow/cron/controller.go

---

📝 Walkthrough

Walkthrough

Workflow and cron controllers now store environment-derived timing and feature settings per instance. Workflow semaphore indexing accepts explicit configuration. Runtime operations use controller fields instead of package-level values.

Changes

Workflow controller configuration

Layer / File(s)	Summary
Controller settings and wiring workflow/controller/controller.go, workflow/controller/controller_test.go	WorkflowController stores configuration for timing, cache garbage collection, health checks, semaphore behavior, queueing, and indexing. Construction and tests use these fields.
Semaphore indexing contract and tests workflow/controller/indexes/workflow_index.go, workflow/controller/indexes/workflow_index_test.go	WorkflowSemaphoreKeysIndexFunc receives an explicit enabled flag. Tests cover enabled, completed, and disabled indexing.
Controller runtime consumers workflow/controller/agent.go, workflow/controller/cache_gc.go, workflow/controller/healthz.go, workflow/controller/operator.go, workflow/controller/rate_limiters.go	Agent patch rates, cache cleanup, health checks, operation deadlines, and rate limiting use instance-scoped settings.

Cron controller configuration

Layer / File(s)	Summary
Cron sync-period configuration workflow/cron/controller.go	Controller stores the resolved sync period. Construction validates time zones and loads CRON_SYNC_PERIOD; synchronization uses the stored value.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 4ad6e

This refactor relocates configuration reads into constructors without any supplied evidence of a current-head correctness, availability, or deployment risk. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: isubasinghe

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main refactor to move environment configuration reads from package initialization into constructors.
Description check	✅ Passed	The description covers motivation, modifications, verification, documentation, AI usage, behavior changes, and known test limitations.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/controller-env-config

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}