[#217] Defer component supplementary check construction

[bguidolim]

Summary

ExternalPackAdapter.convertComponent() eagerly called ProjectDetector.findProjectRoot() for each component with inline doctorChecks, baking the result into ComponentDefinition.supplementaryChecks at construction time. This caused global-scope doctor runs to use CWD's project root instead of nil, and made allDoctorChecks(projectRoot:) inconsistent — derived checks used the passed scope, supplementary checks ignored it.

Closes #217

Changes

• Changed ComponentDefinition.supplementaryChecks from [any DoctorCheck] to a factory closure @Sendable (URL?, Environment) -> [any DoctorCheck] that defers construction to execution time
• Updated allDoctorChecks(projectRoot:environment:) to call the factory with scope-correct parameters
• Replaced eager check construction in ExternalPackAdapter.convertComponent() with a closure, eliminating the ProjectDetector.findProjectRoot() call
• Updated ComponentExecutor.isAlreadyInstalled() to call the factory with (nil, Environment())
• Added #if DEBUG convenience init on ComponentDefinition accepting [any DoctorCheck] for test ergonomics
• Added --ifdef noindent to .swiftformat and reformatted Environment.swift

Test plan

• swift test passes locally (782 tests in 92 suites)
• swiftformat --lint . and swiftlint pass without violations

Checklist for engine changes

• .shellCommand components have supplementaryDoctorChecks defined (deriveDoctorCheck() returns nil for shell actions)

[bguidolim]

Code Review Summary

Branch: bruno/217-defer-supplementary-check-construction → main | Ticket: #217

Clean, well-scoped change. The core problem — supplementaryChecks being eagerly evaluated at component construction time with a potentially incorrect projectRoot — is correctly addressed by deferring construction to a factory closure. The approach mirrors the existing pack-level pattern (TechPack.supplementaryDoctorChecks(projectRoot:) from PR #216).

No Critical Issues

Important Issues (2)

• ExternalPackAdapter.swift:161 — compactMap used on ExternalDoctorCheckFactory.makeCheck() which returns non-optional Fixed in latest push (compactMap → map)

• Test coverage gap — No test exercises ExternalPackAdapter.convertComponent() with non-nil doctorChecks. The entire deferred closure construction path (the primary bug fix site) is untested. Every ExternalPackAdapterTests test passes doctorChecks: nil. Similarly, no test verifies that allDoctorChecks(projectRoot:environment:) forwards parameters to the supplementary checks closure — existing tests use the #if DEBUG init which wraps a static array in { _, _ in checks }, masking the forwarding contract.

Suggestions (3)

• Typealias — Consider typealias SupplementaryCheckFactory = @Sendable (URL?, Environment) -> [any DoctorCheck] for readability and a grep-able name for the concept. [Component.swift:40]

• Documenting comment — Add a comment to isAlreadyInstalled explaining why nil is passed for projectRoot (project-scoped supplementary checks will .skip, safe because convergent actions are idempotent). [ComponentExecutor.swift:381]

• Regression test — No regression test for the specific global-scope doctor bug. DoctorRunnerIntegrationTests.globalOnlyChecksGlobalScope uses a MockTechPack with no component-level doctor checks, so it doesn't exercise the supplementary check closure path.

Strengths

• Correct architectural fix — derived and supplementary checks now see consistent projectRoot/environment parameters
• Clean closure captures — checksDefinitions, runner, path bound to local let before the closure, avoiding self capture
• #if DEBUG convenience init now uses self.init(...) delegation (latest push), reducing duplication
• @Sendable annotation is forward-looking and enforces concurrency safety at the compiler level
• MisconfiguredDoctorCheck pattern preserved — invalid check definitions still surface as warnings

Pre-existing Issues (outside PR scope)

• try? usage in ExternalDoctorCheck.swift (lines 170, 201, 340-341) violates the project's "never use try?" rule — ExternalFileContainsCheck, ExternalFileNotContainsCheck, and ExternalSettingsKeyEqualsCheck silently conflate I/O errors with missing files