mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-26 20:45:37 +02:00
fix(daemon): handle bidirectional .local drift and case drift in legacy merge
Review on #1220 flagged two gaps in the legacy-id migration candidate set: 1. Reverse .local: LegacyDaemonIDs only added the stripped variant when the current hostname ended in `.local`. The opposite direction — DB has `foo.local`, current host is `foo` — was missed, so runtimes registered under the `.local` variant stayed orphaned after upgrade. Now both variants (`foo` and `foo.local`) are always emitted, regardless of what `os.Hostname()` currently returns, plus their `-<profile>` suffix forms. 2. Case drift: os.Hostname() has been observed returning different casings on the same machine across mDNS/reboot state. A case-sensitive `=` comparison stranded rows like `Jiayuans-MacBook-Pro.local` when the daemon later reported `jiayuans-macbook-pro.local`. FindLegacyRuntimeByDaemonID now uses `LOWER(daemon_id) = LOWER(@daemon_id)` on both sides, so casing differences merge rather than orphan. The (workspace_id, provider) prefix still bounds the scan to a tiny set of rows so the non-indexed LOWER() comparison has negligible cost. Tests: TestLegacyDaemonIDs gets the mixed-case + reverse-direction cases; daemon_test.go adds TestDaemonRegister_MergesLegacyDaemonIDRuntime_ReverseDotLocal and TestDaemonRegister_MergesLegacyDaemonIDRuntime_CaseDrift.
This commit is contained in:
@@ -89,25 +89,29 @@ func EnsureDaemonID(profile string) (string, error) {
|
||||
// - pre-#1070: "<hostname>" (raw hostname, often ends in .local)
|
||||
// - current: "<hostname>" with .local drift depending on system state
|
||||
//
|
||||
// Because os.Hostname() may return either "foo" or "foo.local" depending on
|
||||
// system state at registration time, we always emit both variants as legacy
|
||||
// candidates so migration covers any prior registration.
|
||||
// .local drift is bidirectional — at different times os.Hostname() has
|
||||
// returned both "foo" and "foo.local" on the same machine (mDNS state,
|
||||
// system restart, login item order). So regardless of which form is current
|
||||
// now, we always emit BOTH the bare and .local-suffixed variants so migration
|
||||
// covers whichever form was persisted previously. Case drift is handled on
|
||||
// the server side via case-insensitive lookup, so we don't also emit cased
|
||||
// permutations here.
|
||||
func LegacyDaemonIDs(hostname, profile string) []string {
|
||||
host := strings.TrimSpace(hostname)
|
||||
if host == "" {
|
||||
return nil
|
||||
}
|
||||
stripped := strings.TrimSuffix(host, ".local")
|
||||
dotLocal := stripped + ".local"
|
||||
|
||||
candidates := []string{host}
|
||||
if stripped != host {
|
||||
candidates = append(candidates, stripped)
|
||||
}
|
||||
hostForms := []string{stripped, dotLocal}
|
||||
|
||||
candidates := make([]string, 0, len(hostForms)*2)
|
||||
candidates = append(candidates, hostForms...)
|
||||
if profile != "" {
|
||||
candidates = append(candidates,
|
||||
host+"-"+profile,
|
||||
stripped+"-"+profile,
|
||||
)
|
||||
for _, h := range hostForms {
|
||||
candidates = append(candidates, h+"-"+profile)
|
||||
}
|
||||
}
|
||||
|
||||
seen := make(map[string]struct{}, len(candidates))
|
||||
|
||||
@@ -96,30 +96,39 @@ func TestLegacyDaemonIDs(t *testing.T) {
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
// Bare hostname now — but the DB may still hold the previously
|
||||
// registered `.local` variant, so we must emit both.
|
||||
name: "plain hostname, no profile",
|
||||
hostname: "MacBook-Pro",
|
||||
want: []string{"MacBook-Pro"},
|
||||
want: []string{"MacBook-Pro", "MacBook-Pro.local"},
|
||||
},
|
||||
{
|
||||
// Dot-local hostname now — the stripped variant may be what the
|
||||
// DB holds from a prior registration where .local was absent.
|
||||
name: "dot-local hostname, no profile",
|
||||
hostname: "MacBook-Pro.local",
|
||||
want: []string{"MacBook-Pro.local", "MacBook-Pro"},
|
||||
want: []string{"MacBook-Pro", "MacBook-Pro.local"},
|
||||
},
|
||||
{
|
||||
name: "plain hostname with profile",
|
||||
hostname: "MacBook-Pro",
|
||||
profile: "staging",
|
||||
want: []string{"MacBook-Pro", "MacBook-Pro-staging"},
|
||||
want: []string{
|
||||
"MacBook-Pro",
|
||||
"MacBook-Pro.local",
|
||||
"MacBook-Pro-staging",
|
||||
"MacBook-Pro.local-staging",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "dot-local hostname with profile",
|
||||
hostname: "MacBook-Pro.local",
|
||||
profile: "staging",
|
||||
want: []string{
|
||||
"MacBook-Pro.local",
|
||||
"MacBook-Pro",
|
||||
"MacBook-Pro.local-staging",
|
||||
"MacBook-Pro.local",
|
||||
"MacBook-Pro-staging",
|
||||
"MacBook-Pro.local-staging",
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -127,6 +136,17 @@ func TestLegacyDaemonIDs(t *testing.T) {
|
||||
hostname: "",
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
// Case drift is handled on the server side (LOWER=LOWER match).
|
||||
// We still emit the hostname in its current casing here; the SQL
|
||||
// query normalizes both sides.
|
||||
name: "mixed case hostname preserved as-is",
|
||||
hostname: "Jiayuans-MacBook-Pro.local",
|
||||
want: []string{
|
||||
"Jiayuans-MacBook-Pro",
|
||||
"Jiayuans-MacBook-Pro.local",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
@@ -334,7 +334,7 @@ func (h *Handler) mergeLegacyRuntimes(r *http.Request, registered db.AgentRuntim
|
||||
old, err := h.Queries.FindLegacyRuntimeByDaemonID(r.Context(), db.FindLegacyRuntimeByDaemonIDParams{
|
||||
WorkspaceID: registered.WorkspaceID,
|
||||
Provider: provider,
|
||||
DaemonID: strToText(legacyID),
|
||||
DaemonID: legacyID,
|
||||
})
|
||||
if err != nil {
|
||||
// sql.ErrNoRows is the expected common case — nothing to migrate.
|
||||
|
||||
@@ -784,6 +784,130 @@ func TestDaemonRegister_MergesLegacyDaemonIDRuntime(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDaemonRegister_MergesLegacyDaemonIDRuntime_ReverseDotLocal covers the
|
||||
// direction missed by the initial implementation: the stored runtime row is
|
||||
// `host` (no `.local`) but the daemon's current `os.Hostname()` now returns
|
||||
// `host.local`. The daemon must emit the bare variant as a legacy candidate
|
||||
// and the server must match it.
|
||||
func TestDaemonRegister_MergesLegacyDaemonIDRuntime_ReverseDotLocal(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
const legacyDaemonID = "ReverseDotLocalHost" // stored without .local
|
||||
const emittedLegacyID = "ReverseDotLocalHost.local" // daemon now reports with .local
|
||||
const newDaemonID = "0192a7b0-0011-7ee9-9c21-30a5bcf86aa2"
|
||||
|
||||
var legacyRuntimeID string
|
||||
if err := testPool.QueryRow(ctx, `
|
||||
INSERT INTO agent_runtime (workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, owner_id, last_seen_at)
|
||||
VALUES ($1, $2, 'legacy-runtime-reverse', 'local', 'claude', 'offline', '', '{}'::jsonb, $3, now())
|
||||
RETURNING id
|
||||
`, testWorkspaceID, legacyDaemonID, testUserID).Scan(&legacyRuntimeID); err != nil {
|
||||
t.Fatalf("seed legacy runtime: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, legacyRuntimeID)
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := newRequest("POST", "/api/daemon/register", map[string]any{
|
||||
"workspace_id": testWorkspaceID,
|
||||
"daemon_id": newDaemonID,
|
||||
"legacy_daemon_ids": []string{"ReverseDotLocalHost", emittedLegacyID},
|
||||
"device_name": "ReverseDotLocalHost",
|
||||
"runtimes": []map[string]any{
|
||||
{"name": "reverse-runtime", "type": "claude", "version": "1.0.0", "status": "online"},
|
||||
},
|
||||
})
|
||||
testHandler.DaemonRegister(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("DaemonRegister: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var resp map[string]any
|
||||
json.NewDecoder(w.Body).Decode(&resp)
|
||||
newRuntimeID := resp["runtimes"].([]any)[0].(map[string]any)["id"].(string)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, newRuntimeID)
|
||||
})
|
||||
|
||||
var legacyCount int
|
||||
if err := testPool.QueryRow(ctx, `SELECT count(*) FROM agent_runtime WHERE id = $1`, legacyRuntimeID).Scan(&legacyCount); err != nil {
|
||||
t.Fatalf("count legacy runtime: %v", err)
|
||||
}
|
||||
if legacyCount != 0 {
|
||||
t.Fatalf("expected legacy row to be merged and deleted, still present")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDaemonRegister_MergesLegacyDaemonIDRuntime_CaseDrift verifies that
|
||||
// case-only drift in os.Hostname() output (e.g. `Jiayuans-MacBook-Pro.local`
|
||||
// vs `jiayuans-macbook-pro.local`) still merges the legacy row. The daemon
|
||||
// emits the id in its current casing; the server-side lookup uses LOWER() on
|
||||
// both sides so stored and emitted casings can differ without orphaning.
|
||||
func TestDaemonRegister_MergesLegacyDaemonIDRuntime_CaseDrift(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
const storedDaemonID = "Jiayuans-MacBook-Pro.local" // DB has original mixed case
|
||||
const emittedLegacyID = "jiayuans-macbook-pro.local" // Daemon now reports lowercased
|
||||
const newDaemonID = "0192a7b0-0022-7ee9-9c21-30a5bcf86aa3"
|
||||
|
||||
var legacyRuntimeID string
|
||||
if err := testPool.QueryRow(ctx, `
|
||||
INSERT INTO agent_runtime (workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, owner_id, last_seen_at)
|
||||
VALUES ($1, $2, 'legacy-runtime-case', 'local', 'claude', 'offline', '', '{}'::jsonb, $3, now())
|
||||
RETURNING id
|
||||
`, testWorkspaceID, storedDaemonID, testUserID).Scan(&legacyRuntimeID); err != nil {
|
||||
t.Fatalf("seed legacy runtime: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, legacyRuntimeID)
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := newRequest("POST", "/api/daemon/register", map[string]any{
|
||||
"workspace_id": testWorkspaceID,
|
||||
"daemon_id": newDaemonID,
|
||||
"legacy_daemon_ids": []string{emittedLegacyID},
|
||||
"device_name": "jiayuans-macbook-pro",
|
||||
"runtimes": []map[string]any{
|
||||
{"name": "case-drift-runtime", "type": "claude", "version": "1.0.0", "status": "online"},
|
||||
},
|
||||
})
|
||||
testHandler.DaemonRegister(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("DaemonRegister: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var resp map[string]any
|
||||
json.NewDecoder(w.Body).Decode(&resp)
|
||||
newRuntimeID := resp["runtimes"].([]any)[0].(map[string]any)["id"].(string)
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, newRuntimeID)
|
||||
})
|
||||
|
||||
var legacyCount int
|
||||
if err := testPool.QueryRow(ctx, `SELECT count(*) FROM agent_runtime WHERE id = $1`, legacyRuntimeID).Scan(&legacyCount); err != nil {
|
||||
t.Fatalf("count legacy runtime: %v", err)
|
||||
}
|
||||
if legacyCount != 0 {
|
||||
t.Fatalf("expected case-drift legacy row to be merged and deleted, still present")
|
||||
}
|
||||
|
||||
var legacyTrace *string
|
||||
if err := testPool.QueryRow(ctx, `SELECT legacy_daemon_id FROM agent_runtime WHERE id = $1`, newRuntimeID).Scan(&legacyTrace); err != nil {
|
||||
t.Fatalf("read legacy_daemon_id: %v", err)
|
||||
}
|
||||
if legacyTrace == nil || *legacyTrace != emittedLegacyID {
|
||||
t.Fatalf("expected legacy_daemon_id trace = %q, got %v", emittedLegacyID, legacyTrace)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDaemonRegister_LegacyIDNoMatchIsNoop guards the common case where the
|
||||
// daemon sends legacy candidates but no matching row exists (e.g. first
|
||||
// registration on a fresh machine). Registration must still succeed, the new
|
||||
|
||||
@@ -118,18 +118,24 @@ const findLegacyRuntimeByDaemonID = `-- name: FindLegacyRuntimeByDaemonID :one
|
||||
SELECT id, workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, last_seen_at, created_at, updated_at, owner_id, legacy_daemon_id FROM agent_runtime
|
||||
WHERE workspace_id = $1
|
||||
AND provider = $2
|
||||
AND daemon_id = $3
|
||||
AND LOWER(daemon_id) = LOWER($3)
|
||||
`
|
||||
|
||||
type FindLegacyRuntimeByDaemonIDParams struct {
|
||||
WorkspaceID pgtype.UUID `json:"workspace_id"`
|
||||
Provider string `json:"provider"`
|
||||
DaemonID pgtype.Text `json:"daemon_id"`
|
||||
DaemonID string `json:"daemon_id"`
|
||||
}
|
||||
|
||||
// Looks up a runtime row keyed on a prior (hostname-derived) daemon_id. Used
|
||||
// at register-time to find rows owned by the same machine under its old
|
||||
// identity so agents/tasks can be re-pointed at the new UUID-keyed row.
|
||||
//
|
||||
// Comparison is case-insensitive because os.Hostname() has been observed to
|
||||
// return different casings on the same machine (e.g. `Jiayuans-MacBook-Pro`
|
||||
// vs `jiayuans-macbook-pro`) across reboots/mDNS state changes. A case-
|
||||
// sensitive `=` would strand the old row; LOWER() on both sides handles drift
|
||||
// without forcing the daemon to enumerate cased permutations.
|
||||
func (q *Queries) FindLegacyRuntimeByDaemonID(ctx context.Context, arg FindLegacyRuntimeByDaemonIDParams) (AgentRuntime, error) {
|
||||
row := q.db.QueryRow(ctx, findLegacyRuntimeByDaemonID, arg.WorkspaceID, arg.Provider, arg.DaemonID)
|
||||
var i AgentRuntime
|
||||
|
||||
@@ -83,10 +83,16 @@ DELETE FROM agent WHERE runtime_id = $1 AND archived_at IS NOT NULL;
|
||||
-- Looks up a runtime row keyed on a prior (hostname-derived) daemon_id. Used
|
||||
-- at register-time to find rows owned by the same machine under its old
|
||||
-- identity so agents/tasks can be re-pointed at the new UUID-keyed row.
|
||||
--
|
||||
-- Comparison is case-insensitive because os.Hostname() has been observed to
|
||||
-- return different casings on the same machine (e.g. `Jiayuans-MacBook-Pro`
|
||||
-- vs `jiayuans-macbook-pro`) across reboots/mDNS state changes. A case-
|
||||
-- sensitive `=` would strand the old row; LOWER() on both sides handles drift
|
||||
-- without forcing the daemon to enumerate cased permutations.
|
||||
SELECT * FROM agent_runtime
|
||||
WHERE workspace_id = $1
|
||||
AND provider = $2
|
||||
AND daemon_id = $3;
|
||||
WHERE workspace_id = @workspace_id
|
||||
AND provider = @provider
|
||||
AND LOWER(daemon_id) = LOWER(@daemon_id);
|
||||
|
||||
-- name: ReassignAgentsToRuntime :execrows
|
||||
-- Re-points every agent referencing old_runtime_id at new_runtime_id.
|
||||
|
||||
Reference in New Issue
Block a user