internal/cli's open() leaks partition files, as Server.Close did

Type

Bug — resource leak, unreachable today

Found while

SAPE-19

Triage

Defer

Status

Open

Severity

Low — harmless while one command is one process, and only while that holds

Description

open() (internal/cli/cli.go:562) opens a database for a CLI command and returns a closer. A database is not one file: after opened.Keep(folder, settings.Key) the store opens a file per partition it is asked for, each under an exclusive lock of its own. The closer lets go of the leader and the directory lock, and nothing else:

// internal/cli/cli.go:674
return opened, func() { pages.Close(); held.Close() }, nil

opened.Close() — the call that gives the partition files back — is never made.

Evidence

• This is the same bug, in the same shape, that was fixed on the server side in 390e976. Server.Close now closes the partitions first, then the leader, and the comment at internal/server/server.go:349-357 is the diagnosis written out: "closing db.pages lets go of the leader only. Before this line, those partition files stayed locked for the life of the process… and the next Server on the same directory was refused with vfs.ErrLocked on a .part file until a garbage collection happened to run their finalizers."
• The CLI closer never calls store.Close. The store is the only holder of the partition handles; once the closer runs, they are unreachable with nobody holding a name for them.
• Why it is invisible today: every sapedb subcommand is one process that exits when it is done, and process exit releases the locks the same way Close would. The server is a long-lived process, which is why the same bug was fatal there and is not here.

Triage

Does it lose data, break a promise on a public surface, or stop somebody installing and running? No. Nothing is written or lost; the file's contents are committed before the closer runs. Nobody's install or first run is affected. The leaked handles live exactly as long as the process that made them.

Defer. It is a correctness debt whose only trigger is a second open() in one process, and today no code path does that: internal/cli's own package doc says a command is refused while the server is up, and each subcommand is its own process.

Worth being explicit that the reasoning is about the caller, not the code — the code is wrong, and the thing keeping it from mattering is an assumption ("one command, one process") that nothing in the tree enforces.

If deferred, what it costs to wait

• The day that assumption changes, this stops being free and starts being the same symptom the server had: an operation refused with vfs.ErrLocked on a .part file, with no obvious cause, until a finalizer happens to run. The likely triggers are an interactive shell session that opens more than one database, a test harness that opens several in one process, or anything that embeds the CLI as a library.
• The shell (internal/cli/shell.go) is already the longest-lived thing in this package, which makes it the nearest candidate.
• The fix is one line in the closer, in the order the server already settled on: partitions, then leader, then the directory lock. Waiting does not make it harder — it only makes it more likely that somebody rediscovers it from the symptom instead of from this page.