Sync dev to main

[tis24dev]

• Align secondary path validation across config load, CLI install, and TUI
• Align --new-install confirmation flow across CLI and TUI
• Align existing backup.env handling across CLI and TUI
• Fix AGE setup validation and install TUI messaging alignment
• Align Telegram setup flow across CLI and TUI
• Align decrypt secret prompt semantics across CLI and TUI
• Add end-to-end coverage for the production decrypt TUI flow
• Align secondary disable semantics across CLI and TUI
• Align install cron scheduling across CLI and TUI
• Add cron install regression coverage for CLI and TUI
• test(orchestrator): stabilize decrypt TUI end-to-end tests
• fix(install): guard optional bootstrap logging in TUI install flow
• fix(newkey): guard success logging when bootstrap is nil
• fix(decrypt): reject unchanged destination paths in CLI and TUI prompts
• refactor: simplify ticker wait in rollback countdown
• fix: respect configured recipient file in --newkey
• test(tui): complete coverage for new install and telegram setup
• test(tui): add missing wizard coverage
• test(tui): harden Telegram setup TUI tests
• fix(storage): correct UsedSpace and stop deriving it from Bavail
• fix(orchestrator): avoid duplicate file close in bundle and restore paths
• fix(orchestrator): make AGE recipient overwrite flow safe and atomic
• fix(restore): extract regular files atomically
• fix(config): align notification defaults and legacy env compatibility
• storage: bound cloud retry backoff and remove overflow-prone shifts
• Fix binary integrity validation flow
• Fix GFS daily minimum normalization across retention paths
• notify: honor context cancellation in email, relay, and webhook paths
• fixstorage: normalize bundle paths in cloud and secondary store flows
• Fix restore extraction metadata handling on FakeFS
• fix(tui): honor canceled context in age prompts
• test(storage): allow one-block drift in used space assertion
• fix(storage): normalize secondary bundle source path
• storage: make cloud retry backoff context-aware
• fix(orchestrator): respect context cancellation in decrypt TUI prompts
• test: align notification storage fixtures with total space
• fix(orchestrator): validate decrypt UI before backup selection
• test(orchestrator): increase initial decrypt TUI E2E waits
• fix: align secondary path handling with SECONDARY_ENABLED
• fix(config): normalize legacy notification env aliases in overrides
• test: replace manual env cleanup with t.Setenv
• test: finish migrating env-based tests to t.Setenv
• fix(security): close TOCTOU gap in executable integrity check
• Verify each finding against the current code and only fix it if needed. In @internal/tui/wizard/telegram_setup_tui.go around lines 281 - 291, The ESC key handler reads result.Verified without holding the mutex that checkHandler uses; acquire the same mutex that guards result.Verified (i.e., the mutex used in checkHandler/telegramSetupQueueUpdateDraw) before reading result.Verified in the app.SetInputCapture callback, then release it immediately (use a brief lock/unlock or defer unlock) and then call doClose(true/false) based on the protected value; ensure you reference the same mutex variable used by checkHandler to avoid introducing a new synchronization primitive.
• fix(orchestrator): write backup bundles atomically
• fix(install): make new-install reset safe when bootstrap logger is nil
• fix(storage): avoid duplicate cloud uploads when bundle exists
• fix(wizard): guard nil install data with exported sentinel error
• test(orchestrator): track all created bundle temp files
• Harden secondary storage bundle-copy coverage.
• fix(restore): log all temp file cleanup failures
• test(orchestrator): stop timed TUI key injection on cleanup
• test(orchestrator): rely on context timeout in decrypt TUI helper
• test(tui): shorten telegram setup backstop timeout
• refactor(orchestrator): remove redundant form index guard
• test(storage): expand retention normalization coverage
• fix(tui): avoid double slash in preserved entry formatting
• fix(orchestrator): ignore blank TUI new-path input before cleaning
• test(cmd): make newkey stdout capture cleanup-safe
• test(cmd): fail fast on telegram CLI skip-path regressions
• fix(cmd): cap telegram CLI verification retries
• Refactor TUI screen builders and unify shared screen shells
• Harden FakeFS path mapping and unify TUI screen shells
• Harden test sandbox paths and stabilize decrypt TUI e2e waits
• Align decrypt TUI abort startup waits with success flow
• Simplify BootstrapLogger Flush nil handling
• Deduplicate preserved entries fixture in new install tests
• Use fd-based permission fixes in binary integrity checks
• Deduplicate bundle fixture setup in bundle tests
• Deduplicate age wizard runner setup in adapter tests
• Centralize AGE setup type constants in wizard flow
• Clarify cancellation timing in TUI test
• Centralize decrypt TUI E2E mutex in helper
• Fix indentation in age setup wizard
• Escape TUI text for safe display
• Preserve surrounding spaces in decrypt prompt
• Add context-aware RunWithContext to App
• Pass context to age wizard runner
• Escape header text in BuildScreen
• Use context-aware TUI runners
• Propagate context to TUI run functions
• Propagate contexts for identity and file ops
• Escape TUI status messages with tview.Escape
• Escape bracketed text in restore TUI prompts
• Remove redundant SkipConfigWizard assignment
• Propagate context to new-install TUI
• Improve testability and add HA/new-install tests
• Only append slash for existing directories

Summary by CodeRabbit

• New Features

  • Interactive AGE encryption recipient setup during install/newkey flows; CLI/TUI adapters included
  • Telegram pairing CLI with centralized verification and a “Keep existing & continue” new-install option
  • Cron time selection in the install wizard; improved TUI screens and context-aware TUI execution

• Bug Fixes

  • Stronger validation for secondary storage paths (must be absolute local paths when enabled)
  • Better cancellation/timeout handling across install, restore, and notification flows
  • More accurate storage usage reporting (used-space metrics) and safer bundle/restore file operations

• Documentation

  • Updated install, encryption, Telegram, and secondary-storage docs to match new behaviors

[codecov]

Codecov Report

❌ Patch coverage is 72.41848% with 406 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/orchestrator/age_setup_workflow.go	61.93%	45 Missing and 14 partials ⚠️
internal/orchestrator/encryption.go	56.00%	31 Missing and 24 partials ⚠️
internal/orchestrator/restore_tui.go	59.67%	19 Missing and 6 partials ⚠️
internal/orchestrator/age_setup_ui_cli.go	50.00%	22 Missing and 2 partials ⚠️
internal/tui/wizard/age_ui_adapter.go	42.10%	22 Missing ⚠️
internal/orchestrator/restore.go	55.55%	12 Missing and 8 partials ⚠️
internal/identity/identity.go	40.62%	10 Missing and 9 partials ⚠️
...al/orchestrator/workflow_ui_tui_decrypt_prompts.go	86.42%	11 Missing and 8 partials ⚠️
internal/storage/cloud.go	64.81%	16 Missing and 3 partials ⚠️
internal/security/security.go	61.70%	11 Missing and 7 partials ⚠️
... and 18 more

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

internal/environment/detect_additional_test.go (1)

266-278: ⚠️ Potential issue | 🟡 Minor

Add t.Setenv for test isolation consistency.

Unlike TestExtendPath, this test doesn't set a known initial PATH before calling extendPath(). This means:

1. The test depends on whatever PATH exists in the environment
2. PATH modifications from extendPath() persist after the test (no automatic cleanup)

For consistency with the migration pattern and proper test isolation:

Proposed fix

 // TestExtendPathIdempotent tests that extendPath doesn't duplicate paths
 func TestExtendPathIdempotent(t *testing.T) {
+	t.Setenv("PATH", "/usr/local/bin")
+
 	// Call extendPath twice
 	extendPath()
 	pathAfterFirst := os.Getenv("PATH")

🤖 Prompt for AI Agents

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

In `@internal/environment/detect_additional_test.go` around lines 266 - 278,
TestExtendPathIdempotent relies on the real environment PATH and leaves changes
behind; update the test to use t.Setenv to set a known initial PATH before
invoking extendPath so the test is isolated and changes are automatically
reverted, then call extendPath twice and compare os.Getenv("PATH") values as
before; locate TestExtendPathIdempotent and add a t.Setenv("PATH",
"<known-value>") (e.g., an empty or minimal PATH string) at the start to ensure
determinism and cleanup.

internal/tui/wizard/install.go (1)

657-658: ⚠️ Potential issue | 🟠 Major

Reject non-regular config paths before showing this prompt.

This only checks that os.Stat succeeds. A directory/FIFO/socket will still enter the TUI flow here, while the CLI path rejects non-regular files up front in cmd/proxsave/install_existing_config.go, Lines 28-38. That breaks the new CLI/TUI alignment and defers the failure until later install steps.

🤖 Prompt for AI Agents

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

In `@internal/tui/wizard/install.go` around lines 657 - 658, The
CheckExistingConfig function currently only verifies that os.Stat succeeds and
therefore allows non-regular paths (directories, FIFOs, sockets) into the TUI
flow; update CheckExistingConfig to call os.Stat to obtain FileInfo and reject
any path where fi.Mode().IsRegular() is false (return the same
error/ExistingConfigAction used by the CLI flow), so non-regular config paths
are rejected before showing the prompt; reference the CheckExistingConfig
function and ensure the returned error behavior matches the existing CLI check
for consistency.

🧹 Nitpick comments (8)

cmd/proxsave/main.go (1)

1569-1570: The continue statement is redundant.

Since <-ticker.C is the last statement in the loop body, the continue has no effect and can be removed for cleaner code.

♻️ Suggested simplification

-		<-ticker.C
-		continue
+		<-ticker.C

🤖 Prompt for AI Agents

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

In `@cmd/proxsave/main.go` around lines 1569 - 1570, Remove the redundant
`continue` that follows the `<-ticker.C` receive; since `<-ticker.C` is the last
statement in the loop body, the `continue` has no effect—delete the `continue`
line (near the `ticker` receive) so the loop behavior remains unchanged and the
code is cleaner.

internal/tui/screen_test.go (1)

11-25: Consider extending helper to handle additional tview primitives.

The primitiveContainsText helper currently only handles *tview.TextView and *tview.Flex. While this is sufficient for the current tests, you may want to add support for *tview.Box or use a more general approach if future tests need broader coverage.

♻️ Optional: Add fallback for other primitive types

 func primitiveContainsText(p tview.Primitive, want string) bool {
 	switch v := p.(type) {
 	case nil:
 		return false
 	case *tview.TextView:
 		return strings.Contains(v.GetText(false), want)
 	case *tview.Flex:
 		for i := 0; i < v.GetItemCount(); i++ {
 			if primitiveContainsText(v.GetItem(i), want) {
 				return true
 			}
 		}
+	case *tview.Grid:
+		for row := 0; row < v.GetRowCount(); row++ {
+			for col := 0; col < v.GetColumnCount(); col++ {
+				if primitiveContainsText(v.GetItem(row, col), want) {
+					return true
+				}
+			}
+		}
 	}
 	return false
 }

🤖 Prompt for AI Agents

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

In `@internal/tui/screen_test.go` around lines 11 - 25, The helper
primitiveContainsText only inspects *tview.TextView and *tview.Flex, so extend
it to also check other relevant primitives (e.g., *tview.Box, *tview.Form, or
any primitive exposing text) or add a generic fallback: detect and extract
visible text where possible (for example use v.GetText for Box-like types or
iterate contained items for containers), and ensure primitiveContainsText calls
that extraction for types beyond *tview.TextView/*tview.Flex so tests that
render text in other primitives succeed; update the type switch in
primitiveContainsText to include cases for *tview.Box and any other concrete
primitives you expect, plus a default/fallback branch that attempts a safe
string probe.

internal/tui/abort_context_test.go (1)

129-152: These stop-path tests don't prove the app actually kept running.

Lines 137-138 and 150-151 only assert the final error after a delayed Stop(). An implementation that returns nil immediately would still satisfy both tests, so they don't really verify "runs until stopped". Consider synchronizing on a started signal, or asserting that RunWithContext stays blocked until the goroutine calls Stop().

🤖 Prompt for AI Agents

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

In `@internal/tui/abort_context_test.go` around lines 129 - 152, The tests
currently only check the final error and can pass if RunWithContext returns
immediately; update both TestAppRunWithContext_NilContextRunsUntilStopped and
TestAppRunWithContext_ReturnsNilWhenStoppedWithoutCancellation to synchronize on
the app actually starting before calling app.Stop() — e.g., have
newSimulationApp provide a started channel or WaitGroup signal the test can wait
on (or otherwise expose a Started() hook), then launch the goroutine to sleep
and call app.Stop() only after that start signal, and finally assert that
app.RunWithContext(nil) / app.RunWithContext(context.Background()) remains
blocked until Stop is invoked and returns nil.

internal/orchestrator/workflow_ui_cli_test.go (1)

13-40: Make captureCLIStdout failure-safe.

If fn() panics or calls t.Fatal, Lines 35-39 never execute. That leaves os.Stdout redirected and the io.Copy goroutine blocked on the still-open pipe, which can bleed into later tests.

♻️ Suggested change

 func captureCLIStdout(t *testing.T, fn func()) string {
 	t.Helper()

 	oldStdout := os.Stdout
 	r, w, err := os.Pipe()
 	if err != nil {
 		t.Fatalf("os.Pipe: %v", err)
 	}
-	os.Stdout = w
-	t.Cleanup(func() {
-		os.Stdout = oldStdout
-	})

 	var buf bytes.Buffer
 	done := make(chan struct{})
 	go func() {
 		_, _ = io.Copy(&buf, r)
 		close(done)
 	}()
+
+	os.Stdout = w
+	defer func() {
+		os.Stdout = oldStdout
+		_ = w.Close()
+		<-done
+		_ = r.Close()
+	}()

 	fn()

-	_ = w.Close()
-	<-done
-	_ = r.Close()
-
-	os.Stdout = oldStdout
 	return buf.String()
 }

🤖 Prompt for AI Agents

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

In `@internal/orchestrator/workflow_ui_cli_test.go` around lines 13 - 40, The
captureCLIStdout helper currently restores stdout and closes the pipe only after
fn() returns, so if fn() panics or calls t.Fatal the cleanup never runs; fix
captureCLIStdout by adding a defer immediately after creating r,w and starting
the io.Copy goroutine that 1) closes w, 2) waits for the goroutine via <-done,
and 3) closes r (and keep the existing t.Cleanup for restoring os.Stdout);
update the function captureCLIStdout to use that defer (referencing r, w, done)
so stdout is restored and the io.Copy goroutine can always exit even when fn()
fails.

internal/config/validation_secondary_test.go (1)

24-37: Consider adding t.Parallel() to subtests for faster test execution.

The parent tests call t.Parallel(), but the subtests don't. Adding parallelization to subtests can improve test execution time.

♻️ Optional: Add t.Parallel() to subtests

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
 			err := ValidateRequiredSecondaryPath(tt.path)

Apply similar change to the other two test functions.

Also applies to: 55-68, 85-98

🤖 Prompt for AI Agents

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

In `@internal/config/validation_secondary_test.go` around lines 24 - 37, Add
t.Parallel() to each subtest goroutine so they run concurrently: inside the
t.Run anonymous func that calls ValidateRequiredSecondaryPath (the subtests
iterating over tests), insert t.Parallel() as the first statement. Apply the
same change to the other two similar test functions in this file that use t.Run
(the subtests around lines referenced in the comment) so all subtests run in
parallel.

internal/orchestrator/workflow_ui_tui_decrypt_prompts.go (1)

124-124: Minor: Double filepath.Clean on new path.

promptNewPathInputTUI applies filepath.Clean at line 124, and the caller promptExistingPathDecisionTUI applies it again at line 74. While this is harmless (idempotent), you could consider removing one for clarity.

🤖 Prompt for AI Agents

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

In `@internal/orchestrator/workflow_ui_tui_decrypt_prompts.go` at line 124, The
code calls filepath.Clean twice for the same path (once in promptNewPathInputTUI
and again in promptExistingPathDecisionTUI); remove the redundant call by
deleting the filepath.Clean invocation in promptNewPathInputTUI so it simply
returns newPath (let the caller promptExistingPathDecisionTUI perform the single
Clean), or alternatively remove the Clean in promptExistingPathDecisionTUI if
you prefer centralizing normalization in the callee—update whichever function
you choose (promptNewPathInputTUI or promptExistingPathDecisionTUI) so only one
filepath.Clean is applied.

cmd/proxsave/encryption_setup_test.go (1)

14-40: Note: Similar test UI stub exists in age_setup_workflow_test.go.

The testAgeSetupUI implementation is nearly identical to mockAgeSetupUI in internal/orchestrator/age_setup_workflow_test.go. Consider extracting a shared test helper to reduce duplication, though this is a minor concern for test code.

🤖 Prompt for AI Agents

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

In `@cmd/proxsave/encryption_setup_test.go` around lines 14 - 40, The test UI stub
testAgeSetupUI duplicates mockAgeSetupUI from
internal/orchestrator/age_setup_workflow_test.go; refactor by extracting a
shared test helper (e.g., a reusable struct and methods like
ConfirmOverwriteExistingRecipient, CollectRecipientDraft,
ConfirmAddAnotherRecipient) into a common test package or a test helper file
used by both tests, replace both local definitions with imports/uses of that
shared helper, and keep the existing method signatures and field names
(overwrite, drafts, addMore) so existing tests compile without changes.

internal/orchestrator/age_setup_ui_cli.go (1)

32-71: Consider handling invalid menu options explicitly.

The switch statement handles options "1", "2", "3" but doesn't have a default case for other inputs. If promptOptionAge returns an unexpected value (e.g., "5" or empty string), the loop silently continues and re-prompts.

This is likely acceptable behavior (re-prompt on invalid input), but an explicit default case would make the intent clearer.

💡 Optional: Add explicit default case

 		switch option {
 		case "1":
 			// ...
 		case "2":
 			// ...
 		case "3":
 			// ...
+		default:
+			fmt.Println("Invalid option. Please select 1-4.")
 		}

🤖 Prompt for AI Agents

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

In `@internal/orchestrator/age_setup_ui_cli.go` around lines 32 - 71, The
CollectRecipientDraft loop lacks an explicit default branch for the switch over
option returned by promptOptionAge; add a default case in the switch (inside
CollectRecipientDraft) that handles unexpected values by warning the user (e.g.,
via u.warn or fmt.Println) and continuing the loop so the intent to re-prompt is
explicit; reference the symbols CollectRecipientDraft, promptOptionAge,
AgeRecipientDraft,
AgeRecipientInputExisting/AgeRecipientInputPassphrase/AgeRecipientInputPrivateKey
and ErrAgeRecipientSetupAborted when locating where to add the default branch.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@cmd/proxsave/install.go`:
- Around line 555-557: The function detectTelegramCode is dead/unreferenced and
should be removed to resolve the U1000 static analysis failure; delete the
detectTelegramCode wrapper function (which simply calls
detectTelegramCodeWithContext) and ensure there are no callers expecting it—keep
detectTelegramCodeWithContext as the canonical implementation and update any
import/docs if they referenced the removed symbol.

In `@cmd/proxsave/telegram_setup_cli.go`:
- Around line 83-87: The code prints status.Message directly (in
cmd/proxsave/telegram_setup_cli.go where msg :=
strings.TrimSpace(status.Message) and fmt.Printf("Telegram: %s\n", msg) is
used), which can echo terminal escape sequences from the raw HTTP body built in
internal/notify/telegram_registration.go; fix it by sanitizing msg before
printing (e.g., strip ANSI/terminal escape sequences and control characters,
fallback to a safe truncated string), and use the sanitized value in the
fmt.Printf call so untrusted server-controlled text cannot inject terminal
escapes into the user's shell.

In `@internal/identity/identity.go`:
- Around line 90-91: The existing-id upgrade path swallows context cancellation
because maybeUpgradeIdentityFileWithContext doesn't propagate errors from
writeIdentityFileWithContext; update maybeUpgradeIdentityFileWithContext to
return an error and ensure DetectWithContext checks and returns that error
(including context.Canceled/context.DeadlineExceeded) instead of always
returning success; specifically propagate errors coming out of
writeIdentityFileWithContext through maybeUpgradeIdentityFileWithContext and
have DetectWithContext (the branch that currently calls
maybeUpgradeIdentityFileWithContext) return the error immediately when non-nil
so cancellations/timeout abort detection; apply the same pattern to the other
upgrade-call site(s) that currently ignore the returned error.

In `@internal/notify/email_relay_test.go`:
- Line 8: The test has a data race because the server goroutine mutates attempts
while the test goroutine reads it; change the attempts counter in
internal/notify/email_relay_test.go to an atomic integer (e.g., int32 or int64)
and use sync/atomic operations: replace direct increments in the handler (where
attempts is modified) with atomic.AddInt32/AddInt64 and replace reads in the
test assertions with atomic.LoadInt32/LoadInt64 so all access to attempts is
atomic and race-free.

In `@internal/orchestrator/age_setup_workflow.go`:
- Around line 245-252: mapAgeSetupAbort currently returns raw context.Canceled
from the TUI adapter calls
(ConfirmOverwriteExistingRecipient/ConfirmAddAnotherRecipient), bypassing the
expected normalization; update mapAgeSetupAbort to call the existing
normalization helper mapInputAbortToAgeAbort(err) (falling back to
ErrAgeRecipientSetupAborted when appropriate) so any context.Canceled or
input-abort errors from the TUI are normalized the same way as the CLI path;
reference mapAgeSetupAbort and mapInputAbortToAgeAbort to implement this change.

In `@internal/orchestrator/decrypt_tui_e2e_helpers_test.go`:
- Around line 53-59: The test currently locks decryptTUIE2EMu and sets orig :=
newTUIApp but only registers rollback after screen.Init(), so if screen.Init()
fails the mutex remains locked and newTUIApp isn't restored; move the
cleanup/rollback registration to immediately after acquiring the lock (i.e.,
call t.Cleanup to restore newTUIApp and unlock decryptTUIE2EMu right after
decryptTUIE2EMu.Lock() and setting orig) so that regardless of screen.Init() or
any early t.Fatalf, the mutex is unlocked and newTUIApp is rolled back.

In `@internal/orchestrator/decrypt_tui_simulation_test.go`:
- Around line 60-72: The test
TestTUIWorkflowUIPromptDecryptSecret_ZeroInputAborts verifies that entering the
rune '0' is treated as an abort and returns ErrDecryptAborted; update the
user-facing prompts in newTUIWorkflowUI and the PromptDecryptSecret flow to
explicitly mention that entering "0" will abort the operation (and/or add a
short help line displayed with path/candidate/secret prompts) so users know "0"
is a reserved abort sentinel while preserving the current ErrDecryptAborted
behavior.

In `@internal/orchestrator/encryption.go`:
- Around line 384-390: The backup path uses
recipientTime(tp).Format("20060102-150405") which is second-resolution and can
collide if two operations run in the same second; change the backup name
generation (the backupPath construction) to include higher-resolution time (e.g.
nanoseconds via Format with .000000000 or append time.UnixNano()) or a short
random/UUID suffix so each backupPath is unique, then call
copyRecipientFileWithDeps(fs, path, backupPath, perm) as before; update any
tests or callers that rely on the old format and keep references to
recipientTime and copyRecipientFileWithDeps intact.

In `@internal/orchestrator/notification_adapter.go`:
- Around line 163-176: The code renders "0%" by calling calculateUsagePercent
when total==0 before warning, so move or add the inconsistent-usage warning to
run when used>0 && total==0 prior to formatting the percent: ensure
n.warnOnInconsistentUsageStats is invoked (or calculateUsagePercent returns an
indicator) before calling calculateUsagePercent/formatPercentString so you log
diagnostic info for cases where used>0 and total==0; update the local and
secondary blocks (the calls around n.warnOnInconsistentUsageStats,
calculateUsagePercent, and formatPercentString) and the other similar blocks
that handle usage stats to follow this order or add the check inside
calculateUsagePercent to trigger n.warnOnInconsistentUsageStats for the used>0
&& total==0 case.

In `@internal/orchestrator/restore_tui.go`:
- Around line 998-1002: The error path returns without waiting for the
background ticker goroutine to exit, which can allow that goroutine to touch app
after unwinding; after app.RunWithContext(ctx) returns an error, close stopCh
and stop ticker as you're doing, then block waiting for the goroutine completion
signal (the done channel used on the success path) before returning; update the
error branch around app.RunWithContext to close stopCh, ticker.Stop(), then read
from done (or otherwise wait for the same goroutine completion signal) so the
ticker goroutine cannot run after the function begins unwinding.

In `@internal/orchestrator/restore.go`:
- Around line 1578-1586: In extractDirectory, avoid creating directories with
the archived header.Mode up-front; instead create the directory with a temporary
owner-accessible mode (e.g., 0700) via restoreFS.MkdirAll(target, ...) so
non-root processes can open it, then open the directory with
restoreFS.Open(target) and only after obtaining the handle apply the final mode
from header.Mode (e.g., via Chmod/restoreFS.Chmod) to preserve archive
permissions; update the code paths around restoreFS.MkdirAll, restoreFS.Open and
the subsequent mode application to follow this sequence.
- Around line 37-38: The current temp-file naming logic (using
restoreTempSequence and composing temp name by appending a suffix to the final
basename) can produce ENAMETOOLONG for targets near NAME_MAX; change the
temp-file creation to create the temporary file in the same directory without
depending on the final basename by calling
restoreFS.CreateTemp(filepath.Dir(target), ".proxsave-tmp-*") (replace the
manual temp-name assembly used around the restoreTempSequence usage and where
restoreGlob is used to find temp files), write to that tempfile, fsync/close as
before, then atomically rename it to target; ensure any cleanup or glob-based
detection is adjusted to match the new pattern.

In `@internal/orchestrator/tui_simulation_test.go`:
- Around line 31-57: The draw-handshake can hang because the injector goroutine
in newTUIApp waits indefinitely on readyCh; change the implementation to include
a cancellable or timeout path (e.g., accept a context with cancel or use a
time.After) so the goroutine selects between readyCh and a done/timeout signal
and exits if the prompt never draws; ensure SetAfterDrawFunc still closes
readyCh and writes to drawCh on first draw, and update the injector goroutine
(the closure started inside injectOnce.Do that calls screen.InjectKey) to return
on ctx.Done/timeouts to fail fast and avoid blocking tests.

In `@internal/storage/storage_test.go`:
- Around line 1642-1657: The UsedSpace assertion is flaky because
stats.UsedSpace and wantUsed are derived from separate safefs.Statfs snapshots
of the whole mount; replace the fragile check by either stubbing safefs.Statfs
in the test to return a stable, controlled Statfs result (so
SpaceUsageFromStatfs(wrappedStat) yields a deterministic wantUsed), or change
the assertion around stats.UsedSpace/ wantUsed to a much looser invariant that
does not rely on mount-wide stability (e.g., only assert that diff is below a
large, explicit upper bound or remove the check entirely); update references to
safefs.Statfs, SpaceUsageFromStatfs, stats.UsedSpace, wantUsed, and tolerance
accordingly so the test no longer depends on concurrent filesystem activity.

In `@internal/tui/wizard/age.go`:
- Around line 121-122: Replace the hard-coded context.Background() used when
launching the AGE confirmation dialogs with the caller-provided context (e.g.,
ctx) so the modal lifecycles respect cancellation; specifically, pass the
surrounding function's context into ageWizardRunner (and any modal start calls
for the confirmation dialogs) in both places where context.Background() is
currently used (the two ageWizardRunner invocations), ensuring the dialogs are
cancelled when the parent flow is cancelled.

In `@internal/tui/wizard/install.go`:
- Around line 705-706: The call to checkExistingConfigRunner currently uses
context.Background(), which drops the install's parent context; replace
context.Background() with the install flow's cancellable context variable (e.g.,
ctx or parentCtx used in the surrounding function) so
checkExistingConfigRunner(ctx, app, flex, modal) receives the real install
context and can be cancelled; ensure the chosen context variable is in scope
where the call to checkExistingConfigRunner is made and propagate that change
only to this call.
- Around line 665-672: The modal currently defaults focus to the first
(destructive) button and does not validate file type; before showing the modal
validate the target path using the same regular-file check (call IsRegular() or
equivalent on the config path) and return/error if not regular, and set the
modal's safe default button by calling modal.SetFocus(2) so "Keep & continue" is
focused; update the modal creation flow (where modal is constructed and
escapedConfigPath is used) to perform the IsRegular() check first and then call
modal.SetFocus(2) on the modal instance.

In `@internal/tui/wizard/new_install.go`:
- Around line 21-38: formatPreservedEntries currently calls os.Stat on the raw
trimmed entry (which resolves against the process CWD); change
formatPreservedEntries to accept a baseDir string and, before calling os.Stat,
resolve the entry against baseDir (e.g., via filepath.Join(baseDir, trimmed) or
equivalent) so directory checks use the dialog's target dir; update all callers
to pass the appropriate baseDir and apply the same resolution fix to the other
identical occurrence referenced in the diff.

---

Outside diff comments:
In `@internal/environment/detect_additional_test.go`:
- Around line 266-278: TestExtendPathIdempotent relies on the real environment
PATH and leaves changes behind; update the test to use t.Setenv to set a known
initial PATH before invoking extendPath so the test is isolated and changes are
automatically reverted, then call extendPath twice and compare os.Getenv("PATH")
values as before; locate TestExtendPathIdempotent and add a t.Setenv("PATH",
"<known-value>") (e.g., an empty or minimal PATH string) at the start to ensure
determinism and cleanup.

In `@internal/tui/wizard/install.go`:
- Around line 657-658: The CheckExistingConfig function currently only verifies
that os.Stat succeeds and therefore allows non-regular paths (directories,
FIFOs, sockets) into the TUI flow; update CheckExistingConfig to call os.Stat to
obtain FileInfo and reject any path where fi.Mode().IsRegular() is false (return
the same error/ExistingConfigAction used by the CLI flow), so non-regular config
paths are rejected before showing the prompt; reference the CheckExistingConfig
function and ensure the returned error behavior matches the existing CLI check
for consistency.

---

Nitpick comments:
In `@cmd/proxsave/encryption_setup_test.go`:
- Around line 14-40: The test UI stub testAgeSetupUI duplicates mockAgeSetupUI
from internal/orchestrator/age_setup_workflow_test.go; refactor by extracting a
shared test helper (e.g., a reusable struct and methods like
ConfirmOverwriteExistingRecipient, CollectRecipientDraft,
ConfirmAddAnotherRecipient) into a common test package or a test helper file
used by both tests, replace both local definitions with imports/uses of that
shared helper, and keep the existing method signatures and field names
(overwrite, drafts, addMore) so existing tests compile without changes.

In `@cmd/proxsave/main.go`:
- Around line 1569-1570: Remove the redundant `continue` that follows the
`<-ticker.C` receive; since `<-ticker.C` is the last statement in the loop body,
the `continue` has no effect—delete the `continue` line (near the `ticker`
receive) so the loop behavior remains unchanged and the code is cleaner.

In `@internal/config/validation_secondary_test.go`:
- Around line 24-37: Add t.Parallel() to each subtest goroutine so they run
concurrently: inside the t.Run anonymous func that calls
ValidateRequiredSecondaryPath (the subtests iterating over tests), insert
t.Parallel() as the first statement. Apply the same change to the other two
similar test functions in this file that use t.Run (the subtests around lines
referenced in the comment) so all subtests run in parallel.

In `@internal/orchestrator/age_setup_ui_cli.go`:
- Around line 32-71: The CollectRecipientDraft loop lacks an explicit default
branch for the switch over option returned by promptOptionAge; add a default
case in the switch (inside CollectRecipientDraft) that handles unexpected values
by warning the user (e.g., via u.warn or fmt.Println) and continuing the loop so
the intent to re-prompt is explicit; reference the symbols
CollectRecipientDraft, promptOptionAge, AgeRecipientDraft,
AgeRecipientInputExisting/AgeRecipientInputPassphrase/AgeRecipientInputPrivateKey
and ErrAgeRecipientSetupAborted when locating where to add the default branch.

In `@internal/orchestrator/workflow_ui_cli_test.go`:
- Around line 13-40: The captureCLIStdout helper currently restores stdout and
closes the pipe only after fn() returns, so if fn() panics or calls t.Fatal the
cleanup never runs; fix captureCLIStdout by adding a defer immediately after
creating r,w and starting the io.Copy goroutine that 1) closes w, 2) waits for
the goroutine via <-done, and 3) closes r (and keep the existing t.Cleanup for
restoring os.Stdout); update the function captureCLIStdout to use that defer
(referencing r, w, done) so stdout is restored and the io.Copy goroutine can
always exit even when fn() fails.

In `@internal/orchestrator/workflow_ui_tui_decrypt_prompts.go`:
- Line 124: The code calls filepath.Clean twice for the same path (once in
promptNewPathInputTUI and again in promptExistingPathDecisionTUI); remove the
redundant call by deleting the filepath.Clean invocation in
promptNewPathInputTUI so it simply returns newPath (let the caller
promptExistingPathDecisionTUI perform the single Clean), or alternatively remove
the Clean in promptExistingPathDecisionTUI if you prefer centralizing
normalization in the callee—update whichever function you choose
(promptNewPathInputTUI or promptExistingPathDecisionTUI) so only one
filepath.Clean is applied.

In `@internal/tui/abort_context_test.go`:
- Around line 129-152: The tests currently only check the final error and can
pass if RunWithContext returns immediately; update both
TestAppRunWithContext_NilContextRunsUntilStopped and
TestAppRunWithContext_ReturnsNilWhenStoppedWithoutCancellation to synchronize on
the app actually starting before calling app.Stop() — e.g., have
newSimulationApp provide a started channel or WaitGroup signal the test can wait
on (or otherwise expose a Started() hook), then launch the goroutine to sleep
and call app.Stop() only after that start signal, and finally assert that
app.RunWithContext(nil) / app.RunWithContext(context.Background()) remains
blocked until Stop is invoked and returns nil.

In `@internal/tui/screen_test.go`:
- Around line 11-25: The helper primitiveContainsText only inspects
*tview.TextView and *tview.Flex, so extend it to also check other relevant
primitives (e.g., *tview.Box, *tview.Form, or any primitive exposing text) or
add a generic fallback: detect and extract visible text where possible (for
example use v.GetText for Box-like types or iterate contained items for
containers), and ensure primitiveContainsText calls that extraction for types
beyond *tview.TextView/*tview.Flex so tests that render text in other primitives
succeed; update the type switch in primitiveContainsText to include cases for
*tview.Box and any other concrete primitives you expect, plus a default/fallback
branch that attempts a safe string probe.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53ae7f19-6431-4f0d-8829-b7e5bae63fc4

📥 Commits

Reviewing files that changed from the base of the PR and between 90dfba1 and 8d5f2ec.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (138)

• cmd/proxsave/encryption_setup.go
• cmd/proxsave/encryption_setup_test.go
• cmd/proxsave/helpers_test.go
• cmd/proxsave/install.go
• cmd/proxsave/install_existing_config.go
• cmd/proxsave/install_existing_config_test.go
• cmd/proxsave/install_test.go
• cmd/proxsave/install_tui.go
• cmd/proxsave/main.go
• cmd/proxsave/new_install.go
• cmd/proxsave/new_install_test.go
• cmd/proxsave/newkey.go
• cmd/proxsave/newkey_test.go
• cmd/proxsave/prompts.go
• cmd/proxsave/runtime_helpers.go
• cmd/proxsave/runtime_helpers_more_test.go
• cmd/proxsave/schedule_helpers.go
• cmd/proxsave/schedule_helpers_test.go
• cmd/proxsave/telegram_setup_cli.go
• cmd/proxsave/telegram_setup_cli_test.go
• cmd/proxsave/upgrade.go
• docs/BACKUP_ENV_MAPPING.md
• docs/CLI_REFERENCE.md
• docs/CLOUD_STORAGE.md
• docs/CONFIGURATION.md
• docs/ENCRYPTION.md
• docs/INSTALL.md
• docs/MIGRATION_GUIDE.md
• go.mod
• internal/backup/collector_pve.go
• internal/backup/collector_pve_additional_test.go
• internal/config/config.go
• internal/config/config_test.go
• internal/config/env_mutation.go
• internal/config/env_mutation_test.go
• internal/config/migration.go
• internal/config/migration_test.go
• internal/config/templates/backup.env
• internal/config/validation_secondary.go
• internal/config/validation_secondary_test.go
• internal/cron/cron.go
• internal/cron/cron_test.go
• internal/environment/detect_additional_test.go
• internal/identity/identity.go
• internal/identity/identity_test.go
• internal/logging/bootstrap.go
• internal/logging/bootstrap_test.go
• internal/notify/context_helpers.go
• internal/notify/email.go
• internal/notify/email_mailq_test.go
• internal/notify/email_parsing_test.go
• internal/notify/email_relay.go
• internal/notify/email_relay_test.go
• internal/notify/webhook.go
• internal/notify/webhook_test.go
• internal/orchestrator/additional_helpers_test.go
• internal/orchestrator/age_setup_ui.go
• internal/orchestrator/age_setup_ui_cli.go
• internal/orchestrator/age_setup_workflow.go
• internal/orchestrator/age_setup_workflow_test.go
• internal/orchestrator/backup_sources.go
• internal/orchestrator/backup_sources_test.go
• internal/orchestrator/bundle_test.go
• internal/orchestrator/decrypt_additional_test.go
• internal/orchestrator/decrypt_test.go
• internal/orchestrator/decrypt_tui.go
• internal/orchestrator/decrypt_tui_e2e_helpers_test.go
• internal/orchestrator/decrypt_tui_e2e_test.go
• internal/orchestrator/decrypt_tui_simulation_test.go
• internal/orchestrator/decrypt_tui_test.go
• internal/orchestrator/decrypt_workflow_ui.go
• internal/orchestrator/decrypt_workflow_ui_test.go
• internal/orchestrator/deps_test.go
• internal/orchestrator/encryption.go
• internal/orchestrator/encryption_exported_test.go
• internal/orchestrator/encryption_more_test.go
• internal/orchestrator/encryption_test.go
• internal/orchestrator/helpers_test.go
• internal/orchestrator/network_apply_workflow_ui.go
• internal/orchestrator/notification_adapter.go
• internal/orchestrator/notification_adapter_test.go
• internal/orchestrator/orchestrator.go
• internal/orchestrator/restore.go
• internal/orchestrator/restore_access_control_ui_additional_test.go
• internal/orchestrator/restore_errors_test.go
• internal/orchestrator/restore_firewall_additional_test.go
• internal/orchestrator/restore_ha.go
• internal/orchestrator/restore_ha_additional_test.go
• internal/orchestrator/restore_test.go
• internal/orchestrator/restore_tui.go
• internal/orchestrator/restore_tui_simulation_test.go
• internal/orchestrator/restore_tui_test.go
• internal/orchestrator/restore_workflow_integration_test.go
• internal/orchestrator/storage_adapter.go
• internal/orchestrator/storage_adapter_test.go
• internal/orchestrator/telegram_setup_bootstrap.go
• internal/orchestrator/telegram_setup_bootstrap_test.go
• internal/orchestrator/tui_screen_env.go
• internal/orchestrator/tui_simulation_test.go
• internal/orchestrator/workflow_ui_cli.go
• internal/orchestrator/workflow_ui_cli_test.go
• internal/orchestrator/workflow_ui_tui_decrypt.go
• internal/orchestrator/workflow_ui_tui_decrypt_prompts.go
• internal/orchestrator/workflow_ui_tui_decrypt_test.go
• internal/orchestrator/workflow_ui_tui_restore.go
• internal/orchestrator/workflow_ui_tui_restore_test.go
• internal/orchestrator/workflow_ui_tui_shared.go
• internal/safefs/safefs.go
• internal/safefs/safefs_test.go
• internal/security/security.go
• internal/security/security_test.go
• internal/storage/backup_files.go
• internal/storage/cloud.go
• internal/storage/cloud_test.go
• internal/storage/local.go
• internal/storage/retention.go
• internal/storage/retention_normalize.go
• internal/storage/retention_test.go
• internal/storage/secondary.go
• internal/storage/secondary_test.go
• internal/storage/storage_test.go
• internal/tui/abort_context_test.go
• internal/tui/app.go
• internal/tui/screen.go
• internal/tui/screen_test.go
• internal/tui/wizard/age.go
• internal/tui/wizard/age_test.go
• internal/tui/wizard/age_ui_adapter.go
• internal/tui/wizard/age_ui_adapter_test.go
• internal/tui/wizard/install.go
• internal/tui/wizard/install_test.go
• internal/tui/wizard/new_install.go
• internal/tui/wizard/new_install_test.go
• internal/tui/wizard/post_install_audit_tui.go
• internal/tui/wizard/post_install_audit_tui_test.go
• internal/tui/wizard/screen.go
• internal/tui/wizard/telegram_setup_tui.go
• internal/tui/wizard/telegram_setup_tui_test.go

💤 Files with no reviewable changes (2)

• internal/orchestrator/restore_workflow_integration_test.go
• internal/orchestrator/decrypt_tui_test.go