mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 01:45:52 +02:00
fix(daemon): make the Codex store profile→namespace map injective (MUL-4424)
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 <github@multica.ai>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 <agent>/ shells behind.
|
||||
|
||||
Reference in New Issue
Block a user