feat(schedule): add Temporal schedule for periodic scanning

[revied]

Summary

• Adds Temporal Schedule API support so the OrchestratorWorkflow runs automatically on a configurable cron (default: daily at 06:00 UTC per the design doc)
• Schedule manager uses a create-or-update pattern to handle server restarts gracefully (if schedule already exists, updates it if cron changed)
• Disabled by default via SCHEDULE_ENABLED=false for backwards compatibility
• Wires SCHEDULE_* env vars through docker-compose.yaml so the feature can be toggled for local end-to-end testing

Configuration

Env Var	Default	Purpose
SCHEDULE_ENABLED	false	Opt-in to scheduled scanning
SCHEDULE_CRON	0 6 * * *	Cron expression (daily at 06:00 UTC)
SCHEDULE_ID	version-guard-scan	Stable schedule ID for idempotent restarts
SCHEDULE_JITTER	5m	Random jitter to prevent thundering herd

Changes

New:

• pkg/schedule/schedule.go — Schedule manager with create-or-update logic
• pkg/schedule/schedule_test.go — 8 unit tests covering all paths

Modified:

• cmd/server/main.go — Config fields, schedule wiring with graceful failure
• pkg/workflow/orchestrator/workflow.go — ScanID fallback from workflow execution ID for scheduled runs
• docker-compose.yaml — SCHEDULE_* env var passthrough

Bug fixed during review

Initial implementation mutated only CronExpressions on the update path. Temporal parses CronExpressions into structured Calendars server-side on create, so subsequent describes return the cron inside Calendars with CronExpressions empty. Mutating only CronExpressions left the stale calendar in place, causing the schedule to fire on both the old and new crons after every restart with a changed cron.

Fixed by replacing the entire Spec on update. Regression test TestEnsureSchedule_Update_ReplacesStaleCalendars simulates the real Temporal describe response and asserts Calendars is cleared.

Test plan

• go build ./... compiles
• go test ./pkg/schedule/... — 8/8 tests pass
• go test ./... — full suite passes
• golangci-lint run ./pkg/schedule/... clean
• Manual end-to-end against temporal server start-dev:
  • Start 1: SCHEDULE_ENABLED=true SCHEDULE_CRON="0 6 * * *" → temporal schedule describe shows one calendar entry for 06:00
  • Start 2: restart with SCHEDULE_CRON="*/30 * * * *" → server logs Schedule updated, temporal schedule describe shows a single calendar entry matching */30 (stale entry correctly cleared)
  • temporal workflow list --query 'WorkflowType = "OrchestratorWorkflow"' confirms scheduled runs fire on cron boundaries

Reproducing locally

# Terminal 1: start Temporal dev server
temporal server start-dev --namespace version-guard-dev

# Terminal 2: run the server with scheduling enabled
SCHEDULE_ENABLED=true \
SCHEDULE_CRON="*/5 * * * *" \
SCHEDULE_ID="version-guard-local" \
SCHEDULE_JITTER=1m \
TEMPORAL_ENDPOINT=localhost:7233 \
TEMPORAL_NAMESPACE=version-guard-dev \
WIZ_CLIENT_ID_SECRET=<your-id> \
WIZ_CLIENT_SECRET_SECRET=<your-secret> \
WIZ_REPORT_IDS='{"aurora-postgresql":"<report-id>"}' \
CONFIG_PATH=config/resources.yaml \
go run ./cmd/server

# Terminal 3: verify via CLI
temporal schedule list --namespace version-guard-dev
temporal schedule describe --schedule-id version-guard-local --namespace version-guard-dev
temporal workflow list --namespace version-guard-dev \
  --query 'WorkflowType = "OrchestratorWorkflow"'

Note: docker-compose up works too once SCHEDULE_ENABLED is exported, but the bundled Temporal image currently needs a fix on main before compose boots a healthy server — out of scope for this PR.

🤖 Generated with Claude Code

[bakayolo]

🤖 Automated Expert Review

Summary

Adds Temporal Schedule API support for periodic orchestrator scanning with a solid create-or-update pattern. Well-structured code with good test coverage, but has a potential nil pointer panic in the update path and missing startup timeout.

---

Findings

🚫 BLOCKER: Nil pointer dereference in DoUpdate closure

File: pkg/schedule/schedule.go:97-103
The DoUpdate closure directly accesses input.Description.Schedule.Spec.CronExpressions without nil-checking Spec. While the outer code (line 86) handles nil existingSpec, the Temporal SDK provides a fresh ScheduleUpdateInput inside DoUpdate — if that input has a nil Spec, this will panic and crash the server on startup.
Suggestion: Add a nil guard inside the closure:

if input.Description.Schedule.Spec == nil {
    input.Description.Schedule.Spec = &client.ScheduleSpec{}
}

⚠️ WARNING: Schedule update only propagates Spec, not Action

File: pkg/schedule/schedule.go:97-103
When updating an existing schedule, only the CronExpressions and Jitter are overwritten. If someone changes the TaskQueue via config, the existing schedule will continue firing with the old task queue/timeout settings.
Suggestion: Also update the action in DoUpdate:

if action, ok := input.Description.Schedule.Action.(*client.ScheduleWorkflowAction); ok {
    action.TaskQueue = cfg.TaskQueue
}

⚠️ WARNING: No timeout on EnsureSchedule during startup

File: cmd/server/main.go:352-358
EnsureSchedule makes multiple RPCs to Temporal (Create, Describe, Update). Using the base context.Background() means this can block startup indefinitely if Temporal is unresponsive.
Suggestion: Wrap with a timeout:

schedCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
schedErr := scheduleMgr.EnsureSchedule(schedCtx, ...)

ℹ️ INFO: Missing test for nil Spec edge case

File: pkg/schedule/schedule_test.go
All test mocks populate Spec in describeOut. Adding a TestEnsureSchedule_AlreadyExists_NilSpec case would have caught the nil pointer issue above.

ℹ️ INFO: Nice ScanID fallback pattern

File: pkg/workflow/orchestrator/workflow.go:56-60
Using workflow.GetInfo(ctx).WorkflowExecution.ID as fallback ScanID for scheduled runs is deterministic and replay-safe — great choice. 👍

---

Checklist

Check	Status	Notes
🔒 Security	✅	No secrets, credentials, or injection vectors
🏗️ Reliability	⚠️	Nil pointer risk in DoUpdate; missing startup timeout
🚀 Staging before Prod	N/A	Feature flag gated (SCHEDULE_ENABLED=false default)
📦 Atlantis Plan	N/A	Not an infrastructure PR
🧪 Tests	⚠️	6 tests cover happy paths well; missing nil Spec edge case
⚙️ CI Checks	🚫	"Test" check is failing (1m49s)
💰 Cost/Blast Radius	✅	Disabled by default, low risk

---

Verdict: REQUEST CHANGES

The BLOCKER (nil pointer panic in DoUpdate) must be fixed before merge. The CI test failure also needs investigation. The two WARNINGs (partial update propagation, missing startup timeout) should ideally be addressed but are not strictly blocking.

---

_{🤖 Review by Amp agent · not a substitute for human review}

[codecov]

Codecov Report

❌ Patch coverage is 88.46154% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/schedule/schedule.go	88.46%	4 Missing and 2 partials ⚠️

Files with missing lines	Coverage Δ
pkg/workflow/orchestrator/workflow.go	0.00% <ø> (ø)
pkg/schedule/schedule.go	88.46% <88.46%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[revied]

🤖 Automated Expert Review — @bakayolo

All findings from this review have been addressed:

Finding	Status	Commit
BLOCKER: Nil pointer dereference in DoUpdate closure	✅ Fixed — added nil guard for Spec	9002fee
WARNING: Schedule update only propagates Spec, not Action	✅ Fixed — DoUpdate now propagates TaskQueue via Action type assertion	9002fee
WARNING: No timeout on EnsureSchedule during startup	✅ Fixed — 10s timeout context; startup continues with warning on failure	9002fee
INFO: Missing test for nil Spec edge case	✅ Added TestEnsureSchedule_AlreadyExists_NilSpec	9002fee

Follow-up lint fixes in b3d8304 and 011e66a (renamed stuttering types, fixed struct field alignment).

Branch has been rebased onto latest main with clean linear history (no merge commits). All tests and lints pass.