Skip to content

fix(install): do not remove local source dirs in CleanupDiscovery#140

Merged
runkids merged 2 commits intorunkids:mainfrom
massukio:fix/preserve-local-path-on-discovery-cleanup
Apr 22, 2026
Merged

fix(install): do not remove local source dirs in CleanupDiscovery#140
runkids merged 2 commits intorunkids:mainfrom
massukio:fix/preserve-local-path-on-discovery-cleanup

Conversation

@massukio
Copy link
Copy Markdown

Local discovery sets RepoPath to the user's directory; the UI discover handler defers CleanupDiscovery and was deleting that tree after the response. Skip RemoveAll for SourceTypeLocalPath so UI local installs match CLI behavior.

Adds TestCleanupDiscovery_PreservesLocalSource regression coverage.

Type

  • Bug fix
  • Small improvement (docs, typo, minor refactor)
  • Feature proposal (proposals/ only — see CONTRIBUTING.md)

Linked Issue

Closes #

Checklist

  • I've read CONTRIBUTING.md
  • Tests included and passing (make check) — for code changes
  • No unrelated changes in the diff
  • Scope is focused — one concern per PR

- Local discovery sets RepoPath to the user's directory.
- the UI discover handler defers CleanupDiscovery and was deleting that tree after the response.
- Skip RemoveAll for SourceTypeLocalPath so UI local installs match CLI behavior.
- Adds TestCleanupDiscovery_PreservesLocalSource regression coverage.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request prevents the accidental deletion of local user directories during the discovery cleanup process by verifying the source type before calling os.RemoveAll. It also introduces a regression test to ensure local paths remain intact. The review feedback suggests a more robust safety check by returning early if the source information is missing and using the IsGit() helper to restrict cleanup to temporary remote repositories.

Comment thread internal/install/install_discovery.go Outdated
- Address review: only RemoveAll discovery temp dirs when Source is non-nil and IsGit(), avoiding deletion for local paths, nil Source, or unknown types.
- Add TestCleanupDiscovery_SkipsWhenSourceNil.
@massukio massukio force-pushed the fix/preserve-local-path-on-discovery-cleanup branch from 8d0c418 to 2a55f88 Compare April 22, 2026 09:19
@runkids
Copy link
Copy Markdown
Owner

runkids commented Apr 22, 2026

Thanks for the quick fix and the clear write-up! 🙌

@runkids runkids merged commit d0cc43a into runkids:main Apr 22, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants