Skip to content

Commit a576dec

Browse files
committed
session: sanitize imported runtime names
1 parent ffa1516 commit a576dec

8 files changed

Lines changed: 238 additions & 11 deletions

File tree

cmd/gc/pool.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,8 +399,7 @@ func discoverPoolInstances(agentName, agentDir string, sp0 scaleParams, a *confi
399399
if templatePrefix != "" && strings.HasPrefix(qnSanitized, templatePrefix) {
400400
qnSanitized = qnSanitized[len(templatePrefix):]
401401
}
402-
// Unsanitize: "--" → "/"
403-
qn := strings.ReplaceAll(qnSanitized, "--", "/")
402+
qn := agent.UnsanitizeQualifiedNameFromSession(qnSanitized)
404403
names = append(names, qn)
405404
}
406405
}

cmd/gc/pool_session_name.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"path"
55
"strings"
66

7+
"github.com/gastownhall/gascity/internal/agent"
78
"github.com/gastownhall/gascity/internal/beads"
89
"github.com/gastownhall/gascity/internal/config"
910
)
@@ -13,9 +14,7 @@ import (
1314
// Named sessions with an alias use the alias instead.
1415
func PoolSessionName(template, beadID string) string {
1516
base := path.Base(template)
16-
// Sanitize: replace "/" with "--" for tmux compatibility.
17-
base = strings.ReplaceAll(base, "/", "--")
18-
return base + "-" + beadID
17+
return agent.SanitizeQualifiedNameForSession(base) + "-" + beadID
1918
}
2019

2120
// GCSweepSessionBeads closes open session beads that have no remaining

cmd/gc/pool_session_name_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ func TestPoolSessionName(t *testing.T) {
1717
{"claude", "mc-abc", "claude-mc-abc"},
1818
{"myrig/codex", "mc-123", "codex-mc-123"},
1919
{"control-dispatcher", "mc-wfc", "control-dispatcher-mc-wfc"},
20+
{"gs.polecat", "mc-dot", "gs__polecat-mc-dot"},
21+
{"myrig/gs.polecat", "mc-rigdot", "gs__polecat-mc-rigdot"},
2022
}
2123
for _, tt := range tests {
2224
got := PoolSessionName(tt.template, tt.beadID)

internal/agent/session_name.go

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,50 @@ import (
88
"text/template"
99
)
1010

11+
var sessionNameQualifiedReplacer = strings.NewReplacer(
12+
"/", "--",
13+
".", "__",
14+
)
15+
16+
var sessionNameQualifiedReverseReplacer = strings.NewReplacer(
17+
"--", "/",
18+
"__", ".",
19+
)
20+
1121
// sessionData holds template variables for custom session naming.
1222
type sessionData struct {
1323
City string // workspace name
14-
Agent string // tmux-safe qualified name (/ --)
24+
Agent string // tmux-safe qualified name (/ -> --, . -> __)
1525
Dir string // rig/dir component (empty for singletons)
1626
Name string // bare agent name
1727
}
1828

29+
// SanitizeQualifiedNameForSession converts a qualified identity into the
30+
// deterministic tmux-safe form used by runtime session_name values.
31+
func SanitizeQualifiedNameForSession(agentName string) string {
32+
return sessionNameQualifiedReplacer.Replace(agentName)
33+
}
34+
35+
// UnsanitizeQualifiedNameFromSession best-effort decodes a tmux-safe runtime
36+
// session name fragment back to the corresponding qualified identity.
37+
func UnsanitizeQualifiedNameFromSession(name string) string {
38+
return sessionNameQualifiedReverseReplacer.Replace(name)
39+
}
40+
1941
// SessionNameFor returns the session name for a city agent.
2042
// This is the single source of truth for the naming convention.
2143
// sessionTemplate is a Go text/template string; empty means use the
2244
// default pattern "{agent}" (the sanitized agent name). With per-city
2345
// tmux socket isolation as the default, the city prefix is unnecessary.
2446
//
25-
// For rig-scoped agents (name contains "/"), the dir and name
26-
// components are joined with "--" to avoid tmux naming issues:
47+
// For qualified identities, structural separators are encoded to avoid tmux
48+
// naming issues while preserving slash-vs-dot distinction:
2749
//
2850
// "mayor" → "mayor"
2951
// "hello-world/polecat" → "hello-world--polecat"
52+
// "gastown.mayor" → "gastown__mayor"
3053
func SessionNameFor(cityName, agentName, sessionTemplate string) string {
31-
// Pre-sanitize: replace "/" with "--" for tmux safety.
32-
sanitized := strings.ReplaceAll(agentName, "/", "--")
54+
sanitized := SanitizeQualifiedNameForSession(agentName)
3355

3456
if sessionTemplate == "" {
3557
// Default: just the sanitized agent name. Per-city tmux socket
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package agent
2+
3+
import "testing"
4+
5+
func TestSanitizeQualifiedNameForSession(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
input string
9+
want string
10+
}{
11+
{name: "plain", input: "mayor", want: "mayor"},
12+
{name: "rig scoped", input: "repo/polecat", want: "repo--polecat"},
13+
{name: "imported city scoped", input: "wendy.wendy", want: "wendy__wendy"},
14+
{name: "imported rig scoped", input: "repo/wendy.wendy", want: "repo--wendy__wendy"},
15+
}
16+
17+
for _, tt := range tests {
18+
t.Run(tt.name, func(t *testing.T) {
19+
if got := SanitizeQualifiedNameForSession(tt.input); got != tt.want {
20+
t.Fatalf("SanitizeQualifiedNameForSession(%q) = %q, want %q", tt.input, got, tt.want)
21+
}
22+
if got := SessionNameFor("city", tt.input, ""); got != tt.want {
23+
t.Fatalf("SessionNameFor(%q) = %q, want %q", tt.input, got, tt.want)
24+
}
25+
})
26+
}
27+
}
28+
29+
func TestUnsanitizeQualifiedNameFromSession(t *testing.T) {
30+
tests := []struct {
31+
input string
32+
want string
33+
}{
34+
{input: "mayor", want: "mayor"},
35+
{input: "repo--polecat", want: "repo/polecat"},
36+
{input: "wendy__wendy", want: "wendy.wendy"},
37+
{input: "repo--wendy__wendy", want: "repo/wendy.wendy"},
38+
}
39+
40+
for _, tt := range tests {
41+
if got := UnsanitizeQualifiedNameFromSession(tt.input); got != tt.want {
42+
t.Fatalf("UnsanitizeQualifiedNameFromSession(%q) = %q, want %q", tt.input, got, tt.want)
43+
}
44+
}
45+
}

internal/api/handler_agents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ func discoverUnlimitedPool(a config.Agent, poolName, cityName, sessTmpl string,
415415
if templatePrefix != "" && strings.HasPrefix(qnSanitized, templatePrefix) {
416416
qnSanitized = qnSanitized[len(templatePrefix):]
417417
}
418-
qn := strings.ReplaceAll(qnSanitized, "--", "/")
418+
qn := agent.UnsanitizeQualifiedNameFromSession(qnSanitized)
419419
result = append(result, expandedAgent{
420420
qualifiedName: qn,
421421
rig: a.Dir,

internal/api/handler_agents_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,53 @@ func TestAgentListUnlimitedPoolDiscovery(t *testing.T) {
125125
}
126126
}
127127

128+
func TestAgentListUnlimitedImportedPoolDiscovery(t *testing.T) {
129+
state := newFakeState(t)
130+
state.cfg.Agents = []config.Agent{
131+
{
132+
Name: "polecat",
133+
Dir: "myrig",
134+
BindingName: "gs",
135+
MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(-1),
136+
},
137+
}
138+
state.sp.Start(context.Background(), "myrig--gs__polecat-1", runtime.Config{}) //nolint:errcheck
139+
state.sp.Start(context.Background(), "myrig--gs__polecat-2", runtime.Config{}) //nolint:errcheck
140+
srv := New(state)
141+
142+
req := httptest.NewRequest("GET", "/v0/agents", nil)
143+
rec := httptest.NewRecorder()
144+
srv.ServeHTTP(rec, req)
145+
146+
if rec.Code != http.StatusOK {
147+
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
148+
}
149+
150+
var resp struct {
151+
Items []agentResponse `json:"items"`
152+
Total int `json:"total"`
153+
}
154+
if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil {
155+
t.Fatalf("decode: %v", err)
156+
}
157+
158+
if resp.Total != 2 {
159+
t.Fatalf("Total = %d, want 2", resp.Total)
160+
}
161+
162+
for i, item := range resp.Items {
163+
if item.Name != "myrig/gs.polecat-1" && item.Name != "myrig/gs.polecat-2" {
164+
t.Errorf("Items[%d].Name = %q, want imported pool member name", i, item.Name)
165+
}
166+
if item.Pool != "myrig/gs.polecat" {
167+
t.Errorf("Items[%d].Pool = %q, want %q", i, item.Pool, "myrig/gs.polecat")
168+
}
169+
if !item.Running {
170+
t.Errorf("Items[%d].Running = false, want true", i)
171+
}
172+
}
173+
}
174+
128175
func TestFindAgentUnlimitedPoolMember(t *testing.T) {
129176
cfg := &config.City{
130177
Agents: []config.Agent{
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
//go:build acceptance_a
2+
3+
package acceptance_test
4+
5+
import (
6+
"encoding/json"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
"time"
11+
12+
helpers "github.com/gastownhall/gascity/test/acceptance/helpers"
13+
)
14+
15+
type namedSessionListEntry struct {
16+
Template string `json:"Template"`
17+
SessionName string `json:"SessionName"`
18+
}
19+
20+
func TestImportedNamedSessionsUseSafeRuntimeNames(t *testing.T) {
21+
c := helpers.NewCity(t, testEnv)
22+
c.Init("claude")
23+
24+
rigDir := filepath.Join(c.Dir, "repo")
25+
mustWriteTestFile(t, filepath.Join(c.Dir, "pack.toml"), `
26+
[pack]
27+
name = "import-regression"
28+
schema = 2
29+
30+
[imports.gs]
31+
source = "./assets/sidecar"
32+
`)
33+
mustWriteTestFile(t, filepath.Join(c.Dir, "city.toml"), `
34+
[workspace]
35+
name = "import-regression"
36+
start_command = "sleep 300"
37+
38+
[[rigs]]
39+
name = "repo"
40+
path = "./repo"
41+
42+
[rigs.imports.gs]
43+
source = "./assets/sidecar"
44+
`)
45+
mustWriteTestFile(t, filepath.Join(c.Dir, "assets", "sidecar", "pack.toml"), `
46+
[pack]
47+
name = "sidecar"
48+
schema = 2
49+
50+
[[named_session]]
51+
template = "captain"
52+
scope = "city"
53+
mode = "always"
54+
55+
[[named_session]]
56+
template = "watcher"
57+
scope = "rig"
58+
mode = "always"
59+
`)
60+
mustWriteTestFile(t, filepath.Join(c.Dir, "assets", "sidecar", "agents", "captain", "agent.toml"), "scope = \"city\"\n")
61+
mustWriteTestFile(t, filepath.Join(c.Dir, "assets", "sidecar", "agents", "captain", "prompt.md"), "You are the imported captain.\n")
62+
mustWriteTestFile(t, filepath.Join(c.Dir, "assets", "sidecar", "agents", "watcher", "agent.toml"), "scope = \"rig\"\n")
63+
mustWriteTestFile(t, filepath.Join(c.Dir, "assets", "sidecar", "agents", "watcher", "prompt.md"), "You are the imported watcher.\n")
64+
if err := os.MkdirAll(rigDir, 0o755); err != nil {
65+
t.Fatalf("creating rig dir: %v", err)
66+
}
67+
68+
if out, err := c.GC("unregister", c.Dir); err != nil {
69+
t.Fatalf("gc unregister: %v\n%s", err, out)
70+
}
71+
72+
c.StartForeground()
73+
74+
var sessions []namedSessionListEntry
75+
deadline := time.Now().Add(20 * time.Second)
76+
for time.Now().Before(deadline) {
77+
out, err := c.GC("session", "list", "--json")
78+
if err == nil {
79+
if unmarshalErr := json.Unmarshal([]byte(out), &sessions); unmarshalErr == nil {
80+
if hasNamedSession(sessions, "gs.captain", "gs__captain") &&
81+
hasNamedSession(sessions, "repo/gs.watcher", "repo--gs__watcher") {
82+
return
83+
}
84+
}
85+
}
86+
time.Sleep(500 * time.Millisecond)
87+
}
88+
89+
out, err := c.GC("session", "list", "--json")
90+
if err != nil {
91+
t.Fatalf("gc session list --json: %v\n%s", err, out)
92+
}
93+
t.Fatalf("imported named sessions never reached safe runtime names:\n%s", out)
94+
}
95+
96+
func hasNamedSession(sessions []namedSessionListEntry, template, sessionName string) bool {
97+
for _, s := range sessions {
98+
if s.Template == template && s.SessionName == sessionName {
99+
return true
100+
}
101+
}
102+
return false
103+
}
104+
105+
func mustWriteTestFile(t *testing.T, path, contents string) {
106+
t.Helper()
107+
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
108+
t.Fatalf("creating %s: %v", filepath.Dir(path), err)
109+
}
110+
if err := os.WriteFile(path, []byte(contents), 0o644); err != nil {
111+
t.Fatalf("writing %s: %v", path, err)
112+
}
113+
}

0 commit comments

Comments
 (0)