Skip to content

OMNIS-35: Keep package manager output out of read-only commands - #41

Merged
JtheGunner merged 1 commit into
mainfrom
fix/OMNIS-35-quiet-read-only-commands
Oct 7, 2026
Merged

JtheGunner merged 1 commit into
mainfrom
fix/OMNIS-35-quiet-read-only-commands

Conversation

@JtheGunner

Copy link
Copy Markdown
Owner

YouTrack: https://youtrack.xn--wrm-hoa.li/issue/OMNIS-35

omnishell list, diff / apply --dry-run and doctor printed the package manager's answers (for example the fzf 0.74.4 line from brew list --versions fzf) into their own output. On hosts where the package manager writes to stdout, omnishell list --json was therefore not valid JSON: it started with one stray line per probed package, followed by the JSON.

Cause

pkgmgr.ExecRunner copies the output of every command it runs to the writers buildEngine was given, which are the process's real stdout and stderr. That is right for installs and hooks, whose output the user is waiting for, but wrong for the read-only "is this package installed?" probes. The same writers are also the engine's own message writers (warning: unknown module ..., the plan text), so simply passing io.Discard would have swallowed those too.

Changes

  • internal/cli/context.go: newEngine takes separate writers for the engine and for its runner. buildEngine behaves as before. New buildQueryEngine keeps the engine's messages on stdout and stderr and drops the runner's output.
  • list, doctor (without --fix), diff and apply --dry-run use buildQueryEngine; runApply reads --dry-run before it builds the engine.
  • Unchanged on purpose: apply, doctor --fix, remove and uninstall still stream installation and hook output. validate and bench do not ask the package manager anything.
  • CHANGELOG.md: entry under Fixed.

Commits

  • fix: keep package manager output out of read-only commands

Test plan

  • New tests in internal/cli/quiet_test.go, written first and failing for the right reason: list --json is valid JSON and the table has no foreign lines even when the runner writes to the writer it is given; the runner gets io.Discard for list, list --json, diff, apply --dry-run and doctor, and the real writers for apply and doctor --fix; the engine's unknown-module warning still reaches stderr.
  • Checked on the real binary with a fake brew on PATH that prints one line per call: before the change list --json started with 15 stray lines, afterwards it parses (22 modules).
  • make vet and make test (race detector) pass; golangci-lint v2.13.2 (the CI version, via go run) reports 0 issues; go.mod and go.sum are unchanged.
  • CI: vet, race tests, golangci-lint, govulncheck and the build matrix pass.
  • Reviewer: on a host with brew or apt, run omnishell list --json | python3 -m json.tool and omnishell diff; neither may show package manager lines.

Open question (not part of this fix)

A real apply still streams everything the runner prints, including the probe answers between the plan and the installation. Probes and installs go through the same runner and writers, so they cannot be separated without changing the Runner interface; this PR leaves that alone.

@JtheGunner
JtheGunner merged commit fe84e92 into main Oct 7, 2026
4 checks passed
@JtheGunner
JtheGunner deleted the fix/OMNIS-35-quiet-read-only-commands branch October 7, 2026 09:03
@JtheGunner JtheGunner mentioned this pull request Oct 7, 2026
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.

1 participant