Skip to content

fix: apply_import was overwriting the user's real config on every cargo test#111

Merged
sassman merged 1 commit intomainfrom
fix/import-apply-writes-real-config
Apr 24, 2026
Merged

fix: apply_import was overwriting the user's real config on every cargo test#111
sassman merged 1 commit intomainfrom
fix/import-apply-writes-real-config

Conversation

@sassman
Copy link
Copy Markdown
Owner

@sassman sassman commented Apr 24, 2026

Why

  • The argumentless Config::save() / ProfileConfig::save() helpers always write to ~/.config/amoxide/, ignoring the AppModel's config_dir. apply_import used them, so tests built with AppModel::new(...) (empty config_dir, no disk write intended) still overwrote the real config.
  • Symptom: every cargo test reset ~/.config/amoxide/config.toml to the single alias produced by test_apply_import_global, silently losing any global aliases added since the previous run.

Fix

  • Route apply_import and the CLI binary through the guarded AppModel::save_* methods that honour config_dir. No behaviour change for the binary (its AppModel::default() sets the real dir anyway); library unit tests are now correctly no-op on disk.
  • Delete the argumentless Config::save / Session::save / ProfileConfig::save / SecurityConfig::save methods so the same footgun can't be reintroduced.

Verify

  • cargo test --workspace → 509 green, zero regressions.
  • stat -f "%m" ~/.config/amoxide/config.toml before and after a workspace test run → mtime unchanged.

…go test

apply_import dispatched SaveConfig and SaveProfiles through the
argumentless Config::save() and ProfileConfig::save() helpers, which
resolve their target via crate::dirs::config_dir() unconditionally —
ignoring the AppModel's config_dir. Every test that exercised
apply_import on an AppModel::new(...) model (notably
test_apply_import_global, in the default test set) wrote
  [aliases]
  ll = "ls -lha"
  [shell]
to ~/.config/amoxide/config.toml, wiping any aliases the user had
added since the previous cargo test run.

Fix both the bug and the class of bug:

1. Route apply_import through model.save_config() / model.save_profiles().
   Those methods honour the empty-config_dir guard, so apply_import
   with an AppModel::new(...) model is a correct no-op on disk.
2. Replace the remaining Config::save / Session::save /
   ProfileConfig::save / SecurityConfig::save call sites in the CLI
   binary with the equivalent AppModel::save_* accessors.
3. Delete the argumentless save() methods on all four config types.
   No caller remains; keeping them would leave the same footgun for
   future code.

Repro before the fix:
  $ echo > /tmp/before ; cat ~/.config/amoxide/config.toml  # pre-state
  $ cargo test
  $ diff /tmp/before <(cat ~/.config/amoxide/config.toml)   # wiped to ll only

Repro after the fix:
  $ stat -f %m ~/.config/amoxide/config.toml   # mtime T
  $ cargo test --workspace
  $ stat -f %m ~/.config/amoxide/config.toml   # mtime still T
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 15.38462% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.78%. Comparing base (7a48bf7) to head (76616d4).

Files with missing lines Patch % Lines
crates/am/src/bin/am.rs 0.00% 9 Missing ⚠️
crates/am/src/import_export.rs 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   74.73%   74.78%   +0.04%     
==========================================
  Files          47       47              
  Lines       12164    12152      -12     
  Branches    12164    12152      -12     
==========================================
- Hits         9091     9088       -3     
+ Misses       2898     2890       -8     
+ Partials      175      174       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sassman sassman merged commit 7f833ba into main Apr 24, 2026
50 of 51 checks passed
@sassman sassman deleted the fix/import-apply-writes-real-config branch April 24, 2026 11:42
@github-actions github-actions Bot mentioned this pull request Apr 23, 2026
sassman pushed a commit that referenced this pull request Apr 27, 2026
## 🤖 New release

* `amoxide`: 0.7.0 -> 0.8.0 (⚠ API breaking changes)
* `amoxide-tui`: 0.7.0 -> 0.8.0

### ⚠ `amoxide` breaking changes

```text
--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field Config.logging in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/config.rs:30
  field Config.logging in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/config.rs:30

--- failure derive_trait_impl_removed: built-in derived trait no longer implemented ---

Description:
A public type has stopped deriving one or more traits. This can break downstream code that depends on those types implementing those traits.
        ref: https://doc.rust-lang.org/reference/attributes/derive.html#derive
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/derive_trait_impl_removed.ron

Failed in:
  type Effect no longer derives Clone, in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:35
  type Effect no longer derives Clone, in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:35

--- failure enum_struct_variant_changed_kind: An enum struct variant changed kind ---

Description:
A pub enum's struct variant with at least one pub field has changed to a different kind of enum variant, breaking access to its pub fields.
        ref: https://doc.rust-lang.org/reference/items/enumerations.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_struct_variant_changed_kind.ron

Failed in:
  variant ProfileAction::Use in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/cli.rs:162
  variant Commands::Use in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/cli.rs:94

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_added.ron

Failed in:
  variant Commands:Sync in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/cli.rs:125
  variant Effect:PrintLines in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:55
  variant Effect:RenderSync in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:56
  variant Effect:PrintLines in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:55
  variant Effect:RenderSync in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/effects.rs:56
  variant Message:Sync in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:38
  variant Message:EnableProfiles in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:41
  variant Message:DeactivateProfiles in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:42
  variant Message:Sync in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:38
  variant Message:EnableProfiles in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:41
  variant Message:DeactivateProfiles in /tmp/.tmpZfglXO/amoxide-rs/crates/am/src/messages.rs:42

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_missing.ron

Failed in:
  variant Message::Hook, previously in file /tmp/.tmpcLonDb/amoxide/src/messages.rs:38
  variant Message::Reload, previously in file /tmp/.tmpcLonDb/amoxide/src/messages.rs:39
  variant Message::Hook, previously in file /tmp/.tmpcLonDb/amoxide/src/messages.rs:38
  variant Message::Reload, previously in file /tmp/.tmpcLonDb/amoxide/src/messages.rs:39
  variant Commands::Hook, previously in file /tmp/.tmpcLonDb/amoxide/src/cli.rs:133
  variant Commands::Reload, previously in file /tmp/.tmpcLonDb/amoxide/src/cli.rs:142

--- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/function_missing.ron

Failed in:
  function amoxide::init::generate_force_init, previously in file /tmp/.tmpcLonDb/amoxide/src/init.rs:102
  function amoxide::hook::generate_hook, previously in file /tmp/.tmpcLonDb/amoxide/src/hook.rs:13
  function amoxide::trust::render_unload_message, previously in file /tmp/.tmpcLonDb/amoxide/src/trust.rs:88
  function amoxide::subcommand::group_by_program, previously in file /tmp/.tmpcLonDb/amoxide/src/subcommand.rs:93
  function amoxide::group_by_program, previously in file /tmp/.tmpcLonDb/amoxide/src/subcommand.rs:93
  function amoxide::init::generate_reload, previously in file /tmp/.tmpcLonDb/amoxide/src/init.rs:134
  function amoxide::trust::render_load_message, previously in file /tmp/.tmpcLonDb/amoxide/src/trust.rs:54
  function amoxide::hook::generate_hook_with_security, previously in file /tmp/.tmpcLonDb/amoxide/src/hook.rs:37

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/inherent_method_missing.ron

Failed in:
  SecurityConfig::save, previously in file /tmp/.tmpcLonDb/amoxide/src/security.rs:67
  Session::save, previously in file /tmp/.tmpcLonDb/amoxide/src/session.rs:38
  Session::save, previously in file /tmp/.tmpcLonDb/amoxide/src/session.rs:38
  ProfileConfig::save, previously in file /tmp/.tmpcLonDb/amoxide/src/profile.rs:174
  ProfileConfig::save, previously in file /tmp/.tmpcLonDb/amoxide/src/profile.rs:174
  Config::save, previously in file /tmp/.tmpcLonDb/amoxide/src/config.rs:56
  Config::save, previously in file /tmp/.tmpcLonDb/amoxide/src/config.rs:56

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/module_missing.ron

Failed in:
  mod amoxide::hook, previously in file /tmp/.tmpcLonDb/amoxide/src/hook.rs:1

--- failure pub_module_level_const_missing: pub module-level const is missing ---

Description:
A public const is missing or renamed
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/pub_module_level_const_missing.ron

Failed in:
  AM_PROJECT_ALIASES in file /tmp/.tmpcLonDb/amoxide/src/env_vars.rs:7
  AM_PROFILE_ALIASES_LEGACY in file /tmp/.tmpcLonDb/amoxide/src/env_vars.rs:21
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `amoxide`

<blockquote>

##
[0.8.0](v0.7.0...v0.8.0) -
2026-04-27

### Bug Fixes

- Apply_import was overwriting the user's real config on every cargo
test ([#111](#111))
- Hook reload local aliases when individual aliases changes
([#107](#107))

### Features

- Add explicit enable/disable flags to am use
([#115](#115))
- Make shell logging on navigation events configurable
([#113](#113))
- Precedence engine with unified am sync, replaces am hook/reload
([#108](#108))

### Miscellaneous Tasks

- Bump clap from 4.6.0 to 4.6.1
([#104](#104))
</blockquote>

## `amoxide-tui`

<blockquote>

##
[0.8.0](v0.7.0...v0.8.0) -
2026-04-27

### Bug Fixes

- Apply_import was overwriting the user's real config on every cargo
test ([#111](#111))
- Hook reload local aliases when individual aliases changes
([#107](#107))

### Features

- Precedence engine with unified am sync, replaces am hook/reload
([#108](#108))
- Add explicit enable/disable flags to am use
([#115](#115))
- Make shell logging on navigation events configurable
([#113](#113))

### Miscellaneous Tasks

- Bump clap from 4.6.0 to 4.6.1
([#104](#104))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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