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

Inline comments:
In `@internal/activation/activation.go`:
- Around line 546-568: The goroutine can leak an SSH connection because
reconnectCh is created buffered (reconnectCh := make(chan *system.SSHSystem, 1))
so the send of s2 from s.(*system.SSHSystem).Clone() never blocks and s2 can
remain in the channel if the receiver times out; change reconnectCh to an
unbuffered channel (make(chan *system.SSHSystem)) so the send will block and the
goroutine's timeout branch (select with time.After(opts.AckTimeout)) will run
and call s2.Close(), or alternatively ensure the send path closes s2 when the
receiver times out by adding a coordinating select/cancel; update uses of
reconnectCh, Clone(), s2 and s2.Close() accordingly.

In `@internal/logger/replay.go`:
- Around line 173-175: ReplayLogger.SetLogLevel is currently a no-op so calls
like logger.SetLogLevel(logger.LogLevelDebug) (e.g., in PreRun) have no effect
when a ReplayLogger is active; implement SetLogLevel on type ReplayLogger to
actually honor the requested level by storing/applying it to the logger instance
(or forwarding it to its underlying sink/parent logger) so verbose/debug flags
take effect — update the method on ReplayLogger (SetLogLevel) to set an internal
field or call the underlying sink's SetLogLevel/Configure method (matching your
existing LogLevel type and any sink methods) and ensure any checks that use the
level (e.g., ReplayLogger.log or level checks) read that stored/applied level.

In `@internal/settings/settings.go`:
- Around line 520-534: The code currently only populates cfg.setDeprecatedFields
inside parse( k *koanf.Koanf ), so dynamic updates via (*Settings).SetValue
don't mark deprecated keys as explicitly set; add a single helper on Settings
(e.g., markDeprecatedFieldSet(key string)) that records keys into
setDeprecatedFields and use it from the parse loop (where you currently set
cfg.setDeprecatedFields[key]) and also call it from (*Settings).SetValue
immediately after a successful assignment; update tests to include a regression
case where SetValue("auto_rollback", "false") is applied and Validate() respects
the explicit false.

In `@nix/tests/magic-rollback.test.nix`:
- Around line 148-149: The test references
nodes.target.system.build.networkConfig via targetNetworkJSON but the target
node only defines targetConfig under system.build, causing evaluation failure;
either add a networkConfig attribute to the target node's system.build (e.g.,
populate nodes.target.system.build.networkConfig with the intended network data)
or remove/stop using targetNetworkJSON so the test no longer reads
nodes.target.system.build.networkConfig—update the test to consistently use
either targetConfig or a newly added networkConfig on the target node.

---

Outside diff comments:
In `@internal/generation/generation.go`:
- Around line 180-184: The code uses os.Readlink directly for
currentGenerationDirname (assigning to currentGenerationLink) which breaks the
filesystem abstraction; replace os.Readlink with the Filesystem abstraction
method by calling s.FS().ReadLink(currentGenerationDirname), propagate/handle
the returned value and error exactly as done elsewhere in this function, and
remove the direct os dependency so remote or mocked filesystems work
consistently with the rest of the code.

In `@internal/systemd/time.go`:
- Around line 16-94: DurationFromTimeSpan can panic or accept invalid values:
check and handle strconv.ParseInt errors, ensure bounds after skipping spaces (i
< len(span)) before reading unit and return a clear error if unit is missing or
empty (fix the loop that skips spaces so it doesn't advance past end), and guard
against numeric overflow when converting num*durationUnit (return an error on
overflow); update the parsing in DurationFromTimeSpan to validate ParseInt's
err, verify unitStart < spanLen and unit contains letters, and detect overflow
when computing totalDuration; add regression tests for inputs like "1   " and an
extremely large numeric component.

---

Nitpick comments:
In `@cmd/apply/apply.go`:
- Around line 702-749: The ReadLink failure for prevLink (when calling
targetHost.FS().ReadLink(activeProfileLink)) should log that because prevLink
couldn't be determined a rollback will be skipped; update the warning emitted
where prevLink is read (referencing activeProfileLink, prevLink, and ReadLink)
to include a note that rollback of the system profile will not be attempted when
the previous link is unknown, and keep newGenerationCreated logic (which
compares prevLink and afterLink) unchanged.

In `@internal/activation/activation.go`:
- Around line 595-598: Change the fmt.Errorf call that reports errors read from
the activationComplete channel to use %w so the error is wrapped for unwrapping;
specifically, where activationErr := <-activationComplete and you currently
return fmt.Errorf("activation supervisor exited with error: %v", activationErr),
replace the format verb to return fmt.Errorf("activation supervisor exited with
error: %w", activationErr) so tools like errors.Is()/errors.As() can inspect the
original activationErr.
- Around line 338-357: The triggerDirectory in EnsureActivationDirectoryExists
is intentionally created world-writable (0o777|os.ModeSticky), which is a
security concern to track; open a tracking issue describing the risk and
proposed hardening (e.g., group-based permissions or configurable
restrictiveness), then add a concise TODO comment above the
EnsureActivationDirectoryExists function referencing the new issue number and
summarizing why the directory is world-writable today and what should be changed
in future; optionally, also add a runtime warning log in
EnsureActivationDirectoryExists (mentioning triggerDirectory) that points to the
issue so operators are aware.

In `@internal/activation/supervisor.sh`:
- Around line 74-100: The rollback() function uses prevToplevel (set from
readlink /run/current-system) without validating it, which can produce an
invalid path; update rollback() to check if prevToplevel is non-empty and points
to an existing directory before constructing prevSwitchToConfigurationScript and
invoking it (verify the resolved path exists and is executable); if prevToplevel
is empty or invalid, log a clear error via the same logging mechanism and exit
with a non-zero status instead of attempting to call
"/bin/switch-to-configuration"; keep the existing behavior for
PREVIOUS_SPECIALISATION when prevToplevel is valid.

In `@internal/logger/replay.go`:
- Around line 6-7: ReplayLogger has an unused field ReplayLogger.level that
should be removed to avoid state drift; remove the level field from the struct
definition and delete any code that initializes or assigns to ReplayLogger.level
(e.g., in the NewReplayLogger constructor or any setters) and ensure only
entries ([]logEntry) remains, updating any methods that referenced level to use
entries or external filtering instead; also remove corresponding unused imports
or tests that reference the removed field.