From 021416425d207decbe1f45827253427cfce38ffd Mon Sep 17 00:00:00 2001 From: J Date: Tue, 14 Jul 2026 19:37:42 +0800 Subject: [PATCH] =?UTF-8?q?fix(daemon):=20make=20the=20Codex=20store=20pro?= =?UTF-8?q?file=E2=86=92namespace=20map=20injective=20(MUL-4424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Elon's seventh-review blocker: the per-profile namespace was derived by dropping unsafe characters, which is not injective. The CLI treats the empty (default) profile and a profile literally named "default" as separate daemons, yet both mapped to namespace "default"; likewise "staging.prod" and "stagingprod" both mapped to "stagingprod". Two distinct daemons then shared one store tree, so one could again reclaim the other's live session — the cross-process blocker reopened for those profile names. Make codexSessionStoreNamespace injective: the empty profile gets a reserved bare literal "default", and every named profile is hex-encoded (bijective, filesystem-safe) under a "p_" prefix a bare literal can never collide with. So "" -> "default" while "default" -> "p_64656661756c74", and "staging.prod" / "stagingprod" get distinct hex segments. sanitizeCodexPathSegment stays for the UUID agent/issue segments (injective for real UUIDs); only the user-controlled profile needed the encoding. Tests: codexSessionStoreNamespace is distinct for "" vs "default", punctuation variants, case variants, and an encoded-looking name; and end-to-end, pruning one profile never reclaims the other's store for the "" vs "default" and "staging.prod" vs "stagingprod" pairs. execenv + daemon under -race, go vet, gofmt all pass. Co-authored-by: multica-agent --- server/internal/daemon/execenv/codex_home.go | 27 ++++++--- .../execenv/codex_home_sessions_test.go | 55 +++++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/server/internal/daemon/execenv/codex_home.go b/server/internal/daemon/execenv/codex_home.go index 32bc787357..f8e40611ec 100644 --- a/server/internal/daemon/execenv/codex_home.go +++ b/server/internal/daemon/execenv/codex_home.go @@ -1,6 +1,7 @@ package execenv import ( + "encoding/hex" "fmt" "io" "log/slog" @@ -210,17 +211,25 @@ func codexSessionStoreDir(sharedHome, key string) string { return filepath.Join(sharedHome, codexSessionStoreRoot, key) } -// codexSessionStoreNamespace isolates one profile-daemon's session stores from -// another's when several run on the same machine sharing one ~/.codex (profiles -// get separate daemon state but the same Codex home). Each daemon writes under, -// and only ever reclaims, its own namespace, so a staging daemon's GC can never -// delete a production task's live store and vice versa. Empty profile -> -// "default". See CodexSessionStorePath / PruneCodexSessionStores (MUL-4424). +// codexSessionStoreNamespace maps a daemon's profile to the directory segment +// that isolates its session stores from another profile-daemon's when several +// run on the same machine sharing one ~/.codex (profiles get separate daemon +// state but the same Codex home). Each daemon writes under, and only ever +// reclaims, its own namespace, so a staging daemon's GC can never delete a +// production task's live store and vice versa. +// +// The map MUST be injective: distinct profiles are distinct daemons and must +// never share a namespace. A lossy "drop unsafe characters" scheme is not — the +// CLI treats "" and "default" as separate daemons, and would sanitize +// "staging.prod" and "stagingprod" to the same segment. So the empty (default) +// profile gets a reserved bare literal, and every named profile is hex-encoded +// (bijective, filesystem-safe) under a "p_" prefix a bare literal can never +// collide with (MUL-4424). func codexSessionStoreNamespace(profile string) string { - if ns := sanitizeCodexPathSegment(profile); ns != "" { - return ns + if profile == "" { + return "default" } - return "default" + return "p_" + hex.EncodeToString([]byte(profile)) } // codexSessionStoreKey builds the per-(profile, agent, issue) key for a task's diff --git a/server/internal/daemon/execenv/codex_home_sessions_test.go b/server/internal/daemon/execenv/codex_home_sessions_test.go index 43df80c77c..2bdf196349 100644 --- a/server/internal/daemon/execenv/codex_home_sessions_test.go +++ b/server/internal/daemon/execenv/codex_home_sessions_test.go @@ -596,6 +596,61 @@ func TestPruneCodexSessionStores_IsolatesProfiles(t *testing.T) { assertAbsent(t, stagingStore) } +// TestCodexSessionStoreNamespaceInjective guards the profile->namespace map +// against the collisions Elon flagged: the CLI treats these profile pairs as +// distinct daemons, so they must never share a store namespace (a lossy +// character-dropping scheme merged them). MUL-4424. +func TestCodexSessionStoreNamespaceInjective(t *testing.T) { + t.Parallel() + pairs := [][2]string{ + {"", "default"}, // empty (default) vs a profile literally named "default" + {"staging.prod", "stagingprod"}, // punctuation must not be dropped into a collision + {"a-b", "a_b"}, + {"prod", "Prod"}, + {"p_default", "default"}, // an encoded-looking name must not alias another + } + for _, p := range pairs { + if a, b := codexSessionStoreNamespace(p[0]), codexSessionStoreNamespace(p[1]); a == b { + t.Errorf("profiles %q and %q collide in namespace %q", p[0], p[1], a) + } + } + // Deterministic for a fixed profile. + if codexSessionStoreNamespace("staging") != codexSessionStoreNamespace("staging") { + t.Error("namespace must be deterministic for a fixed profile") + } +} + +// TestPruneCodexSessionStores_NoCrossProfileCollision proves the injective +// namespace holds end-to-end: profiles the CLI treats as distinct never reclaim +// each other's stores, including the "" vs "default" and punctuation cases that +// a lossy sanitizer collapsed (Elon's round-7 repro). MUL-4424. +func TestPruneCodexSessionStores_NoCrossProfileCollision(t *testing.T) { + for _, pair := range [][2]string{{"", "default"}, {"staging.prod", "stagingprod"}} { + home := t.TempDir() + t.Setenv("CODEX_HOME", home) + a, b := pair[0], pair[1] + storeA := codexSessionStoreDir(home, codexSessionStoreKey(a, "agent", "issue")) + storeB := codexSessionStoreDir(home, codexSessionStoreKey(b, "agent", "issue")) + seedRolloutAt(t, filepath.Join(storeA, "2026", "06", "01", "rollout-2026-06-01T00-00-00-a.jsonl"), 16) + seedRolloutAt(t, filepath.Join(storeB, "2026", "06", "01", "rollout-2026-06-01T00-00-00-b.jsonl"), 16) + chtimesTree(t, storeA, time.Now().Add(-30*24*time.Hour)) + chtimesTree(t, storeB, time.Now().Add(-30*24*time.Hour)) + + // Pruning profile a reclaims a's store only — b's must survive. + if removed, _ := PruneCodexSessionStores(a, 14*24*time.Hour, time.Now(), nil, testLogger()); removed != 1 { + t.Fatalf("prune %q removed=%d, want 1", a, removed) + } + assertAbsent(t, storeA) + assertPresent(t, storeB) + + // Then pruning profile b reclaims b's store. + if removed, _ := PruneCodexSessionStores(b, 14*24*time.Hour, time.Now(), nil, testLogger()); removed != 1 { + t.Fatalf("prune %q removed=%d, want 1", b, removed) + } + assertAbsent(t, storeB) + } +} + // TestPruneCodexSessionStores_RemovesEmptyAgentDir confirms an agent directory // is cleaned up once its last issue store ages out, so the tree doesn't leave // empty / shells behind.