clean-stale --instances-dir never scoped the command (enumerated default dir, deleted from the flag dir)

[XertroV]

Summary

c2c dev instances clean-stale --instances-dir=PATH never scoped the command to
PATH. It enumerated the default directory while removing from PATH,
so a stale instance in one directory could delete a same-named instance in the
other — including a running one.

As of the linked commit the flag is refused rather than silently
mis-scoped. This issue tracks making it actually work.

Root cause

let instances_dir =
  match instances_dir_override with
  | Some d -> Unix.putenv "C2C_INSTANCES_DIR" d; d   (* too late *)
  | None -> instances_dir ()
in
let all_instances = C2c_health_cmd.read_managed_instances () in

C2c_start.instances_dir is a module-level value, not a function:

let instances_dir =
  match Sys.getenv_opt "C2C_INSTANCES_DIR" with
  | Some d when String.trim d <> "" -> String.trim d
  | _ -> Filename.concat (home_dir ()) ".local" // "share" // "c2c" // "instances"

It is evaluated at module initialisation, before any cmdliner term body runs, so
Unix.putenv at command time can never affect it. read_managed_instances ()
therefore listed the default directory, while the removal loop used the local
instances_dir binding (the override):

let path = instances_dir // inst.mi_name in
if Sys.file_exists path then (rm_rf path; ...) else Some inst.mi_name

Two consequences, both bad:

• No same-named instances in PATH: nothing is deleted, but every
  default-directory candidate is reported as removed (the else Some inst.mi_name arm counts a non-existent path as a successful removal).
• Same-named instances in PATH: they are deleted based on the default
  directory's staleness verdict — so a running instance in PATH gets its
  directory removed.

Why the obvious fix is not enough

Threading ?instances_dir into read_managed_instances fixes enumeration but
not correctness. The per-instance status field is computed via
C2c_start.outer_pid_is_ours, which resolves meta.json and outer.pid
through the same module-level global. With enumeration pointed at PATH and
path resolution still pointed at the default, a running instance in PATH
is compared against the other directory's meta.json, classified
Pid_recycled → stopped, and becomes a deletion candidate.

I hit exactly this: the partial fix made a purpose-built regression test delete
a running instance. So a real fix has to thread the directory through both the
instance view and the pid-identity checks (or make C2c_start.instances_dir a
function and audit its ~call sites).

Current behaviour

$ c2c dev instances clean-stale --instances-dir=/tmp/x --dry-run
error: --instances-dir is refused: it never scoped this command and could
delete a running instance in /tmp/x. Use: C2C_INSTANCES_DIR=/tmp/x c2c dev
instances clean-stale

Exit 2. C2C_INSTANCES_DIR is resolved before module init and works correctly
today — it is what every existing clean-stale test uses, which is precisely
why none of them caught this.

Coverage

test_clean_stale_instances_dir_flag_is_refused and
test_clean_stale_instances_dir_flag_removes_nothing in
ocaml/test/test_c2c_start.ml point the env var and the flag at different
populated directories. Mutation-tested: restoring the old putenv behaviour
fails both.

Note for whoever fixes this: a fixture where both directories hold a stale
same-named instance does not discriminate — the broken code deletes the
flag directory's copy and such a test passes. The discriminating fixture is
stale-in-one, running-in-the-other.