Skip to content

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

Description

@technicalpickles

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions