feat: add per task prop selection - #109

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/cli/form.go`:
- Around line 35-42: The current branch flips
state.replaceConfiguredTaskRunRules and clears state.executionTaskRuntimeRules
whenever inputs.defineTaskRuntime is false, which causes existing
workspace-configured rules or explicit --task-runtime flags to be lost if the
user declines the extra form; change the logic in the start-flow handlers (the
block using state.kind == commandKindStart, the collectStartTaskRuntimeForm
call, and the other similar spots) so that you only mutate
state.replaceConfiguredTaskRunRules or clear state.executionTaskRuntimeRules
when the user explicitly chooses to define/replace per-task rules (i.e., when
inputs.defineTaskRuntime is true and/or when collectStartTaskRuntimeForm returns
a positive confirmation), and do not change or clear those fields when the user
declines the form; apply the same fix to the other occurrences mentioned (the
blocks around state.taskRuntimeRules() usages).

In `@internal/cli/root_test.go`:
- Around line 657-673: Wrap the existing
TestAddCommonFlagsUseResilientRetryDefaults body in a t.Run subtest with a
"Should ..." name (e.g., t.Run("Should use resilient retry defaults", func(t
*testing.T) { ... })), move the t.Parallel() call into that subtest, and keep
all existing assertions and lookups (state := newCommandState, cmd :=
newTestCommand, checks for state.maxRetries and flag.DefValue) unchanged inside
the subtest so the test complies with the t.Run("Should...") pattern.

In `@internal/cli/state.go`:
- Around line 73-74: The defaultMaxRetries constant currently set to 2 causes
non-idempotent jobs to be retried by default; change defaultMaxRetries to 0 (no
retries) and ensure retry behavior is opt-in via the existing CLI retry flag or
configuration (update any code paths referencing defaultMaxRetries such as where
retries are parsed/applied and any help text or tests); specifically update the
constant defaultMaxRetries and adjust related logic that merges defaults into
actual retry settings so retries only occur when the user explicitly enables
them (also update any unit tests or docs that assume a default of 2).

In `@internal/cli/workspace_config_test.go`:
- Around line 229-279: The test
TestApplyWorkspaceDefaultsKeepsConfiguredTaskRuntimeRulesAndBuildConfigAppendsCLIOverrides
is not hermetic because applyWorkspaceDefaults reads the global config from
HOME; make the test isolated by setting HOME to a temp directory before calling
state.applyWorkspaceDefaults (or use t.Setenv("HOME", tmpDir) if available) and
restore/cleanup after, or alternatively remove t.Parallel() to avoid concurrent
interference; ensure you perform this HOME isolation before calling
writeCLIWorkspaceConfig/chdirCLITest and before state.applyWorkspaceDefaults so
configuredTaskRuntimeRules only reflects the test fixtures.

In `@internal/core/run/executor/execution.go`:
- Around line 220-222: finalizeExecution() should not use the caller's
cancelable ctx when waiting for observer hooks because that causes shutdown
steps to be skipped on cancellation; change the call to
model.WaitForObserverHooks to use a bounded shutdown context that ignores the
original ctx (e.g., create a new context via
context.WithTimeout(context.Background(), <reasonableDuration>) and defer
cancel()) and pass that to model.WaitForObserverHooks along with
internalCfg.RuntimeManager so the rest of finalization (run.pre_shutdown,
terminal events, task meta refresh, sound notification) still runs even if the
run ctx is canceled.

In `@internal/core/run/executor/runner.go`:
- Around line 205-208: jobRuntimeChanged currently trims runtimes so a hook can
make whitespace-only changes (e.g., "codex" -> "codex ") and bypass the guard
before applyHookModelJob writes the raw value; update jobRuntimeChanged to treat
whitespace-only edits as a runtime mutation by comparing both the trimmed values
and the raw values (or explicitly checking if rawBefore != rawAfter even when
trimmedBefore == trimmedAfter) and return true for any difference, then keep
applyHookModelJob as-is; make the same change for the other occurrence
referenced (lines 272-275) so whitespace-only changes cannot slip through.

In `@internal/core/run/internal/acpshared/command_io_test.go`:
- Around line 186-194: The test TestCreateACPClientUsesPerJobRuntimeWhenPresent
must not call t.Parallel() because SwapNewAgentClientForTest mutates
package-global constructor state; remove the t.Parallel() invocation from that
test (or convert the test into a non-parallel setup section that calls
SwapNewAgentClientForTest and then runs parallel subtests with t.Run +
t.Parallel() if needed) so that the global factory swap performed by
SwapNewAgentClientForTest is not observed by other parallel tests.

In `@internal/core/workspace/config_test.go`:
- Around line 361-394: The tests TestLoadConfigParsesStartTaskRuntimeRules (and
the similar one at 434-452) call LoadConfig which merges global HOME config, so
isolate the test from a developer's ~/.compozy/config.toml by invoking
isolateWorkspaceConfigHome(t) (or stubbing osUserHomeDir) before calling
LoadConfig, and remove t.Parallel() from the test since
isolateWorkspaceConfigHome requires non-parallel execution; update both test
functions to call isolateWorkspaceConfigHome(t) at the start (or set a temporary
HOME via osUserHomeDir stub) and then call LoadConfig as before.

In `@pkg/compozy/events/event_test.go`:
- Around line 141-142: Rename the failing subtest names that use plain strings
to the required t.Run("Should...") pattern: update the subtest names currently
written as "job queued" and "job started" (and the other case at lines ~159-160)
to descriptive "Should..." names (e.g., "Should queue job" and "Should start
job") wherever t.Run is invoked in the event tests that construct
kinds.JobQueuedPayload / kinds.JobStartedPayload so they comply with the
enforced naming rule.

In `@sdk/extension/compat_test.go`:
- Around line 37-38: Rename the test case name fields that feed into t.Run to
follow the mandated "Should..." pattern: update the case with public:
extension.TaskRuntime{} (currently name "TaskRuntime") to a "Should..."
description (e.g., "Should retain TaskRuntime compatibility") and update the
case with public: extension.TaskRuntimeTask{} (currently name "TaskRuntimeTask")
to a "Should..." description (e.g., "Should retain TaskRuntimeTask
compatibility"); these name values are the tc.name passed into t.Run(tc.name,
...) so ensure the name fields in the test cases slice are changed accordingly.

---

Nitpick comments:
In `@internal/cli/task_runtime_flag.go`:
- Around line 141-144: The helper function stringPointer unnecessarily uses an
intermediate variable; simplify it by returning the address of the incoming
string parameter directly (i.e., modify the stringPointer function to take value
string and return &value) so you remove the cloned variable while preserving
semantics and avoiding extra allocation.

In `@internal/cli/task_runtime_form.go`:
- Around line 364-388: The function formatBaseTaskRuntime contains redundant
trimming and reassignment for label: after setting label :=
strings.TrimSpace(state.ide) the code re-trims and uses a temporary display
variable; remove the extra checks and trims (the second TrimSpace and the
display variable) and simply use the already-trimmed label when deciding whether
to append it to parts, leaving the rest (modelName, reasoning, parts assembly)
unchanged; update references to label, state.ide, and ensure modelName and
reasoning behavior is preserved.

In `@internal/core/model/task_runtime.go`:
- Around line 160-166: The function cloneOptionalString currently trims
whitespace while cloning which is surprising given its name; rename the function
to cloneAndTrimOptionalString (and update every call site) to make the behavior
explicit, or alternatively keep the name but add a short comment above
cloneOptionalString stating that it trims whitespace as part of cloning;
whichever you choose, ensure the function signature and behavior remain
unchanged and update all references to the new name if you rename it.