feat(cli): per-verb help, notice policy, SQL input, and plugin install - #89
Merged
Merged
Conversation
The Lint gate aborted inside `golangci-lint config verify` before analysing any Go file: `.golangci.yml` kept the v1-era `issues.exclude-rules` key while declaring `version: "2"`, and `golangci-lint-action@v7` floated on `version: latest`, so the schema that rejected the file was whatever the runner downloaded that day. The job has been failing since 25a45f3 (2026-09-16). - `.golangci.yml`: move the exclusion to the v2 location (`linters.exclusions.rules`), keeping the same `text`/`linters` fields and the enabled linter set unchanged. - `.github/workflows/ci.yml`: pin the linter to the released v2.13.2 so an upstream release cannot flip this gate on its own. Verified with the pinned binary and the local one (2.12.2): `config verify` exits 0 and `golangci-lint run --timeout=5m` reports 0 issues. Fixes #82 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`TestApplySudoScriptEvidenceAndCleanup` failed at different assertion sites on different runs in some environments (15/16 package runs reported): nil-pointer panics on `*outcome.Executed`, digest mismatches, and wrong error kinds, with `sh: line 25: printf: write error: Broken pipe` in the fixture output. The fixture piped the generated script through the child's stdin and captured stdout/stderr into `bytes.Buffer` values, which makes `os/exec` add pipes and copier goroutines; a child that finishes while a copier unwinds can see EPIPE/SIGPIPE and report a truncated privileged result instead of the behavior under test. - Run the script from a file (`sh <script>`) with stdout/stderr going to real files in the owned fixture directory, then read them back. No stdin pipe, no copier goroutines. - Add `requireApplyOutcome` / `requireApplyExecuted`, which print the captured evidence and the parse error, and route every `*outcome.Executed` dereference through them, so a truncated report fails readably instead of panicking. - Keep all nine subtests and the truncated-report manipulation; the "rename" subtest still asserts unknown (`nil`) publication evidence explicitly. Verified: `go test ./internal/sshclient/ -count=20`, `-count=5 -short -race`, and the isolated `after` subtest repeated. Fixes #83 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`sshx apply` cleaned its staging payload, publication temp, and unverified backup with a bare `rm -f`. On a host where `rm` resolves to a wrapper earlier in PATH that moves files to the Trash (routinely done on macOS), every apply leaked a byte-identical copy of the payload while the privileged report still claimed a clean run. - Try an absolute remover (`/bin/rm`, `/usr/bin/rm`, `/usr/local/bin/rm`), then POSIX `unlink`, and only then PATH `rm`, so a shim cannot absorb the artifact when an absolute remover exists. - Success now means the path is gone (`[ ! -e ] && [ ! -L ]`), not that some remover returned 0. - Unchanged evidence semantics: a removal that cannot complete still reports `cleanup_pending`, exit 4, and `status=remote_io` / "artifact cleanup failed", and only sshx-created paths are ever removed. New `TestApplyCleanupDoesNotDependOnPathRm` shadows `rm` with a fake trash-mover and covers a published apply, a failed publication temp, an unverified backup, and an unremovable artifact. It fails against the previous helper on all three leak subtests. Fixes #79 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A batch of CLI-contract fixes and one provisioning feature, all on the execution surface Agents use. - #84 Per-verb help: every subcommand answers `sshx <verb> --help` instead of reporting `--help` as an unknown option, and `--help --json` emits the same blocks as an `sshx.help.v1` document (`sshx text --help --json` keeps its structured `sshx.text.help.v1` document). The help text now lives once in `internal/app/usage_sections.go` and is composed for both the global and the per-verb surface, so the two cannot drift. `--help` is answered in option position in any order, before any host resolution, connection, or trust-store write (a usage request previously connected and could record a host key). - #85 Compatibility mode rejects an unrecognized option instead of forwarding it as part of the remote command, names the offending token, suggests the intended option when one is close, and explains the `--` separator. The guessed `--local`/`--remote` transfer options name the real surface (`--upload=<local> --to=<remote>`), a missing upload/download destination names `--to`, and the documented `--ssh-password-key=` is now accepted. - #86 `--quiet` (`--no-notices`) suppresses human notices on stderr, so a caller that merges streams (`2>&1`) under `--json` still reads exactly one parseable document in both the success and the failure path. stdout, the exit code, and the JSON result are unchanged. Documented in docs/contract.md. - #87 `sshx sql` takes the statement from a positional argument, after `--`, from `--statement-file=PATH`, or from piped stdin, and a statement that opens with a SQL comment is statement text rather than an "unknown option". Option-shaped typos are still rejected, and the SQL guard stays fail-closed: a statement from a file or stdin goes through the identical classifier. - #78 `sshx run --target=` resolves the sudo keyring reference per host, exactly as single-target verbs do: an explicit `-pk` still wins, but the built-in `master` default no longer shadows a host's `sudo_password_key`. The rule is expressed once (`execution.SudoKeyForTarget`) and used by the plan, the SSH client, the keyring lookup, and the audit trail; each target's audit event names the credential it used, and the run summary records only a caller-level choice instead of misreporting the default. - #88 `sshx plugin install <dir> [--replace] [--trust]` provisions an existing local plugin directory through the audited CLI: it stages the source with sshx's own modes, validates it through the executor's loader before touching the published plugin, refuses symlinks and non-regular entries, bounds the copy (8MiB, 128 files), and can record the published digest in the same step. `plugin list` groups built-in capabilities and local plugins, always names the local plugin root (so "none installed" is visible), reports provenance/trust/ validity/digest, skips staging directories left by an interrupted install, and a missing plugin names the directory that was searched. A publication that cannot be renamed swaps the previous plugin back in or reports where the recovery copy was kept. Docs stay in lock-step: usage text, README.md, README_CN.md, CHANGELOG [Unreleased], docs/contract.md, docs/roadmap.md, and the bundled Agent skill. Verification: make check (fmt, vet, full suite including the compiled-binary E2E), go test -race ./internal/... ./pkg/..., make test-e2e, golangci-lint run --timeout=5m (0 issues), plus a behavioural sweep that reproduces each issue's original symptom. Fixes #78 Fixes #84 Fixes #85 Fixes #86 Fixes #87 Fixes #88 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #78, #79, #82, #83, #84, #85, #86, #87, #88.
Ten issues, all on the surface Agents actually use: option handling, help
discovery, notice discipline, credential resolution, and plugin provisioning.
Each one was reproduced first, then fixed with a regression test, and the CI
lane it belongs to is green locally (
make check,make test-e2e,go test -race ./internal/... ./pkg/...,golangci-lint run→ 0 issues).What changed
#82 — CI
Lintgate pinned (was failing onmainsince25a45f3).golangci.ymlkept the v1-eraissues.exclude-ruleskey while declaringversion: "2", andgolangci-lint-action@v7floated onversion: latest, sothe job aborted inside
config verifybefore analysing any Go file. Migrated tothe v2
linters.exclusions.rulesschema and pinned the linter to the releasedv2.13.2.#84 —
--helpis uniform, and answered before anything else happensEvery subcommand answers
sshx <verb> --help(previously onlysshx textdid);--help --jsonemits the same blocks as ansshx.help.v1document, andsshx text --help --jsonkeeps its structuredsshx.text.help.v1. The helptext now lives once, in
internal/app/usage_sections.go, and is composed forboth the global and the per-verb surface, so they cannot drift.
--helpisrecognized in option position in any order and is answered before host
resolution, connection, or any trust-store write.
#85 — compatibility mode stops silently ignoring what it does not know
An unrecognized option is rejected with the offending token, a near-miss
suggestion, and the
--rule for commands that start with-. The guessed--local/--remotenames resolve to the real transfer surface(
--upload=<local> --to=<remote>), a missing upload/download destination names--to, and the documented--ssh-password-key=is now accepted rather thanforwarded as part of the remote command.
#86 — a defined notice policy for merged streams
--quiet(--no-notices) suppresses human notices on stderr — deprecationwarnings, the policy-block mirror added by #81, sudo-boundary hints,
sshx textscan progress, logger narration — so a caller that merges streams (
2>&1)under
--jsonreads exactly one parseable document in both the success and thefailure path. stdout, exit codes, and JSON documents are untouched. The policy
is written down in
docs/contract.md.#87 — SQL statements no longer have to be shell strings
sshx sqltakes the statement positionally, after--, from--statement-file=PATH, or from piped stdin, and a statement that opens with aSQL comment is statement text instead of an "unknown sql option". Option-shaped
typos are still rejected with a suggestion, and the guard stays fail-closed: a
statement from a file or stdin goes through the identical classifier (verified:
a
DROP TABLEfrom stdin is blocked exactly like the positional form).#78 —
run --target=resolves the sudo key per hostsshx runused the CLI defaultmasterfor every target whileapplyused thehost's configured
sudo_password_key. The rule is now expressed once(
execution.SudoKeyForTarget: explicit caller key → host key → default) andhonoured by the plan, the SSH client config, the keyring lookup, and the audit
evidence. The run summary records only a caller-level choice; each target's
audit event names the credential it used.
#79 / #83 — apply cleanup and its fixture
sshx applycleaned its staging payload, publication temp, and unverifiedbackup with a bare
rm -f, so a host whosermis a trash-move wrappercollected a byte-identical copy of every applied payload. Cleanup now tries an
absolute remover, then POSIX
unlink, then PATHrm, and success means thepath is gone;
cleanup_pending/ exit 4 /remote_ioevidence semantics areunchanged. The new fixture (
TestApplyCleanupDoesNotDependOnPathRm) failsagainst the old helper on all three leak subtests. Separately, the privileged
apply fixture no longer pipes the script through stdin or captures output
through
os/execcopier goroutines, and its evidence assertions are nil-safe,which removes the intermittent failures and nil-pointer panics reported for
TestApplySudoScriptEvidenceAndCleanup.#88 — plugins can be provisioned through the CLI
sshx plugin install <dir> [--replace] [--trust]stages a plugin directory withsshx's own modes, validates it through the executor's loader before touching
the published plugin, refuses symlinks and non-regular entries, bounds the copy
(8MiB, 128 files), and can record the published digest in the same step. A
publication that cannot be renamed restores the previous plugin or reports where
the recovery copy was kept.
plugin listgroups built-in capabilities and localplugins, always names the local plugin root (so "none installed" is visible),
skips staging directories left by an interrupted install, and a missing plugin
names the directory that was searched. Matrix row and E2E updated in
docs/roadmap.md/tests/e2e/inspect_plugin_e2e_test.go.Behavior changes worth knowing
of the remote command. Callers that relied on a typo being executed (or on a
token after the options being silently absorbed) must now use
--.sshx run --target=<name>uses the host'ssudo_password_key; pass-pktooverride per invocation, exactly as before.
sshx sqlwith no statement and a piped stdin reads it to EOF (likepsql);use
--statement-filewhen another process holds the pipe open.Verification
make check(fmt, vet, full suite including the compiled-binary E2E) → passmake test-e2e→ pass;go test -race ./internal/... ./pkg/...→ passgolangci-lint run --timeout=5m→ 0 issues;golangci-lint config verify→ 0(header ordering of
--help,--quietin first position, merged-streamparsing with and without
--quiet, comment-leading SQL, stdin/file SQL,plugin install/show/list/missing-directory, apply cleanup under a fake
trash-move
rm, run sudo-key resolution in the plan and in the audit trail)README.md,README_CN.md,CHANGELOG.md [Unreleased],docs/contract.md,docs/roadmap.md, and thebundled Agent skill.
This branch was rebased onto
mainafter #81 landed; the help refactor keeps#81's new documentation blocks (policy-block mirror, non-leading sudo guidance,
text-scan monitoring) inside the new section constants, and
--quietcovers thenotices #81 added.