Skip to content

Commit 7d77397

Browse files
committed
feat(cli): print one configuration line by default
- reduce the CLI banner to the credential source unless --verbose - print the env-file line too when a command fails - omit the MCP stdio hint from CLI error messages - flush the banner so it survives a buffered stderr
1 parent 97b950a commit 7d77397

5 files changed

Lines changed: 257 additions & 19 deletions

File tree

‎docs/cli-and-mcp.md‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ Global options apply to every command:
7878
- `--json` / `-j` writes machine-readable JSON to stdout and keeps osw's own
7979
progress output on stderr, so it pipes cleanly into `jq`.
8080
- `--read-only` refuses write operations.
81-
- `--verbose` / `-v` shows full tracebacks instead of a one-line message.
81+
- `--verbose` / `-v` shows full tracebacks instead of a one-line message, and
82+
adds the env-file line to the source report described under
83+
[Where settings come from](#where-settings-come-from).
8284

8385
Failures exit non-zero with a short message on stderr and no traceback.
8486

@@ -253,13 +255,29 @@ these forms:
253255
- `(accounts.pwd.yaml found in the working directory)`
254256
- `(accounts.pwd.yaml found in the working directory, ignored: no entry for domain '<domain>')`, when the discovered file is rejected for the reason given in step 2 above.
255257

258+
Those four forms all belong to a line labelled `credential file`. When no
259+
credential file is used at all, the line is labelled `credentials` instead and
260+
takes one of these forms:
261+
262+
- `OSW_USERNAME/OSW_PASSWORD (from the environment)`
263+
- `OSW_USERNAME/OSW_PASSWORD (from the env file)`
264+
- `not configured (set OSW_CRED_FILEPATH, or OSW_USERNAME/OSW_PASSWORD)`
265+
266+
The MCP server always prints both lines, the credential one first, then the
267+
env-file one. The CLI prints one line by default, naming where the
268+
credential came from. Pass `--verbose` to also print the env-file line. A
269+
failing command prints both lines regardless of `--verbose`, so a report
270+
still names every source that was checked.
271+
256272
For example, a verbose CLI run or the MCP server prints:
257273

258274
```text
259275
[osw] credential file: /home/me/project/accounts.pwd.yaml (accounts.pwd.yaml found in the working directory)
260276
[osw] env file : /home/me/project/.env (found from the working directory upward)
261277
```
262278

279+
A non-verbose, successful CLI run prints only the first of those two lines.
280+
263281
### Credentials
264282

265283
Keep credentials in a gitignored file. They are read once per process, into that

‎src/osw/cli/main.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,20 @@ def _run(op: Operation, typer_ctx: typer.Context, kwargs: dict[str, Any]) -> Non
117117
kwargs["content"] = json_value(content)
118118

119119
try:
120+
verbose = bool(opts.get("verbose"))
121+
# Before anything that can fail, so every error still reports which
122+
# files were read. --instance is validated against the credential file
123+
# this names, so the banner belongs above that check too. The env-file
124+
# line is suppressed unless the command is verbose or fails.
125+
config.log_config_sources(verbose=verbose)
126+
120127
instance = opts.get("instance")
121128
if instance:
122129
try:
123130
config.set_active_instance(instance)
124131
except ValueError as exc:
125132
raise errors.UnknownInstance(str(exc)) from exc
126133

127-
# Before load(), so a misconfiguration that makes loading raise still
128-
# reports which files were read.
129-
config.log_config_sources()
130134
settings = config.load(strict=False)
131135
policy = Policy(
132136
capture_stdout=bool(opts.get("as_json")),
@@ -138,11 +142,15 @@ def _run(op: Operation, typer_ctx: typer.Context, kwargs: dict[str, Any]) -> Non
138142
bound = bind(op, context)
139143
result = bound(**kwargs)
140144
except OpError as exc:
145+
if not verbose:
146+
config.log_env_file_source()
141147
typer.echo(f"{exc.type}: {exc}", err=True)
142148
raise typer.Exit(exc.exit_code)
143149
except Exception as exc:
144150
if opts.get("verbose"):
145151
raise
152+
# A failing command still reports every source, even non-verbosely.
153+
config.log_env_file_source()
146154
typer.echo(f"{type(exc).__name__}: {exc}", err=True)
147155
raise typer.Exit(1)
148156

‎src/osw/service/config.py‎

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -415,26 +415,45 @@ def _resolve_cred_file() -> Optional[str]:
415415
return None
416416

417417

418-
def log_config_sources(stream=None) -> None:
419-
"""Print where configuration was read from, one line per source.
418+
def _describe_env_file() -> str:
419+
"""Describe where the ``.env`` file came from, for the startup banner."""
420+
return {
421+
"explicit": f"{_env_file_path} (from {ENV_FILE[0]})",
422+
"discovered": f"{_env_file_path} (found from the working directory upward)",
423+
"none": "none found (searched from the working directory upward)",
424+
"not searched": f"not configured (set {ENV_FILE[0]} to use one)",
425+
}[_env_file_origin]
426+
427+
428+
def log_config_sources(stream=None, verbose: bool = True) -> None:
429+
"""Print where configuration was read from, to stderr.
420430
421431
Loads the ``.env`` file first if that has not happened yet, and reads the
422432
environment directly rather than a ``Settings``. Both so this can run
423433
*before* settings are loaded: a misconfiguration makes loading raise, and
424434
that is exactly when knowing which files were read matters most.
425435
436+
The credential line is printed first, since it is the one line that
437+
still appears when ``verbose`` is ``False``; the env-file line is then
438+
printed after it, only when ``verbose`` is ``True``. This keeps the two
439+
lines in the same relative order regardless of ``verbose``, including in
440+
the CLI's failure path, which prints the env-file line (via
441+
:func:`log_env_file_source`) after this one has already run.
442+
443+
``verbose`` defaults to ``True``, which keeps both lines and matches the
444+
MCP server's behaviour (it never passes a value here). The CLI passes
445+
``verbose=False`` for a successful, non-verbose command, and prints the
446+
env-file line separately (via :func:`log_env_file_source`) on failure or
447+
when the user passed ``--verbose``.
448+
426449
Always writes to ``stderr``: under MCP ``stdout`` carries the JSON-RPC
427450
stream, and under ``osw --json`` it carries the result payload.
428451
"""
429452
_load_env_file()
453+
# Every print below flushes: stderr is block-buffered whenever it is not a
454+
# terminal (a pipe, a file, or a test runner's capture buffer), and these
455+
# lines must appear before the command's own output and before any error.
430456
out = sys.stderr if stream is None else stream
431-
described = {
432-
"explicit": f"{_env_file_path} (from {ENV_FILE[0]})",
433-
"discovered": f"{_env_file_path} (found from the working directory upward)",
434-
"none": "none found (searched from the working directory upward)",
435-
"not searched": f"not configured (set {ENV_FILE[0]} to use one)",
436-
}[_env_file_origin]
437-
print(f"[osw] env file : {described}", file=out)
438457
_resolve_cred_file()
439458
if _cred_file_path:
440459
cred_described = {
@@ -446,7 +465,42 @@ def log_config_sources(stream=None) -> None:
446465
f"ignored: no entry for domain '{_first_env(ENV_DOMAIN)}')"
447466
),
448467
}[_cred_file_origin]
449-
print(f"[osw] credential file: {_cred_file_path} {cred_described}", file=out)
468+
print(
469+
f"[osw] credential file: {_cred_file_path} {cred_described}",
470+
file=out,
471+
flush=True,
472+
)
473+
else:
474+
username = _first_env(ENV_USERNAME)
475+
password = _first_env(ENV_PASSWORD)
476+
if username or password:
477+
username_name = next((n for n in ENV_USERNAME if os.getenv(n)), None)
478+
password_name = next((n for n in ENV_PASSWORD if os.getenv(n)), None)
479+
from_env_file = (
480+
username_name in _env_file_supplied
481+
or password_name in _env_file_supplied
482+
)
483+
source = "from the env file" if from_env_file else "from the environment"
484+
cred_described = f"OSW_USERNAME/OSW_PASSWORD ({source})"
485+
else:
486+
cred_described = (
487+
"not configured (set OSW_CRED_FILEPATH, or OSW_USERNAME/OSW_PASSWORD)"
488+
)
489+
print(f"[osw] credentials : {cred_described}", file=out, flush=True)
490+
if verbose:
491+
print(f"[osw] env file : {_describe_env_file()}", file=out, flush=True)
492+
493+
494+
def log_env_file_source(stream=None) -> None:
495+
"""Print only the env-file line, for the CLI's failure path.
496+
497+
Does *not* call :func:`_load_env_file`, so it is safe to call after
498+
:func:`load` has already raised: prints the line that
499+
``log_config_sources(verbose=False)`` suppressed, using whatever
500+
``_env_file_origin`` that earlier call already recorded.
501+
"""
502+
out = sys.stderr if stream is None else stream
503+
print(f"[osw] env file : {_describe_env_file()}", file=out, flush=True)
450504

451505

452506
def load(strict: bool = True) -> Settings:
@@ -542,15 +596,24 @@ def load(strict: bool = True) -> Settings:
542596
)
543597
else:
544598
fallback_hint = ""
599+
# The stdio-hang rationale only applies to the MCP server: a CLI user
600+
# is not running a stdio server, so it would only confuse them.
601+
stdio_hint = (
602+
""
603+
if _discover_env_file
604+
else (
605+
" The server refuses to start without them to avoid an "
606+
"interactive credential prompt that would hang the stdio "
607+
"transport."
608+
)
609+
)
545610
raise RuntimeError(
546611
"Missing required OSW credential environment variables: "
547612
+ ", ".join(missing)
548613
+ ". Set them in your environment or a .env file "
549614
"(pointed to by OSW_ENV_FILE / OSW_MCP_ENV_FILE), or configure a "
550615
"credential file via OSW_CRED_FILEPATH (or its OSW_MCP_CRED_FILEPATH "
551-
"/ OSL_CRED_FILEPATH aliases)." + fallback_hint + " The server "
552-
"refuses to start without them to avoid an interactive credential "
553-
"prompt that would hang the stdio transport."
616+
"/ OSL_CRED_FILEPATH aliases)." + fallback_hint + stdio_hint
554617
)
555618

556619
kwargs: dict = dict(

‎tests/test_cli.py‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,17 @@ def runner():
7777

7878

7979
def _error_lines(stderr: str) -> list[str]:
80-
"""``stderr`` minus the ``[osw]`` config banner every command prints."""
80+
"""``stderr`` minus the ``[osw]`` configuration lines."""
8181
return [
8282
line for line in stderr.strip().splitlines() if not line.startswith("[osw] ")
8383
]
8484

8585

86+
def _banner_lines(stderr: str) -> list[str]:
87+
"""The ``[osw]`` configuration lines of ``stderr``, in the order printed."""
88+
return [line for line in stderr.strip().splitlines() if line.startswith("[osw] ")]
89+
90+
8691
def _fake_osw_with_page(exists=True):
8792
page = MagicMock()
8893
page.exists = exists
@@ -424,6 +429,48 @@ def test_instance_flag_unknown_iri_exits_cleanly(runner, monkeypatch, tmp_path):
424429
result = runner.invoke(app, ["--instance", "nope.example.org", "instance", "list"])
425430

426431
assert result.exit_code == 3 # UnknownInstance
427-
assert result.stderr.strip().startswith("UnknownInstance:")
432+
assert _error_lines(result.stderr)[0].startswith("UnknownInstance:")
428433
assert "wiki-a.example.org" in result.stderr
429434
assert "Traceback" not in result.stderr
435+
436+
437+
def test_successful_command_reports_only_the_credential_source(
438+
runner, configured_env, monkeypatch, tmp_path
439+
):
440+
monkeypatch.chdir(tmp_path) # no accounts.pwd.yaml to discover
441+
442+
result = runner.invoke(app, ["instance", "list"])
443+
444+
assert result.exit_code == 0, result.stderr
445+
lines = _banner_lines(result.stderr)
446+
assert len(lines) == 1
447+
assert lines[0].startswith("[osw] credentials :")
448+
449+
450+
def test_verbose_adds_the_env_file_line(runner, configured_env, monkeypatch, tmp_path):
451+
monkeypatch.chdir(tmp_path)
452+
453+
result = runner.invoke(app, ["--verbose", "instance", "list"])
454+
455+
assert result.exit_code == 0, result.stderr
456+
lines = _banner_lines(result.stderr)
457+
assert len(lines) == 2
458+
assert lines[0].startswith("[osw] credentials :")
459+
assert lines[1].startswith("[osw] env file :")
460+
461+
462+
def test_failing_command_reports_every_source_without_verbose(
463+
runner, monkeypatch, tmp_path
464+
):
465+
# No credentials at all, so the command fails inside the operation.
466+
monkeypatch.chdir(tmp_path)
467+
468+
result = runner.invoke(app, ["instance", "list"])
469+
470+
assert result.exit_code == 1
471+
lines = _banner_lines(result.stderr)
472+
assert len(lines) == 2
473+
assert lines[0].startswith("[osw] credentials :")
474+
assert lines[1].startswith("[osw] env file :")
475+
# The stdio-hang rationale belongs to the MCP server, not to the CLI.
476+
assert "stdio transport" not in result.stderr

‎tests/test_service_config.py‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,108 @@ def test_log_config_sources_reports_cred_file_from_environment(
819819
assert "from the OSW_CRED_FILEPATH environment variable" in captured.err
820820

821821

822+
def test_log_config_sources_verbose_false_prints_only_credential_line(
823+
monkeypatch, capsys
824+
):
825+
monkeypatch.setenv("OSW_USERNAME", "alice")
826+
monkeypatch.setenv("OSW_PASSWORD", "secret")
827+
828+
config.log_config_sources(verbose=False)
829+
830+
captured = capsys.readouterr()
831+
lines = captured.err.splitlines()
832+
assert len(lines) == 1
833+
assert lines[0].startswith("[osw] credentials :")
834+
835+
836+
def test_log_config_sources_verbose_true_prints_both_lines_credential_first(
837+
monkeypatch, capsys
838+
):
839+
monkeypatch.setenv("OSW_USERNAME", "alice")
840+
monkeypatch.setenv("OSW_PASSWORD", "secret")
841+
842+
config.log_config_sources()
843+
844+
captured = capsys.readouterr()
845+
lines = captured.err.splitlines()
846+
assert len(lines) == 2
847+
assert lines[0].startswith("[osw] credentials :")
848+
assert lines[1].startswith("[osw] env file :")
849+
850+
851+
def test_log_config_sources_reports_username_password_from_env_file(
852+
monkeypatch, tmp_path, capsys
853+
):
854+
env = tmp_path / "creds.env"
855+
env.write_text("OSW_USERNAME=alice\nOSW_PASSWORD=secret\n", encoding="utf-8")
856+
monkeypatch.setenv("OSW_ENV_FILE", str(env))
857+
858+
config.log_config_sources()
859+
860+
captured = capsys.readouterr()
861+
assert "OSW_USERNAME/OSW_PASSWORD (from the env file)" in captured.err
862+
863+
864+
def test_log_config_sources_reports_username_password_from_environment(
865+
monkeypatch, capsys
866+
):
867+
monkeypatch.setenv("OSW_USERNAME", "alice")
868+
monkeypatch.setenv("OSW_PASSWORD", "secret")
869+
870+
config.log_config_sources()
871+
872+
captured = capsys.readouterr()
873+
assert "OSW_USERNAME/OSW_PASSWORD (from the environment)" in captured.err
874+
875+
876+
def test_log_config_sources_reports_credentials_not_configured(monkeypatch, capsys):
877+
config.log_config_sources()
878+
879+
captured = capsys.readouterr()
880+
assert (
881+
"not configured (set OSW_CRED_FILEPATH, or OSW_USERNAME/OSW_PASSWORD)"
882+
in captured.err
883+
)
884+
885+
886+
def test_log_env_file_source_prints_only_env_file_line(monkeypatch, tmp_path, capsys):
887+
env = tmp_path / "creds.env"
888+
env.write_text("", encoding="utf-8")
889+
monkeypatch.setenv("OSW_ENV_FILE", str(env))
890+
config._load_env_file()
891+
892+
config.log_env_file_source()
893+
894+
captured = capsys.readouterr()
895+
lines = captured.err.splitlines()
896+
assert len(lines) == 1
897+
assert lines[0].startswith("[osw] env file :")
898+
assert str(env) in lines[0]
899+
900+
901+
def test_missing_credentials_message_has_stdio_hint_when_discovery_disabled(
902+
monkeypatch,
903+
):
904+
with pytest.raises(RuntimeError) as exc:
905+
config.load()
906+
907+
assert "stdio transport" in str(exc.value)
908+
909+
910+
def test_missing_credentials_message_omits_stdio_hint_when_discovery_enabled(
911+
monkeypatch, tmp_path
912+
):
913+
# chdir away from the repo root: it has its own accounts.pwd.yaml for local
914+
# dev, which discovery would otherwise pick up and satisfy the check with.
915+
monkeypatch.chdir(tmp_path)
916+
config.set_env_file_discovery(True)
917+
918+
with pytest.raises(RuntimeError) as exc:
919+
config.load()
920+
921+
assert "stdio transport" not in str(exc.value)
922+
923+
822924
# -- Settings validation (pydantic) ------------------------------------------
823925

824926

0 commit comments

Comments
 (0)