Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| edbc078321 |
@@ -538,10 +538,10 @@ func splitChatIDs(raw string) []string {
|
||||
return ids
|
||||
}
|
||||
|
||||
// truncID is a no-op: workspace IDs are full UUIDs used consistently in
|
||||
// container naming, A2A routing, and logging. Previously truncated to 12
|
||||
// characters; retained as-is since it is only used for log brevity.
|
||||
func truncID(id string) string {
|
||||
if len(id) > 12 {
|
||||
return id[:12]
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
|
||||
@@ -452,12 +452,9 @@ func resolveOrgTemplate(configsDir, wsName string) (path, label string) {
|
||||
|
||||
// configDirName returns the standard config directory name for a workspace ID.
|
||||
// Used by resolveConfigDir in templates.go for host-side template resolution.
|
||||
// Uses the full workspace UUID to match container naming (no truncation).
|
||||
func configDirName(workspaceID string) string {
|
||||
id := workspaceID
|
||||
if len(id) > 12 {
|
||||
id = id[:12]
|
||||
}
|
||||
return "ws-" + id
|
||||
return "ws-" + workspaceID
|
||||
}
|
||||
|
||||
// knownRuntimes is the allowlist of runtime strings the provisioner will
|
||||
|
||||
@@ -46,10 +46,12 @@ func TestConfigDirName(t *testing.T) {
|
||||
expected string
|
||||
}{
|
||||
{"abc-def-ghi", "ws-abc-def-ghi"},
|
||||
{"abcdefghijklmnop", "ws-abcdefghijkl"}, // truncated at 12
|
||||
{"abcdefghijklmnop", "ws-abcdefghijklmnop"}, // no truncation
|
||||
{"short", "ws-short"},
|
||||
{"123456789012", "ws-123456789012"}, // exactly 12
|
||||
{"1234567890123", "ws-123456789012"}, // 13 chars, truncated
|
||||
{"123456789012", "ws-123456789012"}, // exactly 12
|
||||
{"1234567890123", "ws-1234567890123"}, // 13 chars, full
|
||||
// Real UUID: matches container naming (no truncation).
|
||||
{"ab078012-c305-42be-b93d-b6e8a78d5409", "ws-ab078012-c305-42be-b93d-b6e8a78d5409"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
@@ -130,11 +130,7 @@ const (
|
||||
|
||||
// ConfigVolumeName returns the Docker named volume for a workspace's configs.
|
||||
func ConfigVolumeName(workspaceID string) string {
|
||||
id := workspaceID
|
||||
if len(id) > 12 {
|
||||
id = id[:12]
|
||||
}
|
||||
return fmt.Sprintf("ws-%s-configs", id)
|
||||
return fmt.Sprintf("ws-%s-configs", workspaceID)
|
||||
}
|
||||
|
||||
// ClaudeSessionVolumeName returns the Docker named volume for a workspace's
|
||||
@@ -142,11 +138,7 @@ func ConfigVolumeName(workspaceID string) string {
|
||||
// config volume so it can be discarded independently (via WORKSPACE_RESET_SESSION
|
||||
// or ?reset=true) without wiping the user's config. Issue #12.
|
||||
func ClaudeSessionVolumeName(workspaceID string) string {
|
||||
id := workspaceID
|
||||
if len(id) > 12 {
|
||||
id = id[:12]
|
||||
}
|
||||
return fmt.Sprintf("ws-%s-claude-sessions", id)
|
||||
return fmt.Sprintf("ws-%s-claude-sessions", workspaceID)
|
||||
}
|
||||
|
||||
// Provisioner manages Docker containers for workspace agents.
|
||||
@@ -164,12 +156,11 @@ func New() (*Provisioner, error) {
|
||||
}
|
||||
|
||||
// ContainerName returns the Docker container name for a workspace.
|
||||
// The full workspace UUID is used (not truncated) to ensure A2A routing
|
||||
// URLs resolve correctly: InternalURL() derives the dispatch hostname from
|
||||
// this value, and both must match the container's actual hostname.
|
||||
func ContainerName(workspaceID string) string {
|
||||
id := workspaceID
|
||||
if len(id) > 12 {
|
||||
id = id[:12]
|
||||
}
|
||||
return fmt.Sprintf("ws-%s", id)
|
||||
return fmt.Sprintf("ws-%s", workspaceID)
|
||||
}
|
||||
|
||||
// containerNamePrefix is the shared prefix every workspace container
|
||||
@@ -196,12 +187,11 @@ func managedLabels() map[string]string {
|
||||
return map[string]string{LabelManaged: "true"}
|
||||
}
|
||||
|
||||
// ListWorkspaceContainerIDPrefixes returns the 12-char workspace ID
|
||||
// prefixes of every running ws-* container the Docker daemon knows
|
||||
// about. The 12-char form matches ContainerName's truncation, so the
|
||||
// orphan sweeper can intersect this set against `SELECT
|
||||
// substring(id::text, 1, 12) FROM workspaces WHERE status = 'removed'`
|
||||
// without an extra round-trip per row.
|
||||
// ListWorkspaceContainerIDPrefixes returns the full workspace ID
|
||||
// of every running ws-* container the Docker daemon knows about.
|
||||
// The IDs are used by the orphan sweeper to query `SELECT id FROM
|
||||
// workspaces WHERE status = 'removed' AND id = ANY($1)` to find leaked
|
||||
// containers without an extra round-trip per row.
|
||||
//
|
||||
// Returns an empty slice on any Docker error (sweeper treats that as
|
||||
// "skip this round" — better than a partial scan that misses leaks).
|
||||
@@ -222,8 +212,8 @@ func (p *Provisioner) ListWorkspaceContainerIDPrefixes(ctx context.Context) ([]s
|
||||
prefixes := make([]string, 0, len(containers))
|
||||
for _, c := range containers {
|
||||
// Container names from the API include a leading slash:
|
||||
// "/ws-abc123def456". Strip both the slash and our prefix
|
||||
// to recover the 12-char workspace ID.
|
||||
// "/ws-<uuid>". Strip the slash and our prefix to recover the full
|
||||
// workspace ID.
|
||||
//
|
||||
// The Docker name filter is a SUBSTRING match (not a prefix
|
||||
// match), so something like "my-ws-thing" would also be
|
||||
@@ -246,7 +236,7 @@ func (p *Provisioner) ListWorkspaceContainerIDPrefixes(ctx context.Context) ([]s
|
||||
return prefixes, nil
|
||||
}
|
||||
|
||||
// ListManagedContainerIDPrefixes returns the workspace ID prefix of every
|
||||
// ListManagedContainerIDPrefixes returns the full workspace ID of every
|
||||
// container carrying the LabelManaged stamp. Distinct from
|
||||
// ListWorkspaceContainerIDPrefixes (name-filtered, may include sibling
|
||||
// platforms' containers on a shared Docker daemon): this method is the
|
||||
|
||||
@@ -357,8 +357,10 @@ func TestContainerName(t *testing.T) {
|
||||
}{
|
||||
{"short", "ws-short"},
|
||||
{"exactly12ch", "ws-exactly12ch"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than-"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than-twelve-characters"},
|
||||
{"abc", "ws-abc"},
|
||||
// Real UUID: matches Docker container hostname used for A2A routing.
|
||||
{"ab078012-c305-42be-b93d-b6e8a78d5409", "ws-ab078012-c305-42be-b93d-b6e8a78d5409"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -377,7 +379,7 @@ func TestConfigVolumeName(t *testing.T) {
|
||||
}{
|
||||
{"short", "ws-short-configs"},
|
||||
{"exactly12ch", "ws-exactly12ch-configs"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than--configs"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than-twelve-characters-configs"},
|
||||
{"abc", "ws-abc-configs"},
|
||||
}
|
||||
|
||||
@@ -392,8 +394,8 @@ func TestConfigVolumeName(t *testing.T) {
|
||||
// ---------- #12 — claude-sessions volume naming ----------
|
||||
|
||||
// TestClaudeSessionVolumeName_Deterministic: same ID → same volume name, and
|
||||
// the name follows the ws-<id[:12]>-claude-sessions shape used everywhere
|
||||
// else in the provisioner.
|
||||
// the name follows the ws-<id>-claude-sessions shape used everywhere
|
||||
// else in the provisioner. Full UUIDs are used (no truncation).
|
||||
func TestClaudeSessionVolumeName_Deterministic(t *testing.T) {
|
||||
tests := []struct {
|
||||
id string
|
||||
@@ -401,7 +403,7 @@ func TestClaudeSessionVolumeName_Deterministic(t *testing.T) {
|
||||
}{
|
||||
{"short", "ws-short-claude-sessions"},
|
||||
{"exactly12ch", "ws-exactly12ch-claude-sessions"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than--claude-sessions"},
|
||||
{"longer-than-twelve-characters", "ws-longer-than-twelve-characters-claude-sessions"},
|
||||
{"abc", "ws-abc-claude-sessions"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
|
||||
@@ -137,39 +137,31 @@ func sweepRemovedRows(ctx context.Context, reaper OrphanReaper) {
|
||||
return
|
||||
}
|
||||
|
||||
// Resolve each prefix to a full workspace_id whose status is
|
||||
// 'removed'. The platform's workspace IDs are full UUIDs but
|
||||
// container names are truncated to 12 chars — an UPPER BOUND
|
||||
// of one match per prefix is guaranteed by the DB (UUID v4
|
||||
// collisions in the first 12 chars across active rows are
|
||||
// statistically negligible). Use a single IN-style query so
|
||||
// the cost is one round-trip regardless of leak count.
|
||||
// Resolve each container name to a full workspace_id whose status is
|
||||
// 'removed'. Container names now carry the full UUID (not truncated),
|
||||
// so we can use an exact-match IN-style query at zero additional cost.
|
||||
//
|
||||
// Defence: drop any prefix whose contents fall outside the
|
||||
// hex-and-dash UUID alphabet. Workspace IDs are UUIDs, so
|
||||
// container names follow ws-<12 hex chars>. Anything else is
|
||||
// either a non-workspace container that slipped past the
|
||||
// substring-match Docker filter (workspace-runner, etc.) or a
|
||||
// malformed entry — neither should be turned into a LIKE
|
||||
// pattern. Also blocks SQL LIKE wildcards (`_` and `%`) from
|
||||
// reaching the query, even though Docker's container-name
|
||||
// validation would already have rejected them upstream.
|
||||
likes := make([]string, 0, len(prefixes))
|
||||
// Defence: drop any ID whose contents fall outside the hex-and-dash
|
||||
// UUID alphabet. Workspace IDs are UUIDs, so container names follow
|
||||
// ws-<uuid>. Anything else is either a non-workspace container that
|
||||
// slipped past the Docker name filter (workspace-runner, etc.) or a
|
||||
// malformed entry — neither should reach the DB query.
|
||||
ids := make([]string, 0, len(prefixes))
|
||||
for _, p := range prefixes {
|
||||
if !isLikelyWorkspaceID(p) {
|
||||
continue
|
||||
}
|
||||
likes = append(likes, p+"%")
|
||||
ids = append(ids, p)
|
||||
}
|
||||
if len(likes) == 0 {
|
||||
if len(ids) == 0 {
|
||||
return
|
||||
}
|
||||
rows, err := db.DB.QueryContext(ctx, `
|
||||
SELECT id::text
|
||||
FROM workspaces
|
||||
WHERE status = 'removed'
|
||||
AND id::text LIKE ANY($1::text[])
|
||||
`, pq.Array(likes))
|
||||
AND id::text = ANY($1::text[])
|
||||
`, pq.Array(ids))
|
||||
if err != nil {
|
||||
log.Printf("Orphan sweeper: DB query failed: %v — skipping removed-row pass", err)
|
||||
return
|
||||
|
||||
@@ -83,25 +83,26 @@ func TestSweepOnce_ReconcilesRunningRemovedRows(t *testing.T) {
|
||||
// Docker reports two ws-* containers; one's row is 'removed'
|
||||
// (the leak), the other's is 'online' (the DB rightly excludes
|
||||
// it from the WHERE clause and we should NOT reap it).
|
||||
wsID := "abc123de-f456-1234-abcd-ef0123456789"
|
||||
wsIDOnline := "xyz789gh-i012-4321-fedc-ba0987654321"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: []string{"abc123def456", "xyz789ghi012"},
|
||||
listResponse: []string{wsID, wsIDOnline},
|
||||
}
|
||||
|
||||
// The query asks for status='removed' rows whose id matches the
|
||||
// LIKE patterns built from the running container prefixes. Mock
|
||||
// returns only the leaked one as a UUID-shaped full id.
|
||||
// The query uses exact-match = ANY($1) against full workspace IDs.
|
||||
// Mock returns only the leaked one.
|
||||
mock.ExpectQuery(`SELECT id::text\s+FROM workspaces`).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).
|
||||
AddRow("abc123def456-0000-0000-0000-000000000000"))
|
||||
AddRow(wsID))
|
||||
expectStaleTokenSweepNoOp(mock)
|
||||
|
||||
sweepOnce(context.Background(), reaper)
|
||||
|
||||
if len(reaper.stopCalls) != 1 || reaper.stopCalls[0] != "abc123def456-0000-0000-0000-000000000000" {
|
||||
t.Errorf("Stop calls = %v, want exactly the leaked id", reaper.stopCalls)
|
||||
if len(reaper.stopCalls) != 1 || reaper.stopCalls[0] != wsID {
|
||||
t.Errorf("Stop calls = %v, want exactly the leaked id %q", reaper.stopCalls, wsID)
|
||||
}
|
||||
if len(reaper.removeVolCalls) != 1 || reaper.removeVolCalls[0] != "abc123def456-0000-0000-0000-000000000000" {
|
||||
t.Errorf("RemoveVolume calls = %v, want exactly the leaked id", reaper.removeVolCalls)
|
||||
if len(reaper.removeVolCalls) != 1 || reaper.removeVolCalls[0] != wsID {
|
||||
t.Errorf("RemoveVolume calls = %v, want exactly the leaked id %q", reaper.removeVolCalls, wsID)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
@@ -160,15 +161,15 @@ func TestSweepOnce_StopFailureLeavesVolume(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
wsID := "abc123de-f456-1234-abcd-ef0123456789"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: []string{"abc123def456"},
|
||||
listResponse: []string{wsID},
|
||||
stopErr: map[string]error{
|
||||
"abc123def456-0000-0000-0000-000000000000": errors.New("docker daemon timeout"),
|
||||
wsID: errors.New("docker daemon timeout"),
|
||||
},
|
||||
}
|
||||
mock.ExpectQuery(`SELECT id::text\s+FROM workspaces`).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).
|
||||
AddRow("abc123def456-0000-0000-0000-000000000000"))
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(wsID))
|
||||
expectStaleTokenSweepNoOp(mock)
|
||||
|
||||
sweepOnce(context.Background(), reaper)
|
||||
@@ -188,16 +189,18 @@ func TestSweepOnce_VolumeRemoveErrorIsNonFatal(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
wsID1 := "aaa111bb-b222-0000-0000-000000000001"
|
||||
wsID2 := "ccc333dd-d444-0000-0000-000000000002"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: []string{"aaa111bbb222", "ccc333ddd444"},
|
||||
listResponse: []string{wsID1, wsID2},
|
||||
removeVolErr: map[string]error{
|
||||
"aaa111bbb222-0000-0000-0000-000000000000": errors.New("volume not found"),
|
||||
wsID1: errors.New("volume not found"),
|
||||
},
|
||||
}
|
||||
mock.ExpectQuery(`SELECT id::text\s+FROM workspaces`).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).
|
||||
AddRow("aaa111bbb222-0000-0000-0000-000000000000").
|
||||
AddRow("ccc333ddd444-0000-0000-0000-000000000000"))
|
||||
AddRow(wsID1).
|
||||
AddRow(wsID2))
|
||||
expectStaleTokenSweepNoOp(mock)
|
||||
|
||||
sweepOnce(context.Background(), reaper)
|
||||
@@ -215,7 +218,7 @@ func TestSweepOnce_VolumeRemoveErrorIsNonFatal(t *testing.T) {
|
||||
// through. The HasPrefix check in the provisioner trims those, but
|
||||
// the in-sweeper isLikelyWorkspaceID guard is the second line of
|
||||
// defence: anything outside the UUID alphabet (hex + dashes) is
|
||||
// rejected before being turned into a SQL LIKE pattern. Locks in
|
||||
// rejected before being included in the exact-match query. Locks in
|
||||
// that no DB query fires when every prefix is filtered out.
|
||||
func TestSweepOnce_FiltersNonWorkspacePrefixes(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
@@ -308,9 +311,11 @@ func TestSweepOnce_WipedDBReapsLabeledOrphans(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
wsID1 := "abc123de-f456-1234-abcd-ef0123456789"
|
||||
wsID2 := "ee001122-3344-0000-0000-000000000001"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: nil, // no name-filter matches in this scenario
|
||||
managedListResponse: []string{"abc123def456", "ee0011223344"},
|
||||
managedListResponse: []string{wsID1, wsID2},
|
||||
}
|
||||
|
||||
// First-pass query is skipped (listResponse is nil → early return
|
||||
@@ -325,7 +330,7 @@ func TestSweepOnce_WipedDBReapsLabeledOrphans(t *testing.T) {
|
||||
if len(reaper.stopCalls) != 2 {
|
||||
t.Fatalf("expected 2 Stop calls (both prefixes reaped), got %v", reaper.stopCalls)
|
||||
}
|
||||
wantStops := map[string]struct{}{"abc123def456": {}, "ee0011223344": {}}
|
||||
wantStops := map[string]struct{}{wsID1: {}, wsID2: {}}
|
||||
for _, c := range reaper.stopCalls {
|
||||
if _, ok := wantStops[c]; !ok {
|
||||
t.Errorf("unexpected Stop call: %q", c)
|
||||
@@ -347,17 +352,19 @@ func TestSweepOnce_WipedDBSkipsLabeledContainersWithRows(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
wsID1 := "abc123de-f456-1234-abcd-ef0123456789"
|
||||
wsID2 := "ee001122-3344-0000-0000-000000000001"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: nil,
|
||||
managedListResponse: []string{"abc123def456", "ee0011223344"},
|
||||
managedListResponse: []string{wsID1, wsID2},
|
||||
}
|
||||
|
||||
// The reverse-lookup returns both prefixes — both have rows in DB.
|
||||
// The reverse-lookup returns both IDs — both have rows in DB.
|
||||
// Sweeper should not reap either.
|
||||
mock.ExpectQuery(`SELECT lk\s+FROM unnest`).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"lk"}).
|
||||
AddRow("abc123def456%").
|
||||
AddRow("ee0011223344%"))
|
||||
AddRow(wsID1).
|
||||
AddRow(wsID2))
|
||||
expectStaleTokenSweepNoOp(mock)
|
||||
|
||||
sweepOnce(context.Background(), reaper)
|
||||
@@ -376,22 +383,22 @@ func TestSweepOnce_WipedDBReapsOnlyTheUnknownOnes(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
const keep = "abcdef012345"
|
||||
const reap = "fedcba543210"
|
||||
wsIDKeep := "abcdef01-2345-6789-abcd-ef0123456789"
|
||||
wsIDReap := "fedcba54-3210-fedc-ba98-76543210abcd"
|
||||
reaper := &fakeReaper{
|
||||
listResponse: nil,
|
||||
managedListResponse: []string{keep, reap},
|
||||
managedListResponse: []string{wsIDKeep, wsIDReap},
|
||||
}
|
||||
|
||||
mock.ExpectQuery(`SELECT lk\s+FROM unnest`).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"lk"}).
|
||||
AddRow(keep + "%"))
|
||||
AddRow(wsIDKeep))
|
||||
expectStaleTokenSweepNoOp(mock)
|
||||
|
||||
sweepOnce(context.Background(), reaper)
|
||||
|
||||
if len(reaper.stopCalls) != 1 || reaper.stopCalls[0] != reap {
|
||||
t.Errorf("expected 1 Stop call for %s, got %v", reap, reaper.stopCalls)
|
||||
if len(reaper.stopCalls) != 1 || reaper.stopCalls[0] != wsIDReap {
|
||||
t.Errorf("expected 1 Stop call for %s, got %v", wsIDReap, reaper.stopCalls)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
|
||||
Reference in New Issue
Block a user