Skip to content

Fix(cli): QA sweep: help alignment, vault remove UX, startup stall, record-page frontmatter, config-file safety - #6

Merged
adibhanna merged 10 commits into
mainfrom
fix/cli-qa-sweep
Oct 1, 2026
Merged

adibhanna merged 10 commits into
mainfrom
fix/cli-qa-sweep

Conversation

@adibhanna

Copy link
Copy Markdown
Contributor

Summary

An end-to-end QA sweep of every zn command on a Linux box (Ubuntu 26.04, Go 1.27), against a local vault, a Docker-hosted ZenNotes server (adibhanna/zennotes:2.56.0 via zn connect), and native managed servers under systemd --user (real znserver 2.55.0 → 2.56.0 release downloads, update and rollback). Roughly 1,400 scripted assertions over ~730 logged command invocations, plus MCP over stdio (35 tools) and the TUI driven in a real pty (open, :e, insert-mode edit, :w, :qa, remote server, first-run wizard). Everything that was broken is fixed below with a regression test; the managed-server lifecycle, remote parity, MCP and TUI held up to everything else.

Starts from the two Discord reports (help misalignment, vault remove trial-and-error).

Fixes

# Problem Fix
1 Help columns misaligned (Discord). Names wider than the command column (comment reply <path> <id> "<body>", --workspace-source <app|terminal>) ran straight into their descriptions. A name too wide for the column takes its own line; the description always starts at the column. Test asserts every row across --help and scoped help.
2 vault remove discoverability (Discord). vault list showed the desktop app's vaults/servers indistinguishably from zn's own; vault remove could only forget zn's own and its error pointed back at the list. vault list marks each entry's source (terminal/app, text and JSON) and says which commands act on which. vault remove/disconnect/use explain when an entry belongs to the desktop app, suggest the one saved name containing what was typed (workspace → workspace (zennotes.mydomain.com)), and list zn's own names. vault remove of a server also drops its token (as disconnect does), supports --json, completes names; zn use <desktop vault name> adopts that folder.
3 Every command stalled 5 s on terminals that don't answer OSC 11 (Linux console, some IDE/serial terminals), even zn --version / --desktop-integration. Bubble Tea v1 queries the terminal background in a package init. The late reply could also land in the zn setup prompt's stdin read. New internal/termbg initialises before Bubble Tea (Go orders package init by import path among ready packages) and presets Lip Gloss's answer, so startup never touches the terminal. The two consumers of the real answer (TUI auto theme, read --pretty) call termbg.Detect() deliberately. Linux pty test runs the binary on a terminal that never answers and fails on any query or slow start. Measured: 5.04 s → 0.02 s.
4 Record pages grew --- fences on every base set. ComposePageBody wrote an empty frontmatter block for rows with only a title; Frontmatter() didn't recognise it, so each re-mirror prepended another pair. Frontmatter accepts an empty block (empty form tried first so a body's horizontal rule can't be taken as the closing fence); no block is written when there is nothing to mirror; re-mirroring is idempotent and old pages shed stray fences.
5 --body "---\n…" rejected as "needs a value": any value starting with -- was treated as the next flag, so a frontmatter body couldn't be passed. Only --, -h and --<letter> count as the next flag; --- and -x are values.
6 Unparseable workspaces.toml/credentials.toml treated as empty → the next connect/init/vault add wrote the empty list back and lost every saved vault, server and token. Load remembers the parse error; saving refuses with the error and what to do; a broken token store yields no tokens. doctor gains a "saved vaults" check and extends the credentials check; status warns; vault list says zn's own entries aren't shown.
7 zn delete <missing> --yes printed "Deleted", exit 0 locally (server 404s). Local vault reports Note not found.
8 zn tag find --tag <t> parsed, then rejected for a missing positional. The flag satisfies the positional, like --path elsewhere.
9 zn lsit suggested init, not list. Typo suggestions count adjacent swaps as one edit.
10 Ctrl-C on zn server run printed context canceled twice, exit 130; zn mcp exited 1 with server is closing: EOF whenever the client closed the pipe (normal MCP shutdown). Both exit quietly.

Out of scope, noted from the same Discord thread: "closing the local vault returns to the welcome screen even with a remote connection" is desktop-app behaviour, not this repo.

Verification

  • go test -race ./..., go vet ./..., gofmt -l, git diff --check on Linux; go test ./... on macOS; cross-compiles for windows/amd64, darwin/arm64, linux/arm64; go mod tidy is a no-op.
  • bash scripts/sandbox.sh --smoke and bash scripts/sandbox.sh --server --smoke (real server download) pass.
  • QA phases with the fixed binary, all green: local vault 296 + data matrix 245; Docker server connect 38 + matrix 245 + disconnect 7; managed servers 151 (install → start → status/logs → restart → config --bind/--base-path incl. rollback on an occupied port → update --check → update 2.55.0→2.56.0 → --rollback → pin/downgrade → stop → foreground run → second instance); diagnostics/config/completion/vault commands 187; MCP + TUI (pty) 36.
  • The Linux box was cleaned afterwards: containers, images, systemd units, QA directory and temp files removed; the user's real ZenNotes config was never touched (everything ran under ZENNOTES_CONFIG_DIR).

Behaviour changes to be aware of

  • vault list JSON gains a source field; text output gains a source column and a hint only when desktop-app entries are present.
  • vault remove <server> now also removes the saved token (matches disconnect).
  • zn delete of a missing note is now an error on local vaults too.
  • Commands that would save over an unparseable workspaces.toml/credentials.toml now fail instead of clobbering it.
  • zn server run exits 0 on Ctrl-C; zn mcp exits 0 when the client disconnects.

…lt remove can forget

Names wider than the help's command column ran straight into their
descriptions (`"<body>"Answer in a thread`, `<app|terminal>Follow the
desktop…`). Such a name now takes a line of its own and the description
starts at the column, so every row stays aligned.

`zn vault list` shows the desktop app's vaults and servers next to zn's
own without saying so, and `zn vault remove` could only forget zn's own;
its error pointed back at the list. The list now marks each entry's
source (terminal or app, also in the JSON) and says which commands act on
which. `vault remove`, `disconnect` and `use` explain that an entry
belongs to the desktop app, suggest the one saved name containing the
word that was typed, and list zn's own names. `vault remove` of a server
also drops its token, as `disconnect` does, supports --json, and
completes zn's saved names. `zn use <desktop vault name>` adopts that
folder instead of failing.
… startup

Bubble Tea v1 asks Lip Gloss for the terminal background in a package
init, so on a terminal that never answers OSC 11 (the Linux console, some
IDE and serial terminals) every zn command, `zn --version` included,
stalled for termenv's five-second timeout, and a late answer could land in
the next stdin read, such as the `zn setup` prompt.

internal/termbg initializes before Bubble Tea (Go orders package
initialization by import path among ready packages, and ZenNotes sorts
before charmbracelet) and presets Lip Gloss's answer from COLORFGBG or the
dark default, so that init no longer touches the terminal. The two places
that need the real answer, the terminal app's auto theme and
`zn read --pretty`, call termbg.Detect, which asks the terminal once and
on purpose. A Linux pty test runs the binary against a terminal that
never answers and fails if a query is written or startup takes seconds.
… growing fences

A record page for a row with only a title was written with an empty
frontmatter block (`---\n---`), which the frontmatter regex did not
recognise, so every `zn base set` re-mirror treated the fences as body
and prepended another pair: four fences after one edit, six after two.

Frontmatter now accepts an empty block, trying the empty form before the
lazy one so a horizontal rule later in the body cannot be mistaken for
the closing fence; prepend and excerpts honour it too. ComposePageBody
writes no block when a row has no properties to mirror, so re-mirroring
is idempotent, and pages from older builds shed their stray fences on
the next pass. The unused frontmatterOnlyR copy in tasks.go is gone.
…, rank swaps as one typo

A value flag refused any next token starting with `--`, so a body that
opens with a frontmatter fence failed with "--body needs a value". Only
`--` itself, `-h` and `--<letter>` count as the next flag now; `---`
and `-x` are values. `zn tag find --tag <t>` was accepted by the parser
and then rejected for a missing positional; the flag satisfies it like
--path does elsewhere. Typo suggestions count a swap of adjacent letters
as one edit, so `lsit` suggests list instead of init.
… could not parse

A file that failed to parse was treated as an empty list, so the next
command that saved (connect, init, vault add, use, a server setup) wrote
that empty list back and the user lost every saved vault, server and
token. The load now remembers the problem, saving refuses with the parse
error and what to do, and a broken token store yields no tokens rather
than a fresh file holding one. `zn doctor` gains a "saved vaults" check
and extends the credentials check to parse errors; `zn status` warns and
`zn vault list` says zn's own entries are not being shown.
…rver run or MCP session ends

`zn delete <missing> --yes` on a local vault printed "Deleted" and exited
0 while a server answers 404; the vault now reports "Note not found".
Ctrl-C on `zn server run` printed "context canceled" twice (the exec
error joined with the context's) and exited 130 for a shutdown the
operator asked for; it exits 0 after the server's own shutdown log line.
`zn mcp` exited 1 with "server is closing: EOF" whenever the client
closed the pipe, which is how every MCP client ends a session.
Prefer exact saved names during removal, report token cleanup failures, and restore tokens if saving the workspace list fails. Reject invalid credential tables, keep tokens out of parse diagnostics, and preserve the default when connecting with corrupt credentials. Add regression coverage for the PR #6 review findings.
Only suppress cancellation and normal EOF disconnects, including the SDK's exact closing message. Preserve genuine transport failures and deadlines, with regression coverage for normal client disconnects and error classification.
@adibhanna adibhanna changed the title Fix(cli): QA sweep — help alignment, vault remove UX, 5s startup stall, record-page frontmatter, config-file safety Fix(cli): QA sweep: help alignment, vault remove UX, startup stall, record-page frontmatter, config-file safety Oct 1, 2026
@adibhanna
adibhanna merged commit 4c4038e into main Oct 1, 2026
5 checks passed
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