Gusto / Gusto/scope

scope-intercept: one bad ScopeKnownError file disables all known-error matching, not just itself

Open
#357 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
3
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Follow-up from #344 / #356.

#356 makes Config::new hard-fail when any ScopeKnownError fails to convert (bad reserved capture name, broken fix template), so it's no longer silently dropped. scope.rs propagates that failure and exits before running any command, which is the intended behavior there.

scope-intercept is different. It already runs the wrapped command before loading config at all, so the hard-fail doesn't block anyone's shebang script. But its fallback for a config load error is this, in src/bin/scope-intercept.rs:

let found_config = opts.config_options.load_config().await.unwrap_or_else(|e| {
    error!(target: "user", "Unable to load configs from disk: {:?}", e);
    FoundConfig::empty(env::current_dir().unwrap())
});

Before #356, a broken known-error file meant that one check silently didn't fire, everything else in .scope/ still matched normally. After #356, load_config() returns Err as soon as any ScopeKnownError file is broken, and this fallback swaps in a completely empty FoundConfig. So one typo anywhere in the known-error tree now disables known-error matching entirely for that intercept run, not just the broken file. The only signal is a downgraded error! log line, easy to miss in a wrapped command's output.

Options, roughly in order of how much they change:

  1. Leave it. It's still strictly more visible than the old silent-drop, and intercept already tolerates missing config gracefully by design.
  2. Give scope-intercept its own partial-failure path: still load whatever known errors parsed successfully, and just warn about the broken one, instead of discarding everything on any Err.
  3. Have FoundConfig::new return a partial result alongside the failures (not just Result<Self>), so every caller can decide for itself whether "some good, some bad" is acceptable.

Not blocking #356, since intercept's behavior is already a strict improvement over full silence, but the widened blast radius seemed worth a deliberate decision rather than an implicit side effect.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the fallback in src/bin/scope-intercept.rs and trace load_config through FoundConfig::new, Config::new, and scope.rs. Compare the three proposed failure-handling options and decide whether valid known-error entries should remain usable when one file fails; done means the chosen behavior is explicit and the widened blast radius is covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.