LockedCommand creates Environment twice (once for lock file, once in perform)

[bguidolim]

Severity: LOW

LockedCommand.run() in FileLock.swift creates let env = Environment() solely to derive env.lockFile. Then each command's perform() creates a second Environment() (either directly or via PackCommandContext()).

This double-construction affects all LockedCommand conformers (AddPack, RemovePack, UpdatePack, SyncCommand, DoctorCommand, CleanupCommand, ExportCommand).

The cost is negligible in practice (~12 URL path constructions, static-cached tool resolution), but it's architecturally untidy.

Fix: Change LockedCommand protocol to pass the Environment from run() into perform():

protocol LockedCommand: ParsableCommand {
    var skipLock: Bool { get }
    func perform(env: Environment) throws
}

This would require updating all conformers but eliminates the redundant construction and opens the door for injectable Environment in tests (deferred per decision in decision_testing_command_testability_strategy).

Found during review of #109.

[bguidolim]

Assessment (2026-03-06)

Still valid?

Yes. LockedCommand.run() in FileLock.swift:46 creates Environment() for the lock file path. Then each of the 5 conformers (SyncCommand, DoctorCommand, CleanupCommand, ExportCommand, PackCommand via PackCommandContext) creates a second Environment() in perform().

Intentional deferral

The broader fix (passing env into perform()) was deliberately deferred in decision_testing_command_testability_strategy (PR #204). That decision chose Option B (promote specific private methods to internal) over Option A (injectable Environment) because the structural change across all conformers wasn't justified without the testing infrastructure to benefit from it.

Benefits

• Eliminates redundant construction (negligible perf, cleaner architecture)
• Main value: unblocks injectable Environment for end-to-end perform() testing — the real reason to do this

Recommendation

Keep open, defer. The cost is negligible and the fix only becomes valuable as part of a broader testability effort. This issue is the correct breadcrumb for that future work — no changes needed to the issue itself.