Mirrors whose names sanitize to the same token share a warn file and a WIP ref

[ligon]

Deferred from the review of #9 (fixes in #13). #13 fixes the script
collision (staging is now an atomic rename, so a second launch can no
longer truncate a running timer); this is the remaining shared state.

_sanitize_session_token maps anything outside [A-Za-z0-9._-] to _
(mirror.py:198), so K Agg and K/Agg both become K_Agg. Two such
mirrors, or one mirror launched on two targets that share a $HOME
(the reuse-probe is keyed per target, session.py:55), then share:

• $HOME/.cache/sucoder/slurm-deadline-<token>.warn — last writer wins,
  so one job's "allocation may have ended" can be read as the other's;
• refs/sucoder/wip/<token> on origin — the two jobs force-push over
  each other, so a snapshot can be silently replaced by an unrelated
  tree's. With feat(slurm): local-disk tiering for salloc targets; retire the all-on-/local layout #11/feat(prelude): WORKSPACE block tells the agent about local-disk tiering #12 this is live.

The second is the one that loses work.

For "the same mirror on two targets" this is arguably correct — it is
one mirror. For two genuinely different mirrors that happen to collide
it is not. Options: include the target name in the token, use a short
hash of the unsanitized name as a suffix, or refuse at config-load time
when two configured mirrors sanitize alike (cheapest, and it turns a
silent data-loss path into an error at the only moment someone can fix
it).

Low likelihood — it needs deliberately awkward mirror names — but the
failure is silent and costs work, so it should not just stay implicit.

🤖 Generated with Claude Code

https://claude.ai/code/session_018Ty45bJv1jADvUbyLaW4RD

[ligon]

Checked this while triaging #19 and #14. The second bullet — the one described here as "the one that loses work" — does not hold for two genuinely different mirrors. The rest does, and it misses one.

The WIP refs do not collide

The snapshot is pushed to origin, and origin is that mirror's own shared repository. The mirror path never passes through _sanitize_session_token:

mirror_path     = remote.mirror_root / settings.mirror_dirname   # mirror.py:190
mirror_dirname  = mirror_name                                    # config.py:388-389

mirror_dirname is the raw name. K Agg lives at ~/mirrors/K Agg, K/Agg at ~/mirrors/K/Agg — two repositories, two ref namespaces. refs/sucoder/wip/K_Agg in each cannot reach the other. Same for the local-tier clone's origin, which is set to $MIRROR (local_tier.py:128).

So for two colliding-but-distinct mirrors there is no shared ref and no silent snapshot replacement.

What does collide

The shared-$HOME state, exactly as described:

• $HOME/.cache/sucoder/slurm-deadline-<token>.warn (slurm_timer.py:124) — last writer wins;
• $HOME/.cache/sucoder/job-<token>.sh and local-tier-<token>.sh (mirror.py:2808,2819) — the staged batch and prepare scripts. (The timer script is safe: mirror.py:2813 gives it a secrets.token_hex(8) suffix.)

And one this issue does not name, which is worse than either because it fails immediately rather than silently: confined_tmux_target (mirror.py:213-222) builds both the tmux session name and its dedicated socket as sucoder-<token>. Two colliding mirrors confined on the same node land on one tmux server. The comment right above that function already says why that is fatal — the spike found a shared socket reuses another cgroup's server and the new session dies on contact.

With local-disk tiering there is a third: work_path() is <root>/job<ID>/mirrors/<token> (local_tier.py:43-46). Two colliding mirrors would only share it inside one job id, which normally cannot happen — but _adopt_existing_allocation (cli.py:824) adopts a live job by node, so the collaborate path can put a second mirror in an existing job. The prepare script would then find $WORK/.git already present and git remote set-url origin "$MIRROR" (local_tier.py:128) — repointing one mirror's clone at the other. Worth confirming before relying on "cannot happen".

Suggested re-scope

Drop the WIP-ref bullet; keep the warn file and the staged scripts; add the tmux session/socket and the tiering work path. The proposed cheapest fix — refuse at config-load time when two configured mirrors sanitize alike, naming both — is unaffected by the correction and still settles all of them at once. It is also the only one of the three options that does not require every consumer of the token to agree on a new scheme.

The one case where the ref claim is true

or one mirror launched on two targets that share a $HOME

That is one mirror, one origin, two jobs — which is #19, not a token collision. #19 has the live evidence and the D/F-conflict finding that constrains the fix. Suggest this issue hand that case to #19 explicitly and keep only the token-collision scope.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lb2GYSyS7vteb34ziwHQ4c

[ligon]

A gap in the config-load refusal that landed with #21, found while tracing an unrelated attach symptom.

_reject_token_collisions runs inside _parse_mirrors (config.py:944) — i.e. only over mirrors that are configured, at load time. But _create_ephemeral_mirror (cli.py) injects a mirror straight into the live config at runtime:

mirror_name = git_toplevel.name
...
config.mirrors[mirror_name] = ephemeral   # config.mirrors is a plain dict at runtime

That path never goes through _parse_mirrors, so the refusal does not see it. An ephemeral mirror is named from the git root directory name, which can contain a space or any other character the token collapses to _. So a repo in ~/work/K Agg operated on with a bare sucoder collaborate still collides with a configured K/Agg on everything this issue lists: the tmux session and its socket, job-<token>.sh, local-tier-<token>.sh, the warn file, and (with tiering) the local-disk clone path.

Same likelihood class as the original — it needs an awkward directory name rather than an awkward config entry — and the same silent failure. But "the cheapest fix, and it turns a silent data-loss path into an error at the only moment someone can fix it" is weaker than it looked, because config load is not the only moment a mirror comes into existence.

Two options, neither expensive:

1. Check inside _create_ephemeral_mirror: it already has config.mirrors in hand, so comparing the new name's token against the existing ones is a couple of lines, and erroring there is still "the moment someone can fix it" (they can pass an explicit mirror name or rename the directory).
2. Move the check to wherever the token is first consumed — confined_tmux_target is the natural chokepoint, since it is the collision that fails hardest — and have it refuse when two known mirrors map to its argument. Catches any future injection path for free, but it fires later, when the user is mid-launch rather than mid-edit.

(1) preserves the existing "fail at the moment of definition" shape and is what I would do. Leaving this open rather than patching it blind, since it is really a question about how many ways a mirror can come into existence.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lb2GYSyS7vteb34ziwHQ4c

[ligon]

Status: the configured half is closed; the ephemeral half is the whole remaining issue

The cheapest of the three options in the body — refuse at config load — has landed. _reject_token_collisions (config.py:945) raises on two configured mirrors sharing a token, and names all of the shared state:

Mirrors `K Agg`, `K/Agg` both sanitize to the session token `K_Agg`, so they
would share a tmux session and socket, the staged batch and prepare scripts
in $HOME, the deadline warn file, and the local-disk working clone path.
Rename one so they differ in more than non-[A-Za-z0-9._-] characters.

That closes the case the issue body is written around ("two genuinely different mirrors that happen to collide"), because both have to be configured to collide, and config load now refuses.

What it does not cover is the path that does not go through config load. _create_ephemeral_mirror (cli.py:1444) builds a MirrorSettings at runtime and injects it into config.mirrors:

mirror_name = git_toplevel.name

The name comes straight from the git root's directory name, which is unconstrained — it can contain spaces, slashes are impossible but almost anything else is not — and it is injected after _reject_token_collisions has already run. So an ephemeral mirror can still collide with a configured one, or with another ephemeral one, and it does so silently. This is also the more likely collision in practice: nobody deliberately configures K Agg and K/Agg, but ~/work/K Agg is an ordinary directory to have.

Note the severity is not uniform across the shared state. The handoff's re-scoping is right that the WIP-ref claim in the body no longer holds as written (mirror_dirname is the raw name), but the tmux session and socket, the staged scripts, and the warn file are all keyed on the sanitized token and do still collide. Per the body's own note, the tmux collision "fails first and hardest", so an ephemeral collision surfaces as a session that attaches to the wrong job rather than as silent data loss — bad, but louder than the original framing feared.

Suggested shape

Run the same check on the ephemeral path, which is a two-line change and reuses the landed function: after deriving mirror_name, call _reject_token_collisions(list(config.mirrors) + [mirror_name]) before injecting. The error message is already written and already names the right consequences. It turns a silent wrong-session attach into a refusal at the one moment the user can fix it — by cd-ing somewhere sane or configuring the mirror explicitly — which is the same argument that made the config-load refusal the preferred option.

Leaving this open for that. Happy to implement if the shape looks right.

[ligon]

Implemented in d84238b (merged as a2763ed): _create_ephemeral_mirror now runs _reject_token_collisions over the configured names plus the ephemeral one, before injecting.

Reusing the landed function means the error already names the right shared state and the right remedy, and it covers both call sites in _resolve_mirror_name rather than just the no-arg path.

Three tests: a ~/work/K Agg against a configured K_Agg refuses and injects nothing; a non-colliding name still injects; and a name with no configured mirrors at all does not report itself as a collision — that last one because both callers currently guarantee the name is not already configured, and if that ever changed a self-collision would block every command in the repo. Mutation-checked by removing the guard.

One thing to keep in mind about severity, which I noted in the previous comment: this refuses earlier, it does not newly detect something that was silently corrupting data. Per this issue's own framing the tmux collision "fails first and hardest", so what it converts is a wrong-session attach into a refusal at the moment somebody can fix it.

Closing — both halves are now covered, config-load and ephemeral.