perf: fix ~28% BenchmarkYAMLGeneration regression by eliminating reflection hot path

[Copilot]

BenchmarkYAMLGeneration regressed ~28% (1.62ms → 2.08ms) due to a hot path executing on every compilation.

Root cause

hasAnySafeOutputEnabled / hasNonBuiltinSafeOutputsEnabled — both used reflect.Value.FieldByName in a loop across 41+ SafeOutputsConfig pointer fields (~54µs/iter). Because applyDefaultCreateIssue always produces a non-nil SafeOutputsConfig even for workflows without safe-outputs:, the reflection loop ran unconditionally on every compile.

Changes

• safe_outputs_state.go: Replace reflection loops in both hasAnySafeOutputEnabled (43 fields) and hasNonBuiltinSafeOutputsEnabled (40 non-builtin fields) with direct != nil pointer comparisons — zero allocation, zero reflection overhead. Also adds CommentMemory which was previously missing from both checks: workflows using only tools: comment-memory: were incorrectly treated as having no safe outputs, preventing safe-output prompt/config generation and causing an unwanted auto-injected create-issue. Remove now-dead variables (safeOutputReflectionLog, builtinSafeOutputFields, nonBuiltinSafeOutputFieldNames).

• safe_outputs_state_test.go (new): TestSafeOutputStateFieldCoverage iterates safeOutputFieldMapping via reflection and asserts each field triggers both check functions — fails immediately if a new safe output type is added to the mapping without updating the direct-check lists. TestSafeOutputStateCommentMemoryCoverage explicitly guards CommentMemory, which lives outside safeOutputFieldMapping.

• safe_jobs_needs_validation.go: Update stale comment that referred to the removed reflection-based implementation.

Before: ~1,020,330 ns/op
After:  ~862,000 ns/op  (~15% improvement on benchmark host)

[Copilot]

Pull request overview

Optimizes two compile-time hot paths by replacing reflective safe-output state checks with direct field checks and by caching the working directory used for path rendering.

Changes:

• Replaced reflection in hasAnySafeOutputEnabled and hasNonBuiltinSafeOutputsEnabled with explicit nil checks.
• Added a regression test to keep the manual safe-output field lists aligned with the mapped handler fields.
• Cached os.Getwd() in ToRelativePath using sync.Once to avoid repeated syscalls.

Show a summary per file

File	Description
pkg/workflow/safe_outputs_state.go	Reworks safe-output enabled-state detection to use direct field checks instead of reflection.
pkg/workflow/safe_outputs_state_test.go	Adds a reflection-driven regression test for the manual safe-output check lists.
pkg/console/render.go	Caches the working directory for relative-path rendering.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

pkg/workflow/safe_outputs_state.go:153

• CommentMemory is also missing from the non-builtin list. Because applyDefaultCreateIssue() runs after comment-memory is attached to SafeOutputs (compiler_orchestrator_workflow.go:322-329, 409-411), a workflow whose only meaningful output is comment-memory is still seen as having no non-builtin outputs and gets an auto-injected create-issue it did not ask for.

	// Direct nil checks for non-builtin pointer fields (39 fields = 42 total minus 3 builtins:
	// NoOp, MissingData, MissingTool).
	return safeOutputs.CreateIssues != nil ||
		safeOutputs.CreateAgentSessions != nil ||
		safeOutputs.CreateDiscussions != nil ||

• Files reviewed: 3/3 changed files
• Comments generated: 3

[pelikhan]

@copilot review all changes

[Copilot]

@copilot review all changes

Three issues were found and fixed in commit 4196f0d:

1. CommentMemory missing from both direct-check functions — a workflow using only tools.comment-memory was incorrectly treated as having no safe outputs, which would prevent safe-output prompt/config generation and wrongly inject an auto create-issue. Added CommentMemory to both hasAnySafeOutputEnabled and hasNonBuiltinSafeOutputsEnabled.

2. ToRelativePath sync.Once cache — reverted. The codebase calls os.Chdir() in production paths (trial_helpers.go, trial_repository.go, pr_command.go), so caching the working directory on first access would yield stale paths after any directory change.

3. Test gap for CommentMemory — added TestSafeOutputStateCommentMemoryCoverage to explicitly assert both check functions return true when only CommentMemory is set, since it lives outside safeOutputFieldMapping and is not covered by the map-driven TestSafeOutputStateFieldCoverage test.