diff --git a/cmd/gc/adoption_barrier.go b/cmd/gc/adoption_barrier.go index 62b5b8d8b8..3cc7da934c 100644 --- a/cmd/gc/adoption_barrier.go +++ b/cmd/gc/adoption_barrier.go @@ -133,8 +133,13 @@ func runAdoptionBarrier( // base template name (e.g., "city-worker-3" -> "worker"). cfgAgent, isConfigAgent := agentBySession[sessionName] isPoolInstance := false + staleSingletonSuffix := false if !isConfigAgent { - if base := resolvePoolBase(sessionName, store, cityName, st, agentByQN); base != nil { + if base := resolveCanonicalSingletonSuffixBase(sessionName, store, cityName, st, agentByQN); base != nil { + cfgAgent = base + isConfigAgent = true + staleSingletonSuffix = true + } else if base := resolvePoolBase(sessionName, store, cityName, st, agentByQN); base != nil { cfgAgent = base isConfigAgent = true isPoolInstance = true @@ -189,7 +194,11 @@ func runAdoptionBarrier( // instance expansion, to avoid false positives on direct session // names that end in numbers. slot := parsePoolSlot(sessionName) - if slot > 0 && isConfigAgent && cfgAgent.SupportsInstanceExpansion() { + switch { + case slot > 0 && staleSingletonSuffix: + fmt.Fprintf(stderr, "adoption barrier: adopting stale singleton suffix session %s as canonical agent %s without pool_slot metadata\n", //nolint:errcheck + sessionName, cfgAgent.QualifiedName()) + case slot > 0 && isConfigAgent && cfgAgent.SupportsInstanceExpansion(): detail.PoolSlot = slot meta["pool_slot"] = strconv.Itoa(slot) if maxSess := cfgAgent.EffectiveMaxActiveSessions(); maxSess != nil && *maxSess >= 0 && slot > *maxSess { @@ -197,7 +206,7 @@ func runAdoptionBarrier( fmt.Fprintf(stderr, "adoption barrier: %s pool slot %d exceeds max %d (adopt-then-drain)\n", //nolint:errcheck sessionName, slot, *maxSess) } - } else if slot > 0 && !isConfigAgent { + case slot > 0 && !isConfigAgent: // Defensive log (ga-fiw): a session ending in "-N" did not match // any configured agent — either by exact session name or by pool // base resolution. This is the orphan shape that produced the @@ -305,6 +314,25 @@ func resolvePoolBase(sessionName string, store beads.Store, cityName, sessionTem return nil } +func resolveCanonicalSingletonSuffixBase(sessionName string, store beads.Store, cityName, sessionTemplate string, agentByQN map[string]*config.Agent) *config.Agent { + slot := parsePoolSlot(sessionName) + if slot == 0 { + return nil + } + suffix := fmt.Sprintf("-%d", slot) + baseSessName := sessionName[:len(sessionName)-len(suffix)] + for _, a := range agentByQN { + if !a.UsesCanonicalSingletonPoolIdentity() { + continue + } + sn := lookupSessionNameOrLegacy(store, cityName, a.QualifiedName(), sessionTemplate) + if sn == baseSessName { + return a + } + } + return nil +} + // parsePoolSlot extracts the numeric pool slot from a session name suffix. // Returns 0 if no slot suffix is found. func parsePoolSlot(sessionName string) int { diff --git a/cmd/gc/adoption_barrier_test.go b/cmd/gc/adoption_barrier_test.go index b2cab572b8..4a12d13d86 100644 --- a/cmd/gc/adoption_barrier_test.go +++ b/cmd/gc/adoption_barrier_test.go @@ -730,19 +730,15 @@ func TestAdoptionBarrier_SingletonWithNumericSuffix(t *testing.T) { } } -// TestAdoptionBarrier_OrphanDashNSessionLogsWarning verifies the ga-fiw -// defensive log: when a running session ends in "-N" but no configured pool -// agent claims it (because the matching base agent has max_active_sessions=1 -// and SupportsInstanceExpansion()=false), the barrier still adopts the -// session but emits a stderr warning so the leak is traceable. -func TestAdoptionBarrier_OrphanDashNSessionLogsWarning(t *testing.T) { +func TestAdoptionBarrier_StaleDashNSingletonAdoptsCanonicalIdentity(t *testing.T) { store := beads.NewMemStore() // "refinery-1" looks like a pool instance but the base "refinery" agent - // has max_active_sessions=1, so resolvePoolBase rejects the suffix. + // has max_active_sessions=1, so it should be treated as stale singleton + // state rather than a live pool slot. sp := &fakeAdoptionProvider{running: []string{"refinery-1"}} cfg := &config.City{ Agents: []config.Agent{ - {Name: "refinery", MaxActiveSessions: intPtr(1)}, + {Name: "refinery", MaxActiveSessions: intPtr(1), ScaleCheck: "printf 1"}, }, } var stderr bytes.Buffer @@ -751,14 +747,22 @@ func TestAdoptionBarrier_OrphanDashNSessionLogsWarning(t *testing.T) { if result.Adopted != 1 { t.Errorf("Adopted = %d, want 1", result.Adopted) } - if !bytes.Contains(stderr.Bytes(), []byte("refinery-1 ends in -1")) { - t.Errorf("stderr missing orphan -N warning; got: %s", stderr.String()) + if !bytes.Contains(stderr.Bytes(), []byte("adopting stale singleton suffix session refinery-1")) { + t.Errorf("stderr missing stale singleton adoption warning; got: %s", stderr.String()) } - // Verify no pool_slot metadata (we explicitly decline to stamp it). beadList, _ := store.ListByLabel(sessionBeadLabel, 0) for _, b := range beadList { + if b.Metadata["agent_name"] != "refinery" { + t.Errorf("stale singleton agent_name = %q, want canonical refinery", b.Metadata["agent_name"]) + } + if !containsString(b.Labels, "agent:refinery") { + t.Errorf("stale singleton labels = %v, want canonical agent label", b.Labels) + } + if containsString(b.Labels, "agent:refinery-1") { + t.Errorf("stale singleton labels = %v, must not include phantom pool identity", b.Labels) + } if b.Metadata["pool_slot"] != "" { - t.Errorf("orphan -N session should not have pool_slot metadata, got %q", b.Metadata["pool_slot"]) + t.Errorf("stale singleton session should not have pool_slot metadata, got %q", b.Metadata["pool_slot"]) } } } diff --git a/cmd/gc/build_desired_state.go b/cmd/gc/build_desired_state.go index 1750d10c17..2d419a971b 100644 --- a/cmd/gc/build_desired_state.go +++ b/cmd/gc/build_desired_state.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "io" "log" @@ -372,23 +373,19 @@ func buildDesiredStateWithSessionBeads( // No store — use scale_check counts directly. scaleCheckCounts, _ = evaluatePendingPoolsMap(cfg, pendingPools, stderr, trace) for _, pw := range pendingPools { - desiredCount := scaleCheckCounts[cfg.Agents[pw.agentIdx].QualifiedName()] + cfgAgent := &cfg.Agents[pw.agentIdx] + desiredCount := scaleCheckCounts[cfgAgent.QualifiedName()] for slot := 1; slot <= desiredCount; slot++ { - name := cfg.Agents[pw.agentIdx].Name - if cfg.Agents[pw.agentIdx].SupportsInstanceExpansion() { - name = poolInstanceName(cfg.Agents[pw.agentIdx].Name, slot, &cfg.Agents[pw.agentIdx]) - } - qualifiedInstance := cfg.Agents[pw.agentIdx].QualifiedInstanceName(name) - instanceAgent := deepCopyAgent(&cfg.Agents[pw.agentIdx], name, cfg.Agents[pw.agentIdx].Dir) - fpExtra := buildFingerprintExtra(&instanceAgent) - tp, err := resolveTemplatePrepared(bp, &instanceAgent, qualifiedInstance, fpExtra) + instanceAgent, qualifiedInstance, poolSlot := poolDesiredRequestIdentity(cfgAgent, slot) + fpExtra := buildFingerprintExtra(instanceAgent) + tp, err := resolveTemplatePrepared(bp, instanceAgent, qualifiedInstance, fpExtra) if err != nil { fmt.Fprintf(stderr, "buildDesiredState: pool instance %q: %v (skipping)\n", qualifiedInstance, err) //nolint:errcheck continue } - tp.PoolSlot = slot + tp.PoolSlot = poolSlot setTemplateEnvIdentity(&tp, qualifiedInstance) - installAgentSideEffects(bp, &instanceAgent, tp, stderr) + installAgentSideEffects(bp, instanceAgent, tp, stderr) desired[tp.SessionName] = tp } } @@ -1284,6 +1281,10 @@ func discoverSessionBeadsWithRoots( continue } roots[template] = true + if !sessionAlreadyDesired && !isManualSessionBeadForAgent(b, cfgAgent) && !isNamedSessionBead(b) && + desiredHasCanonicalNonExpandingPoolSession(desired, template, cfgAgent) && staleNonExpandingPoolSessionBead(cfgAgent, b) { + continue + } if !isManualSessionBead(b) && !isNamedSessionBead(b) && !isPoolManagedSessionBead(b) && desiredHasConfiguredNamedTemplate(desired, template) { // A configured named session already owns this backing template in // desired state. Treat any extra plain open bead as leaked state so @@ -1309,6 +1310,11 @@ func discoverSessionBeadsWithRoots( if controllerManagedPool && isDrainedSessionBead(b) { continue } + if controllerManagedPool && !manualSession && !isNamedSessionBead(b) && + !sessionAlreadyDesired && cfgAgent.UsesCanonicalSingletonPoolIdentity() && + desiredHasCanonicalNonExpandingPoolSession(desired, template, cfgAgent) { + continue + } if controllerManagedPool && !manualSession && !isNamedSessionBead(b) && !sessionAlreadyDesired && !templateDesired && !scaleCheckPartial { continue @@ -1440,21 +1446,17 @@ func ensureDependencyOnlyTemplate( } if bp.beadStore == nil { - name := cfgAgent.Name - if cfgAgent.SupportsInstanceExpansion() { - name = poolInstanceName(cfgAgent.Name, 1, cfgAgent) - } - qualifiedInstance := cfgAgent.QualifiedInstanceName(name) - instanceAgent := deepCopyAgent(cfgAgent, name, cfgAgent.Dir) - fpExtra := buildFingerprintExtra(&instanceAgent) - tp, err := resolveTemplatePrepared(bp, &instanceAgent, qualifiedInstance, fpExtra) + resolveAgent, qualifiedInstance, poolSlot := poolDesiredRequestIdentity(cfgAgent, 1) + fpExtra := buildFingerprintExtra(resolveAgent) + tp, err := resolveTemplatePrepared(bp, resolveAgent, qualifiedInstance, fpExtra) if err != nil { fmt.Fprintf(stderr, "buildDesiredState: dependency floor %q: %v (skipping)\n", qualifiedInstance, err) //nolint:errcheck return } tp.DependencyOnly = true + tp.PoolSlot = poolSlot setTemplateEnvIdentity(&tp, qualifiedInstance) - installAgentSideEffects(bp, &instanceAgent, tp, stderr) + installAgentSideEffects(bp, resolveAgent, tp, stderr) desired[tp.SessionName] = tp return } @@ -1467,12 +1469,11 @@ func ensureDependencyOnlyTemplate( fmt.Fprintf(stderr, "buildDesiredState: dependency floor %q: %v (skipping)\n", qualifiedName, err) //nolint:errcheck return } - // Env/fingerprint resolution, on the other hand, must use the pool- - // instance identity so this store-backed path agrees with both the - // no-store dependency-floor path above and realizePoolDesiredSessions. - // Otherwise GC_ALIAS would be the base "rig/dog" here and "rig/dog-1" - // on the realize path, oscillating across ticks and triggering the - // reconciler's config-drift drain on the live dependency-floor session. + // Env/fingerprint resolution, on the other hand, must use the same + // canonical-or-instance identity as both the no-store dependency-floor + // path above and realizePoolDesiredSessions. Otherwise GC_ALIAS can + // oscillate across ticks and trigger the reconciler's config-drift drain + // on the live dependency-floor session. resolveAgent, resolveQN := canonicalSessionIdentityWithConfig(cfg, cfgAgent, sessionBead) // Dep-floor slot-1 fallback. The guard triggers when the helper returned // the BASE form — meaning no pool_slot was stamped yet. Keying off @@ -1483,7 +1484,7 @@ func ensureDependencyOnlyTemplate( // (dependency_only beads are never named), but the guard keeps intent // explicit so a future change that relaxes that filter can't silently // overwrite a named identity with "rig/-1". - if cfgAgent.SupportsInstanceExpansion() && resolveQN == cfgAgent.QualifiedName() && !isNamedSessionBead(sessionBead) { + if cfgAgent.SupportsInstanceExpansion() && !cfgAgent.UsesCanonicalSingletonPoolIdentity() && resolveQN == cfgAgent.QualifiedName() && !isNamedSessionBead(sessionBead) { // No pool_slot stamp yet on this freshly-created dep-floor bead. // Default to slot 1, mirroring the no-store path above. instanceName := poolInstanceName(cfgAgent.Name, 1, cfgAgent) @@ -1523,6 +1524,22 @@ func desiredHasConfiguredNamedTemplate(desired map[string]TemplateParams, templa return false } +func desiredHasCanonicalNonExpandingPoolSession(desired map[string]TemplateParams, template string, cfgAgent *config.Agent) bool { + if !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return false + } + canonical := cfgAgent.QualifiedName() + for _, existing := range desired { + if existing.TemplateName != template { + continue + } + if existing.DependencyOnly || existing.InstanceName == canonical || existing.Alias == canonical { + return true + } + } + return false +} + func realizePoolDesiredSessions( bp *agentBuildParams, cfgAgent *config.Agent, @@ -1561,23 +1578,59 @@ func realizePoolDesiredSessions( continue } used[sessionBead.ID] = true - instanceName, qualifiedInstance := poolInstanceIdentity(cfgAgent, slot, stderr) - instanceAgent := deepCopyAgent(cfgAgent, instanceName, cfgAgent.Dir) - fpExtra := buildFingerprintExtra(&instanceAgent) - tp, err := resolveTemplateForSessionBead(bp, &instanceAgent, qualifiedInstance, fpExtra, sessionBead) + manualSession := isManualSessionBeadForAgent(sessionBead, cfgAgent) + var ( + resolveAgent *config.Agent + qualifiedInstance string + poolSlot int + ) + if manualSession { + qualifiedInstance = sessionBeadQualifiedName(bp.cityPath, cfgAgent, bp.rigs, sessionBead) + resolveAgent = sessionBeadConfigAgent(cfgAgent, qualifiedInstance) + } else { + resolveAgent, qualifiedInstance, poolSlot = poolDesiredRequestIdentity(cfgAgent, slot) + } + fpExtra := buildFingerprintExtra(resolveAgent) + tp, err := resolveTemplateForSessionBead(bp, resolveAgent, qualifiedInstance, fpExtra, sessionBead) if err != nil { fmt.Fprintf(stderr, "buildDesiredState: pool %q session %s: %v (skipping)\n", qualifiedName, sessionBead.ID, err) //nolint:errcheck continue } - tp.Alias = qualifiedInstance - tp.InstanceName = qualifiedInstance - tp.PoolSlot = slot - setPoolTemplateRuntimeIdentity(&tp, qualifiedInstance, sessionBead) - installAgentSideEffects(bp, &instanceAgent, tp, stderr) + if manualSession { + tp.ManualSession = true + if manualAlias := strings.TrimSpace(sessionBead.Metadata["alias"]); manualAlias != "" { + tp.Alias = manualAlias + } + if qualifiedInstance != "" { + tp.InstanceName = qualifiedInstance + } else { + tp.InstanceName = tp.SessionName + } + // Manual sessions are user-owned, even when they still carry legacy + // pool_slot metadata from before singleton normalization. + tp.PoolSlot = 0 + } else { + tp.Alias = qualifiedInstance + tp.InstanceName = qualifiedInstance + tp.PoolSlot = poolSlot + setPoolTemplateRuntimeIdentity(&tp, qualifiedInstance, sessionBead) + } + installAgentSideEffects(bp, resolveAgent, tp, stderr) desired[tp.SessionName] = tp } } +func poolDesiredRequestIdentity(cfgAgent *config.Agent, slot int) (*config.Agent, string, int) { + qualifiedName := cfgAgent.QualifiedName() + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return cfgAgent, qualifiedName, 0 + } + instanceName := poolInstanceName(cfgAgent.Name, slot, cfgAgent) + qualifiedInstance := cfgAgent.QualifiedInstanceName(instanceName) + instanceAgent := deepCopyAgent(cfgAgent, instanceName, cfgAgent.Dir) + return &instanceAgent, qualifiedInstance, slot +} + // setPoolTemplateRuntimeIdentity stamps the pool alias unless this bead is in a // known deferred-alias state. Stable legacy pool beads can lack alias metadata; // those keep their historic instance identity until syncSessionBeads backfills. @@ -1614,6 +1667,156 @@ func poolRuntimeAliasIsDeferred(sessionBead beads.Bead) bool { return strings.TrimSpace(sessionBead.Metadata["state"]) == "creating" } +func normalizeNonExpandingPoolSessionBead( + bp *agentBuildParams, + cfgAgent *config.Agent, + sessionBead beads.Bead, +) (beads.Bead, error) { + // The store write is authoritative; callers must use the returned bead + // rather than re-reading bp.sessionBeads for this ID in the same tick. + // If alias acquisition collides, this helper records the deferred state; + // syncSessionBeads owns the retry once the canonical alias holder closes. + if bp == nil || bp.beadStore == nil || !cfgAgent.UsesCanonicalSingletonPoolIdentity() || isManualSessionBeadForAgent(sessionBead, cfgAgent) || isNamedSessionBead(sessionBead) || sessionBead.ID == "" { + return sessionBead, nil + } + canonical := cfgAgent.QualifiedName() + metadata := map[string]string{} + aliasNeedsUpdate := false + clearAliasConflictMetadata := func() { + queueClearPoolAliasConflictMetadata(metadata, sessionBead.Metadata) + } + alias := strings.TrimSpace(sessionBead.Metadata["alias"]) + deferredAlias := strings.TrimSpace(sessionBead.Metadata[poolAliasConflictMetadataKey]) + if nonExpandingPoolIdentitySlot(cfgAgent, sessionBeadAgentName(sessionBead)) > 0 && strings.TrimSpace(sessionBead.Metadata["agent_name"]) != canonical { + metadata["agent_name"] = canonical + } + if (nonExpandingPoolIdentitySlot(cfgAgent, alias) > 0 && alias != canonical) || (alias == "" && deferredAlias == canonical) { + for key, value := range session.UpdatedAliasMetadata(sessionBead.Metadata, canonical) { + metadata[key] = value + } + clearAliasConflictMetadata() + aliasNeedsUpdate = true + } + if alias == canonical { + clearAliasConflictMetadata() + } + if strings.TrimSpace(sessionBead.Metadata["pool_slot"]) != "" { + metadata["pool_slot"] = "" + } + + var title *string + if nonExpandingPoolIdentitySlot(cfgAgent, sessionBead.Title) > 0 && strings.TrimSpace(sessionBead.Title) != canonical { + normalizedTitle := canonical + title = &normalizedTitle + } + + removeLabels := make([]string, 0, len(sessionBead.Labels)) + hasCanonicalAgentLabel := containsString(sessionBead.Labels, "agent:"+canonical) + for _, label := range sessionBead.Labels { + label = strings.TrimSpace(label) + if strings.HasPrefix(label, "agent:") && nonExpandingPoolIdentitySlot(cfgAgent, strings.TrimPrefix(label, "agent:")) > 0 { + removeLabels = append(removeLabels, label) + } + } + var addLabels []string + if (len(metadata) > 0 || title != nil || len(removeLabels) > 0) && !hasCanonicalAgentLabel { + addLabels = []string{"agent:" + canonical} + } + if len(metadata) == 0 && title == nil && len(removeLabels) == 0 && len(addLabels) == 0 { + return sessionBead, nil + } + + apply := func() error { + return bp.beadStore.Update(sessionBead.ID, beads.UpdateOpts{ + Title: title, + Metadata: metadata, + Labels: addLabels, + RemoveLabels: removeLabels, + }) + } + if aliasNeedsUpdate { + if err := session.WithCitySessionAliasLock(bp.cityPath, canonical, func() error { + if err := session.EnsureAliasAvailableWithConfig(bp.beadStore, bp.city, canonical, sessionBead.ID); err != nil { + return err + } + return apply() + }); err != nil { + return sessionBead, fmt.Errorf("normalizing singleton pool identity for bead %s to %q: %w", sessionBead.ID, canonical, err) + } + } else if err := apply(); err != nil { + return sessionBead, fmt.Errorf("normalizing singleton pool identity for bead %s to %q: %w", sessionBead.ID, canonical, err) + } + + if bp.stderr != nil { + fmt.Fprintf(bp.stderr, "buildDesiredState: pool %q: collapsing phantom pool identity for bead %s to %q\n", canonical, sessionBead.ID, canonical) //nolint:errcheck + } + if len(metadata) > 0 && sessionBead.Metadata != nil { + sessionBead.Metadata = cloneStringMap(sessionBead.Metadata) + } + if sessionBead.Metadata == nil { + sessionBead.Metadata = map[string]string{} + } + for key, value := range metadata { + sessionBead.Metadata[key] = value + } + if title != nil { + sessionBead.Title = *title + } + if len(removeLabels) > 0 || len(addLabels) > 0 { + remove := make(map[string]bool, len(removeLabels)) + for _, label := range removeLabels { + remove[label] = true + } + filtered := make([]string, 0, len(sessionBead.Labels)+len(addLabels)) + for _, label := range sessionBead.Labels { + if !remove[label] { + filtered = append(filtered, label) + } + } + sessionBead.Labels = filtered + } + for _, label := range addLabels { + if !containsString(sessionBead.Labels, label) { + sessionBead.Labels = append(sessionBead.Labels, label) + } + } + return sessionBead, nil +} + +func staleNonExpandingPoolSessionBead(cfgAgent *config.Agent, sessionBead beads.Bead) bool { + if !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return false + } + if isManualSessionBeadForAgent(sessionBead, cfgAgent) { + return false + } + if nonExpandingPoolIdentitySlot(cfgAgent, sessionBeadAgentName(sessionBead)) > 0 { + return true + } + if nonExpandingPoolIdentitySlot(cfgAgent, sessionBead.Metadata["alias"]) > 0 { + return true + } + if nonExpandingPoolIdentitySlot(cfgAgent, sessionBead.Title) > 0 { + return true + } + for _, label := range sessionBead.Labels { + label = strings.TrimSpace(label) + if strings.HasPrefix(label, "agent:") && nonExpandingPoolIdentitySlot(cfgAgent, strings.TrimPrefix(label, "agent:")) > 0 { + return true + } + } + return strings.TrimSpace(sessionBead.Metadata["pool_slot"]) != "" +} + +func nonExpandingPoolIdentitySlot(cfgAgent *config.Agent, identity string) int { + if !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return 0 + } + // Accept any numeric -N suffix, not only configured pool bounds: these + // beads are stale singleton artifacts and may have been written externally. + return resolvePersistedPoolIdentitySlot(cfgAgent, true, identity) +} + func setTemplateEnvIdentity(tp *TemplateParams, identity string) { if tp == nil || identity == "" { return @@ -1676,17 +1879,15 @@ func canonicalSessionIdentityWithConfig(cfg *config.City, cfgAgent *config.Agent if isNamedSessionBead(bead) { return cfgAgent, cfgAgent.QualifiedName() } - if !cfgAgent.SupportsInstanceExpansion() { + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { return cfgAgent, cfgAgent.QualifiedName() } slot := existingPoolSlotWithConfig(cfg, cfgAgent, bead) if slot <= 0 { return cfgAgent, cfgAgent.QualifiedName() } - instanceName := poolInstanceName(cfgAgent.Name, slot, cfgAgent) - qualifiedInstance := cfgAgent.QualifiedInstanceName(instanceName) - instanceAgent := deepCopyAgent(cfgAgent, instanceName, cfgAgent.Dir) - return &instanceAgent, qualifiedInstance + instanceAgent, qualifiedInstance, _ := poolDesiredRequestIdentity(cfgAgent, slot) + return instanceAgent, qualifiedInstance } func sessionBeadQualifiedName(cityPath string, cfgAgent *config.Agent, rigs []config.Rig, sessionBead beads.Bead) string { @@ -1776,14 +1977,17 @@ func claimPoolSlotWithConfig(cfg *config.City, cfgAgent *config.Agent, sessionBe } func existingPoolSlot(cfgAgent *config.Agent, sessionBead beads.Bead) int { + if cfgAgent == nil { + return 0 + } + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return 0 + } if sessionBead.Metadata["pool_slot"] != "" { if slot, err := strconv.Atoi(strings.TrimSpace(sessionBead.Metadata["pool_slot"])); err == nil && slot > 0 { return slot } } - if cfgAgent == nil { - return 0 - } if slot := resolvePersistedPoolIdentitySlot(cfgAgent, true, sessionBeadAgentName(sessionBead), sessionBead.Metadata["alias"]); slot > 0 { return slot } @@ -1873,6 +2077,9 @@ func existingPoolSlotWithConfig(cfg *config.City, cfgAgent *config.Agent, sessio if cfgAgent == nil { return 0 } + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return 0 + } storedTemplateMatches := cfg == nil || storedTemplateMatchesPoolTemplate(sessionBeadStoredTemplate(sessionBead), cfgAgent.QualifiedName(), cfg) agentSlot := resolvePersistedPoolIdentitySlot(cfgAgent, storedTemplateMatches, sessionBeadAgentName(sessionBead)) aliasSlot := resolvePersistedPoolIdentitySlot(cfgAgent, storedTemplateMatches, sessionBead.Metadata["alias"]) @@ -1956,63 +2163,207 @@ func selectOrCreatePoolSessionBead( } // Resume tier: reuse the session that has in-progress work assigned. if preferred != nil && preferred.ID != "" && !used[preferred.ID] && !isFailedCreateSessionBead(*preferred) { - slot := claimPoolSlotWithConfig(bp.city, cfgAgent, *preferred, usedSlots) - if slot == 0 { + slot := claimDesiredPoolSlot(bp.city, cfgAgent, *preferred, usedSlots) + if slot == 0 && !cfgAgent.UsesCanonicalSingletonPoolIdentity() { return beads.Bead{}, 0, fmt.Errorf("pool session %s concrete slot already claimed", preferred.ID) } - return *preferred, slot, nil + if isManualSessionBeadForAgent(*preferred, cfgAgent) { + return *preferred, slot, nil + } + bead, err := normalizeNonExpandingPoolSessionBeadForSelection(bp, cfgAgent, *preferred) + return bead, slot, err + } + if canonical, ok := findReusableCanonicalNonExpandingPoolSessionBead(bp, cfgAgent, template, used); ok { + slot := claimDesiredPoolSlot(bp.city, cfgAgent, canonical, usedSlots) + bead, err := normalizeNonExpandingPoolSessionBeadForSelection(bp, cfgAgent, canonical) + return bead, slot, err } // Reuse an existing active/creating session bead. Skip drained, closed, // and asleep — asleep ephemerals are not restarted; a fresh session is // created instead. The reconciler closes orphaned asleep beads. - for _, bead := range bp.sessionBeads.Open() { - if bead.Status == "closed" { - continue - } - if isDrainedSessionBead(bead) { - continue - } - if isFailedCreateSessionBead(bead) { - continue - } - if bead.Metadata["state"] == "asleep" { - continue + for _, bead := range reusablePoolSessionBeads(bp, cfgAgent, template, used) { + if desiredName := strings.TrimSpace(bead.Metadata["session_name"]); desiredName != "" { + slot := claimDesiredPoolSlot(bp.city, cfgAgent, bead, usedSlots) + if slot == 0 && !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + continue + } + bead, err := normalizeNonExpandingPoolSessionBeadForSelection(bp, cfgAgent, bead) + return bead, slot, err } - if isManualSessionBeadForAgent(bead, cfgAgent) { - continue + } + slot := claimDesiredPoolSlot(bp.city, cfgAgent, beads.Bead{}, usedSlots) + _, qualifiedInstance, poolSlot := poolDesiredRequestIdentity(cfgAgent, slot) + bead, err := createPoolSessionBeadWithGuardedAlias(bp, cfgAgent, template, qualifiedInstance, slot) + if err != nil { + delete(usedSlots, slot) + return bead, 0, err + } + return bead, poolSlot, nil +} + +func claimDesiredPoolSlot(cfg *config.City, cfgAgent *config.Agent, sessionBead beads.Bead, used map[int]bool) int { + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return 0 + } + return claimPoolSlotWithConfig(cfg, cfgAgent, sessionBead, used) +} + +func reusablePoolSessionBead(bp *agentBuildParams, cfgAgent *config.Agent, template string, bead beads.Bead, used map[string]bool) bool { + if bp == nil { + return false + } + if bead.Status == "closed" { + return false + } + if isDrainedSessionBead(bead) { + return false + } + if isFailedCreateSessionBead(bead) { + return false + } + if bead.Metadata["state"] == "asleep" { + return false + } + if isManualSessionBeadForAgent(bead, cfgAgent) { + return false + } + if isNamedSessionBead(bead) { + return false + } + if sessionBeadHasAssignedWork(bp.assignedWorkBeads, bead) { + return false + } + if used != nil && used[bead.ID] { + return false + } + return resolvedSessionTemplate(bead, reuseTemplateConfig(bp)) == template +} + +func reusablePoolSessionBeads(bp *agentBuildParams, cfgAgent *config.Agent, template string, used map[string]bool) []beads.Bead { + if bp == nil || bp.sessionBeads == nil { + return nil + } + candidates := []beads.Bead{} + for _, bead := range bp.sessionBeads.Open() { + if reusablePoolSessionBead(bp, cfgAgent, template, bead, used) { + candidates = append(candidates, bead) } - if isNamedSessionBead(bead) { - continue + } + sortSessionBeadsByCreatedAtThenID(candidates) + return candidates +} + +func sortSessionBeadsByCreatedAtThenID(candidates []beads.Bead) { + sort.SliceStable(candidates, func(i, j int) bool { + if !candidates[i].CreatedAt.Equal(candidates[j].CreatedAt) { + return candidates[i].CreatedAt.Before(candidates[j].CreatedAt) } - if sessionBeadHasAssignedWork(bp.assignedWorkBeads, bead) { + return candidates[i].ID < candidates[j].ID + }) +} + +func findReusableCanonicalNonExpandingPoolSessionBead( + bp *agentBuildParams, + cfgAgent *config.Agent, + template string, + used map[string]bool, +) (beads.Bead, bool) { + if bp == nil || bp.sessionBeads == nil || !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return beads.Bead{}, false + } + canonical := cfgAgent.QualifiedName() + for _, bead := range reusablePoolSessionBeads(bp, cfgAgent, template, used) { + if strings.TrimSpace(bead.Metadata["session_name"]) == "" { continue } - if used[bead.ID] { + if staleNonExpandingPoolSessionBead(cfgAgent, bead) { continue } - if normalizedSessionTemplate(bead, &config.City{Agents: bp.agents}) != template { - continue + if beadIdentifiesAsCanonical(bead, canonical) { + return bead, true } - if desiredName := strings.TrimSpace(bead.Metadata["session_name"]); desiredName != "" { - slot := claimPoolSlotWithConfig(bp.city, cfgAgent, bead, usedSlots) - if slot == 0 { - continue - } - return bead, slot, nil + } + return beads.Bead{}, false +} + +func beadIdentifiesAsCanonical(bead beads.Bead, canonical string) bool { + canonical = strings.TrimSpace(canonical) + if canonical == "" { + return false + } + return strings.TrimSpace(bead.Metadata["agent_name"]) == canonical || + strings.TrimSpace(bead.Metadata["alias"]) == canonical || + strings.TrimSpace(bead.Title) == canonical || + containsString(bead.Labels, "agent:"+canonical) +} + +func normalizeNonExpandingPoolSessionBeadForSelection( + bp *agentBuildParams, + cfgAgent *config.Agent, + sessionBead beads.Bead, +) (beads.Bead, error) { + bead, err := normalizeNonExpandingPoolSessionBead(bp, cfgAgent, sessionBead) + if err == nil { + return bead, nil + } + if !cfgAgent.UsesCanonicalSingletonPoolIdentity() || !errors.Is(err, session.ErrSessionAliasExists) { + return bead, err + } + if bp != nil && bp.stderr != nil { + fmt.Fprintf(bp.stderr, "buildDesiredState: pool %q: deferring singleton pool identity normalization for bead %s: %v\n", cfgAgent.QualifiedName(), sessionBead.ID, err) //nolint:errcheck + } + return recordDeferredNonExpandingPoolAliasConflict(bp, cfgAgent, sessionBead) +} + +func recordDeferredNonExpandingPoolAliasConflict( + bp *agentBuildParams, + cfgAgent *config.Agent, + sessionBead beads.Bead, +) (beads.Bead, error) { + // The store write is authoritative; callers must use the returned bead + // rather than re-reading bp.sessionBeads for this ID in the same tick. + canonical := cfgAgent.QualifiedName() + count := 0 + if existing, err := strconv.Atoi(strings.TrimSpace(sessionBead.Metadata[poolAliasConflictCountMetadataKey])); err == nil && existing > 0 { + count = existing + } + metadata := session.UpdatedAliasMetadata(sessionBead.Metadata, "") + metadata[poolAliasConflictMetadataKey] = canonical + metadata[poolAliasConflictCountMetadataKey] = strconv.Itoa(count + 1) + metadata[poolAliasConflictAtMetadataKey] = time.Now().UTC().Format(time.RFC3339) + if bp != nil && bp.beadStore != nil && sessionBead.ID != "" { + if err := bp.beadStore.Update(sessionBead.ID, beads.UpdateOpts{Metadata: metadata}); err != nil { + return sessionBead, fmt.Errorf("recording deferred singleton pool alias conflict for bead %s: %w", sessionBead.ID, err) } } - slot := claimPoolSlotWithConfig(bp.city, cfgAgent, beads.Bead{}, usedSlots) - _, qualifiedInstance := poolInstanceIdentity(cfgAgent, slot, bp.stderr) - bead, err := createPoolSessionBeadWithGuardedAlias(bp, template, qualifiedInstance, slot) - if err != nil { - delete(usedSlots, slot) - return bead, 0, err + sessionBead.Metadata = cloneStringMap(sessionBead.Metadata) + if sessionBead.Metadata == nil { + sessionBead.Metadata = map[string]string{} + } + for key, value := range metadata { + sessionBead.Metadata[key] = value + } + return sessionBead, nil +} + +func queueClearPoolAliasConflictMetadata(metadata, existing map[string]string) { + if existing == nil { + return + } + for _, key := range []string{ + poolAliasConflictMetadataKey, + poolAliasConflictCountMetadataKey, + poolAliasConflictAtMetadataKey, + } { + if existing[key] != "" { + metadata[key] = "" + } } - return bead, slot, nil } func createPoolSessionBeadWithGuardedAlias( bp *agentBuildParams, + cfgAgent *config.Agent, template string, qualifiedInstance string, slot int, @@ -2020,6 +2371,9 @@ func createPoolSessionBeadWithGuardedAlias( if bp == nil { return beads.Bead{}, fmt.Errorf("creating pool session for %q: build params unavailable", template) } + if err := validateAgentSessionTransportForBuild(bp, cfgAgent, qualifiedInstance); err != nil { + return beads.Bead{}, err + } identity := poolSessionCreateIdentity{ AgentName: qualifiedInstance, Slot: slot, @@ -2075,37 +2429,89 @@ func selectOrCreateDependencyPoolSessionBead( cfgAgent *config.Agent, template string, ) (beads.Bead, error) { - for _, bead := range bp.sessionBeads.Open() { - if bead.Status == "closed" || isManualSessionBead(bead) { - continue - } - if isDrainedSessionBead(bead) { - continue - } - if isFailedCreateSessionBead(bead) { - continue - } - if isNamedSessionBead(bead) { - continue - } - if bead.Metadata["dependency_only"] != boolMetadata(true) { - continue - } - if normalizedSessionTemplate(bead, &config.City{Agents: bp.agents}) != template { - continue - } - if desiredName := strings.TrimSpace(bead.Metadata["session_name"]); desiredName != "" { - return bead, nil - } - } if cfgAgent == nil { cfgAgent = findAgentByTemplate(&config.City{Agents: bp.agents}, template) } if cfgAgent == nil { return beads.Bead{}, fmt.Errorf("dependency pool template %q has no configured agent", template) } - _, qualifiedInstance := poolInstanceIdentity(cfgAgent, 1, bp.stderr) - return createPoolSessionBeadWithGuardedAlias(bp, template, qualifiedInstance, 1) + if canonical, ok := findReusableCanonicalNonExpandingDependencyPoolSessionBead(bp, cfgAgent, template); ok { + return normalizeNonExpandingPoolSessionBeadForSelection(bp, cfgAgent, canonical) + } + for _, bead := range reusableDependencyPoolSessionBeads(bp, template) { + return normalizeNonExpandingPoolSessionBeadForSelection(bp, cfgAgent, bead) + } + _, qualifiedInstance, poolSlot := poolDesiredRequestIdentity(cfgAgent, 1) + return createPoolSessionBeadWithGuardedAlias(bp, cfgAgent, template, qualifiedInstance, poolSlot) +} + +func reusableDependencyPoolSessionBeads(bp *agentBuildParams, template string) []beads.Bead { + if bp == nil || bp.sessionBeads == nil { + return nil + } + candidates := []beads.Bead{} + for _, bead := range bp.sessionBeads.Open() { + if reusableDependencyPoolSessionBead(bp, template, bead) { + candidates = append(candidates, bead) + } + } + sortSessionBeadsByCreatedAtThenID(candidates) + return candidates +} + +func reusableDependencyPoolSessionBead(bp *agentBuildParams, template string, bead beads.Bead) bool { + if bp == nil { + return false + } + if bead.Status == "closed" || isManualSessionBead(bead) { + return false + } + if isDrainedSessionBead(bead) { + return false + } + if isFailedCreateSessionBead(bead) { + return false + } + if isNamedSessionBead(bead) { + return false + } + if bead.Metadata["dependency_only"] != boolMetadata(true) { + return false + } + if resolvedSessionTemplate(bead, reuseTemplateConfig(bp)) != template { + return false + } + return strings.TrimSpace(bead.Metadata["session_name"]) != "" +} + +func reuseTemplateConfig(bp *agentBuildParams) *config.City { + if bp == nil { + return nil + } + if bp.city != nil { + return bp.city + } + return &config.City{Agents: bp.agents} +} + +func findReusableCanonicalNonExpandingDependencyPoolSessionBead( + bp *agentBuildParams, + cfgAgent *config.Agent, + template string, +) (beads.Bead, bool) { + if bp == nil || bp.sessionBeads == nil || !cfgAgent.UsesCanonicalSingletonPoolIdentity() { + return beads.Bead{}, false + } + canonical := cfgAgent.QualifiedName() + for _, bead := range reusableDependencyPoolSessionBeads(bp, template) { + if staleNonExpandingPoolSessionBead(cfgAgent, bead) { + continue + } + if beadIdentifiesAsCanonical(bead, canonical) { + return bead, true + } + } + return beads.Bead{}, false } func poolSessionCreateStartedAt(_ *agentBuildParams) time.Time { @@ -2198,7 +2604,17 @@ func validateAgentSessionTransportForBuild(bp *agentBuildParams, cfgAgent *confi if bp == nil || cfgAgent == nil { return nil } - resolved, err := config.ResolveProvider(cfgAgent, bp.workspace, bp.providers, bp.lookPath) + if bp.lookPath == nil { + // Legacy unit tests construct minimal build params without provider + // lookup plumbing. Production controller paths always install lookPath; + // coverage below exercises that production-shaped validation path. + return nil + } + workspace := bp.workspace + if workspace == nil { + workspace = &config.Workspace{} + } + resolved, err := config.ResolveProvider(cfgAgent, workspace, bp.providers, bp.lookPath) if err != nil { return fmt.Errorf("agent %q: %w", qualifiedName, err) } @@ -2298,6 +2714,13 @@ func poolInstanceIdentity(cfgAgent *config.Agent, slot int, stderr io.Writer) (s } return cfgAgent.Name, cfgAgent.QualifiedName() } + if cfgAgent.UsesCanonicalSingletonPoolIdentity() { + if slot > 0 && stderr != nil { + fmt.Fprintf(stderr, "buildDesiredState: pool %q: agent uses canonical singleton identity (max_active_sessions=%s) but slot %d was claimed; using base identity to avoid phantom %q-%d session\n", //nolint:errcheck + cfgAgent.QualifiedName(), formatMaxSessions(cfgAgent), slot, cfgAgent.Name, slot) + } + return cfgAgent.Name, cfgAgent.QualifiedName() + } instanceName := poolInstanceName(cfgAgent.Name, slot, cfgAgent) return instanceName, cfgAgent.QualifiedInstanceName(instanceName) } diff --git a/cmd/gc/build_desired_state_test.go b/cmd/gc/build_desired_state_test.go index 12d974cc07..2c8f5936db 100644 --- a/cmd/gc/build_desired_state_test.go +++ b/cmd/gc/build_desired_state_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "errors" "fmt" "io" @@ -9,6 +10,7 @@ import ( "os" "path" "path/filepath" + "reflect" "strconv" "strings" "sync" @@ -19,6 +21,7 @@ import ( "github.com/gastownhall/gascity/internal/beads/contract" "github.com/gastownhall/gascity/internal/clock" "github.com/gastownhall/gascity/internal/config" + "github.com/gastownhall/gascity/internal/events" "github.com/gastownhall/gascity/internal/fsys" "github.com/gastownhall/gascity/internal/runtime" sessionpkg "github.com/gastownhall/gascity/internal/session" @@ -1643,6 +1646,1389 @@ func TestBuildDesiredState_NewPoolSessionBeadCreatedWithConcreteIdentity(t *test } } +func TestBuildDesiredState_MaxOneAgentDemandUsesCanonicalIdentity(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, io.Discard) + if len(dsResult.State) != 1 { + t.Fatalf("desired sessions = %d, want 1", len(dsResult.State)) + } + var tp TemplateParams + for _, candidate := range dsResult.State { + tp = candidate + } + if tp.InstanceName != "cashmaster/refinery" { + t.Fatalf("InstanceName = %q, want canonical non-pool identity", tp.InstanceName) + } + if tp.Alias != "cashmaster/refinery" { + t.Fatalf("Alias = %q, want canonical non-pool identity", tp.Alias) + } + if tp.PoolSlot != 0 { + t.Fatalf("PoolSlot = %d, want 0 for max_active_sessions=1", tp.PoolSlot) + } + + sessionBeads, err := loadSessionBeads(store) + if err != nil { + t.Fatalf("load session beads: %v", err) + } + if len(sessionBeads) != 1 { + t.Fatalf("session beads = %d, want 1", len(sessionBeads)) + } + got := sessionBeads[0] + if got.Metadata["agent_name"] != "cashmaster/refinery" { + t.Fatalf("agent_name = %q, want canonical non-pool identity", got.Metadata["agent_name"]) + } + if got.Metadata["alias"] != "cashmaster/refinery" { + t.Fatalf("alias = %q, want canonical non-pool identity", got.Metadata["alias"]) + } + if got.Metadata["pool_slot"] != "" { + t.Fatalf("pool_slot = %q, want empty for max_active_sessions=1", got.Metadata["pool_slot"]) + } + if got.Title != "cashmaster/refinery" { + t.Fatalf("title = %q, want canonical non-pool identity", got.Title) + } + if containsString(got.Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("labels = %#v, must not include phantom pool identity", got.Labels) + } + if !containsString(got.Labels, "agent:cashmaster/refinery") { + t.Fatalf("labels = %#v, want canonical agent label", got.Labels) + } +} + +func TestBuildDesiredState_NoStoreMaxOneAgentDemandUsesCanonicalSlotZero(t *testing.T) { + cityPath := t.TempDir() + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), nil, io.Discard) + if len(dsResult.State) != 1 { + t.Fatalf("desired sessions = %d, want 1", len(dsResult.State)) + } + var tp TemplateParams + for _, candidate := range dsResult.State { + tp = candidate + } + if tp.InstanceName != "cashmaster/refinery" { + t.Fatalf("InstanceName = %q, want canonical non-pool identity", tp.InstanceName) + } + if tp.Alias != "cashmaster/refinery" { + t.Fatalf("Alias = %q, want canonical non-pool identity", tp.Alias) + } + if tp.PoolSlot != 0 { + t.Fatalf("PoolSlot = %d, want 0 for no-store max_active_sessions=1", tp.PoolSlot) + } +} + +func TestSyncSessionBeads_DoesNotBackfillPoolSlotForCanonicalMaxOneDemand(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, io.Discard) + var stderr bytes.Buffer + syncSessionBeads( + cityPath, + store, + dsResult.State, + runtime.NewFake(), + allConfiguredDS(dsResult.State), + cfg, + &clock.Fake{Time: time.Date(2026, 5, 15, 10, 0, 0, 0, time.UTC)}, + &stderr, + false, + ) + + sessionBeads, err := loadSessionBeads(store) + if err != nil { + t.Fatalf("load session beads: %v", err) + } + if len(sessionBeads) != 1 { + t.Fatalf("session beads = %d, want 1", len(sessionBeads)) + } + got := sessionBeads[0] + if got.Metadata["agent_name"] != "cashmaster/refinery" { + t.Fatalf("agent_name = %q, want canonical singleton identity; sync stderr=%q", got.Metadata["agent_name"], stderr.String()) + } + if got.Metadata["pool_slot"] != "" { + t.Fatalf("pool_slot = %q, want empty after build plus sync for canonical singleton", got.Metadata["pool_slot"]) + } + if got.Metadata["alias"] != "cashmaster/refinery" { + t.Fatalf("alias = %q, want canonical singleton identity", got.Metadata["alias"]) + } +} + +func TestBuildDesiredState_MaxOneAgentNormalizesStalePoolIdentityBead(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + var stderr bytes.Buffer + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + if len(dsResult.State) != 1 { + t.Fatalf("desired sessions = %d, want 1", len(dsResult.State)) + } + var tp TemplateParams + for _, candidate := range dsResult.State { + tp = candidate + } + if tp.InstanceName != "cashmaster/refinery" { + t.Fatalf("InstanceName = %q, want canonical non-pool identity", tp.InstanceName) + } + if tp.Alias != "cashmaster/refinery" { + t.Fatalf("Alias = %q, want canonical non-pool identity", tp.Alias) + } + if tp.PoolSlot != 0 { + t.Fatalf("PoolSlot = %d, want 0 for normalized max_active_sessions=1 bead", tp.PoolSlot) + } + + got, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got.Metadata["agent_name"] != "cashmaster/refinery" { + t.Fatalf("agent_name = %q, want canonical non-pool identity", got.Metadata["agent_name"]) + } + if got.Metadata["alias"] != "cashmaster/refinery" { + t.Fatalf("alias = %q, want canonical non-pool identity", got.Metadata["alias"]) + } + if got.Metadata["pool_slot"] != "" { + t.Fatalf("pool_slot = %q, want empty after singleton normalization", got.Metadata["pool_slot"]) + } + if got.Title != "cashmaster/refinery" { + t.Fatalf("title = %q, want canonical non-pool identity", got.Title) + } + if containsString(got.Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("labels = %#v, must not include phantom pool identity", got.Labels) + } + if !containsString(got.Labels, "agent:cashmaster/refinery") { + t.Fatalf("labels = %#v, want canonical agent label", got.Labels) + } + if !strings.Contains(stderr.String(), "collapsing phantom pool identity") { + t.Fatalf("stderr = %q, want scoped phantom identity diagnostic", stderr.String()) + } +} + +func TestBuildDesiredState_MaxOneAgentPrefersCanonicalWhenStaleDuplicateExists(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + var stderr bytes.Buffer + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + + if strings.Contains(stderr.String(), "(skipping)") { + t.Fatalf("stderr = %q, want stale duplicate to remain recoverable", stderr.String()) + } + if _, ok := dsResult.State[stale.Metadata["session_name"]]; ok { + t.Fatalf("desired state includes stale duplicate %q; keys=%v", stale.Metadata["session_name"], mapKeys(dsResult.State)) + } + tp, ok := dsResult.State[canonical.Metadata["session_name"]] + if !ok { + t.Fatalf("desired state missing canonical singleton %q; keys=%v", canonical.Metadata["session_name"], mapKeys(dsResult.State)) + } + if tp.InstanceName != "cashmaster/refinery" { + t.Fatalf("InstanceName = %q, want canonical singleton identity", tp.InstanceName) + } + if tp.PoolSlot != 0 { + t.Fatalf("PoolSlot = %d, want 0 for max_active_sessions=1", tp.PoolSlot) + } +} + +func TestBuildDesiredState_MaxOneAgentPreservesManualStaleIdentityBesideCanonicalDemand(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + manual, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-manual", + "state": "awake", + "session_origin": "manual", + "manual_session": "true", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, io.Discard) + + manualTP, ok := dsResult.State[manual.Metadata["session_name"]] + if !ok { + t.Fatalf("desired state missing manual stale singleton %q; keys=%v", manual.Metadata["session_name"], mapKeys(dsResult.State)) + } + if !manualTP.ManualSession { + t.Fatalf("manual stale singleton ManualSession = false, want true") + } + if manualTP.InstanceName != "cashmaster/refinery-1" { + t.Fatalf("manual stale singleton InstanceName = %q, want preserved identity", manualTP.InstanceName) + } + if manualTP.Alias != "cashmaster/refinery-1" { + t.Fatalf("manual stale singleton Alias = %q, want preserved identity", manualTP.Alias) + } + if len(dsResult.State) != 2 { + t.Fatalf("desired sessions = %d, want manual stale session beside canonical demand; keys=%v", len(dsResult.State), mapKeys(dsResult.State)) + } + canonicalFound := false + for sessionName, tp := range dsResult.State { + if sessionName == manual.Metadata["session_name"] { + continue + } + if tp.InstanceName == "cashmaster/refinery" && tp.Alias == "cashmaster/refinery" { + canonicalFound = true + } + } + if !canonicalFound { + t.Fatalf("desired state missing canonical singleton demand beside manual stale session; state=%#v", dsResult.State) + } +} + +func TestBuildDesiredState_MaxOneManualAssignedWorkPreservesManualIdentity(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + manual, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-manual", + "state": "awake", + "session_origin": "manual", + "manual_session": "true", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + priority := 10 + if _, err := store.Create(beads.Bead{ + Title: "manual assigned work", + Type: "task", + Status: "in_progress", + Priority: &priority, + Assignee: "cashmaster/refinery-1", + Metadata: map[string]string{ + "gc.routed_to": "cashmaster/refinery", + }, + }); err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 0", + }}, + } + + var stderr bytes.Buffer + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + + manualTP, ok := dsResult.State[manual.Metadata["session_name"]] + if !ok { + t.Fatalf("desired state missing manual resume session %q; keys=%v stderr=%q", manual.Metadata["session_name"], mapKeys(dsResult.State), stderr.String()) + } + if len(dsResult.State) != 1 { + t.Fatalf("desired sessions = %d, want only manual assigned-work session; keys=%v", len(dsResult.State), mapKeys(dsResult.State)) + } + if !manualTP.ManualSession { + t.Fatal("manual assigned-work singleton ManualSession = false, want true") + } + if manualTP.InstanceName != "cashmaster/refinery-1" { + t.Fatalf("manual assigned-work singleton InstanceName = %q, want preserved identity", manualTP.InstanceName) + } + if manualTP.Alias != "cashmaster/refinery-1" { + t.Fatalf("manual assigned-work singleton Alias = %q, want preserved identity", manualTP.Alias) + } + stored, err := store.Get(manual.ID) + if err != nil { + t.Fatalf("Get(%s): %v", manual.ID, err) + } + if got := stored.Metadata["agent_name"]; got != "cashmaster/refinery-1" { + t.Fatalf("stored agent_name = %q, want preserved manual identity", got) + } + if got := stored.Metadata["alias"]; got != "cashmaster/refinery-1" { + t.Fatalf("stored alias = %q, want preserved manual identity", got) + } + if got := stored.Metadata["pool_slot"]; got != "1" { + t.Fatalf("stored pool_slot = %q, want preserved manual identity", got) + } +} + +func TestBuildDesiredState_MaxOneAgentSkipsCanonicalDuplicateWhenStaleAssignedWorkWinsCap(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + stalePriority := 10 + if _, err := store.Create(beads.Bead{ + Title: "stale assigned work", + Type: "task", + Status: "in_progress", + Priority: &stalePriority, + Assignee: "cashmaster/refinery-1", + Metadata: map[string]string{ + "gc.routed_to": "cashmaster/refinery", + }, + }); err != nil { + t.Fatal(err) + } + canonicalPriority := 1 + if _, err := store.Create(beads.Bead{ + Title: "canonical assigned work", + Type: "task", + Status: "in_progress", + Priority: &canonicalPriority, + Assignee: "cashmaster/refinery", + Metadata: map[string]string{ + "gc.routed_to": "cashmaster/refinery", + }, + }); err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 0", + }}, + } + + var stderr bytes.Buffer + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + + if _, ok := dsResult.State[canonical.Metadata["session_name"]]; ok { + t.Fatalf("desired state includes unselected canonical duplicate %q; keys=%v", canonical.Metadata["session_name"], mapKeys(dsResult.State)) + } + tp, ok := dsResult.State[stale.Metadata["session_name"]] + if !ok { + t.Fatalf("desired state missing stale resume session %q; keys=%v stderr=%q", stale.Metadata["session_name"], mapKeys(dsResult.State), stderr.String()) + } + if len(dsResult.State) != 1 { + t.Fatalf("desired state has %d sessions, want singleton cap enforced; keys=%v", len(dsResult.State), mapKeys(dsResult.State)) + } + if tp.InstanceName != "cashmaster/refinery" { + t.Fatalf("InstanceName = %q, want canonical singleton identity", tp.InstanceName) + } + if tp.Alias != "" { + t.Fatalf("Alias = %q, want deferred alias while canonical duplicate owns it", tp.Alias) + } + + storedStale, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if !containsString(sessionpkg.AliasHistory(storedStale.Metadata), "cashmaster/refinery-1") { + t.Fatalf("alias_history = %#v, want stale singleton alias preserved for next tick", sessionpkg.AliasHistory(storedStale.Metadata)) + } + + var secondStderr bytes.Buffer + secondResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &secondStderr) + if _, ok := secondResult.State[canonical.Metadata["session_name"]]; ok { + t.Fatalf("second desired state includes unselected canonical duplicate %q; keys=%v", canonical.Metadata["session_name"], mapKeys(secondResult.State)) + } + secondTP, ok := secondResult.State[stale.Metadata["session_name"]] + if !ok { + t.Fatalf("second desired state missing stale resume session %q; keys=%v stderr=%q", stale.Metadata["session_name"], mapKeys(secondResult.State), secondStderr.String()) + } + if len(secondResult.State) != 1 { + t.Fatalf("second desired state has %d sessions, want singleton cap enforced; keys=%v", len(secondResult.State), mapKeys(secondResult.State)) + } + if secondTP.InstanceName != "cashmaster/refinery" { + t.Fatalf("second InstanceName = %q, want canonical singleton identity", secondTP.InstanceName) + } + if secondTP.Alias != "" { + t.Fatalf("second Alias = %q, want deferred alias while canonical duplicate owns it", secondTP.Alias) + } +} + +func TestNormalizeNonExpandingPoolSessionBeadDoesNotMutateSnapshotLabels(t *testing.T) { + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + } + bp := &agentBuildParams{ + cityPath: t.TempDir(), + beadStore: store, + sessionBeads: snapshot, + agents: []config.Agent{cfgAgent}, + stderr: io.Discard, + } + + if _, _, err := selectOrCreatePoolSessionBead(bp, &cfgAgent, "cashmaster/refinery", nil, map[string]bool{}, map[int]bool{}); err != nil { + t.Fatalf("selectOrCreatePoolSessionBead: %v", err) + } + + snapshotBeads := snapshot.Open() + if len(snapshotBeads) != 1 { + t.Fatalf("snapshot beads = %d, want 1", len(snapshotBeads)) + } + if !containsString(snapshotBeads[0].Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("snapshot labels = %#v, want original stale agent label preserved", snapshotBeads[0].Labels) + } + if containsString(snapshotBeads[0].Labels, "agent:cashmaster/refinery") { + t.Fatalf("snapshot labels = %#v, must not be mutated to canonical label", snapshotBeads[0].Labels) + } + if got := snapshotBeads[0].Metadata["agent_name"]; got != "cashmaster/refinery-1" { + t.Fatalf("snapshot agent_name = %q, want original stale identity preserved", got) + } + if got := snapshotBeads[0].Metadata["alias"]; got != "cashmaster/refinery-1" { + t.Fatalf("snapshot alias = %q, want original stale identity preserved", got) + } + if got := snapshotBeads[0].Metadata["pool_slot"]; got != "1" { + t.Fatalf("snapshot pool_slot = %q, want original stale slot preserved", got) + } + got, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if !containsString(got.Labels, "agent:cashmaster/refinery") { + t.Fatalf("stored labels = %#v, want canonical label after normalization", got.Labels) + } + if containsString(got.Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("stored labels = %#v, must not include stale label after normalization", got.Labels) + } +} + +func TestNormalizeNonExpandingPoolSessionBeadCopiesSnapshotLabelsBeforeAddOnlyAppend(t *testing.T) { + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + labels := make([]string, 2, 4) + labels[0] = sessionBeadLabel + labels[1] = "template:cashmaster/refinery" + stale.Labels = labels + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + } + bp := &agentBuildParams{ + cityPath: t.TempDir(), + beadStore: store, + sessionBeads: snapshot, + agents: []config.Agent{cfgAgent}, + stderr: io.Discard, + } + + if _, _, err := selectOrCreatePoolSessionBead(bp, &cfgAgent, "cashmaster/refinery", nil, map[string]bool{}, map[int]bool{}); err != nil { + t.Fatalf("selectOrCreatePoolSessionBead: %v", err) + } + + snapshotBeads := snapshot.Open() + if len(snapshotBeads) != 1 { + t.Fatalf("snapshot beads = %d, want 1", len(snapshotBeads)) + } + if cap(snapshotBeads[0].Labels) <= len(snapshotBeads[0].Labels) { + t.Fatalf("snapshot labels capacity = %d, want spare capacity to exercise add-only append", cap(snapshotBeads[0].Labels)) + } + expanded := snapshotBeads[0].Labels[:cap(snapshotBeads[0].Labels)] + if got := expanded[len(snapshotBeads[0].Labels)]; got != "" { + t.Fatalf("snapshot labels backing array was mutated at append slot: %q", got) + } + got, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if !containsString(got.Labels, "agent:cashmaster/refinery") { + t.Fatalf("stored labels = %#v, want canonical label after normalization", got.Labels) + } +} + +func TestRealizePoolDesiredSessionsDefersAliasWhenNormalizationCollides(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + poolAliasConflictCountMetadataKey: "2", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + }}, + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + snapshot.add(canonical) + var stderr bytes.Buffer + bp := newAgentBuildParams("test-city", cityPath, cfg, runtime.NewFake(), time.Now().UTC(), store, &stderr) + bp.sessionBeads = snapshot + desired := map[string]TemplateParams{} + + realizePoolDesiredSessions(bp, &cfg.Agents[0], PoolDesiredState{ + Template: "cashmaster/refinery", + Requests: []SessionRequest{{ + Template: "cashmaster/refinery", + Tier: "resume", + SessionBeadID: stale.ID, + }}, + }, desired, &stderr) + + tp, ok := desired[stale.Metadata["session_name"]] + if !ok { + t.Fatalf("desired state missing stale resume session; keys=%v stderr=%q", mapKeys(desired), stderr.String()) + } + if got := tp.Alias; got != "" { + t.Fatalf("deferred singleton TemplateParams.Alias = %q, want empty while canonical alias is unavailable", got) + } + if got := tp.Env["GC_ALIAS"]; got != "" { + t.Fatalf("deferred singleton GC_ALIAS = %q, want empty while canonical alias is unavailable", got) + } + if got := tp.Env["GC_AGENT"]; got != tp.SessionName { + t.Fatalf("deferred singleton GC_AGENT = %q, want bead session name %q", got, tp.SessionName) + } + if tp.EnvIdentityStamped { + t.Fatal("deferred singleton EnvIdentityStamped = true, want false until alias is available") + } + stored, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := stored.Metadata["alias"]; got != "" { + t.Fatalf("stored deferred singleton alias = %q, want empty while canonical alias is unavailable", got) + } + if got := stored.Metadata[poolAliasConflictMetadataKey]; got != "cashmaster/refinery" { + t.Fatalf("stored pool_alias_conflict = %q, want canonical alias", got) + } + if got := stored.Metadata[poolAliasConflictCountMetadataKey]; got != "3" { + t.Fatalf("stored pool_alias_conflict_count = %q, want incremented retry count", got) + } + if _, err := time.Parse(time.RFC3339, stored.Metadata[poolAliasConflictAtMetadataKey]); err != nil { + t.Fatalf("stored pool_alias_conflict_at = %q, want RFC3339 timestamp: %v", stored.Metadata[poolAliasConflictAtMetadataKey], err) + } + if !strings.Contains(stderr.String(), "deferring singleton pool identity normalization") { + t.Fatalf("stderr = %q, want normalization deferral diagnostic", stderr.String()) + } +} + +func TestSyncSessionBeads_ReclaimsDeferredSingletonAliasAfterConflictClears(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + }}, + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + snapshot.add(canonical) + var buildStderr bytes.Buffer + bp := newAgentBuildParams("test-city", cityPath, cfg, runtime.NewFake(), time.Now().UTC(), store, &buildStderr) + bp.sessionBeads = snapshot + desired := map[string]TemplateParams{} + + realizePoolDesiredSessions(bp, &cfg.Agents[0], PoolDesiredState{ + Template: "cashmaster/refinery", + Requests: []SessionRequest{{ + Template: "cashmaster/refinery", + Tier: "resume", + SessionBeadID: stale.ID, + }}, + }, desired, &buildStderr) + + var persistentStderr bytes.Buffer + persistentClk := &clock.Fake{Time: time.Date(2026, 5, 6, 2, 30, 0, 0, time.UTC)} + syncSessionBeads(cityPath, store, desired, runtime.NewFake(), allConfiguredDS(desired), cfg, persistentClk, &persistentStderr, false) + + stillConflicted, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := stillConflicted.Metadata["alias"]; got != "" { + t.Fatalf("persistent-conflict alias = %q, want still deferred while canonical owner exists", got) + } + if got := stillConflicted.Metadata[poolAliasConflictMetadataKey]; got != "cashmaster/refinery" { + t.Fatalf("persistent-conflict pool_alias_conflict = %q, want canonical alias", got) + } + if got := stillConflicted.Metadata[poolAliasConflictCountMetadataKey]; got != "2" { + t.Fatalf("persistent-conflict pool_alias_conflict_count = %q, want sync retry increment", got) + } + + if err := store.Close(canonical.ID); err != nil { + t.Fatalf("Close(%s): %v", canonical.ID, err) + } + var syncStderr bytes.Buffer + clk := &clock.Fake{Time: time.Date(2026, 5, 6, 3, 0, 0, 0, time.UTC)} + syncSessionBeads(cityPath, store, desired, runtime.NewFake(), allConfiguredDS(desired), cfg, clk, &syncStderr, false) + + got, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got.Metadata["alias"] != "cashmaster/refinery" { + t.Fatalf("alias = %q, want canonical alias after conflict clears; sync stderr=%q", got.Metadata["alias"], syncStderr.String()) + } + if got.Metadata[poolAliasConflictMetadataKey] != "" { + t.Fatalf("pool_alias_conflict = %q, want cleared", got.Metadata[poolAliasConflictMetadataKey]) + } + if got.Metadata[poolAliasConflictCountMetadataKey] != "" { + t.Fatalf("pool_alias_conflict_count = %q, want cleared", got.Metadata[poolAliasConflictCountMetadataKey]) + } + if got.Metadata[poolAliasConflictAtMetadataKey] != "" { + t.Fatalf("pool_alias_conflict_at = %q, want cleared", got.Metadata[poolAliasConflictAtMetadataKey]) + } +} + +func TestNormalizeNonExpandingPoolSessionBeadReclaimsDeferredAlias(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + poolAliasConflictMetadataKey: "cashmaster/refinery", + poolAliasConflictCountMetadataKey: "3", + poolAliasConflictAtMetadataKey: "2026-05-06T02:30:00Z", + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + }}, + } + var stderr bytes.Buffer + bp := newAgentBuildParams("test-city", cityPath, cfg, runtime.NewFake(), time.Now().UTC(), store, &stderr) + + result, err := normalizeNonExpandingPoolSessionBead(bp, &cfg.Agents[0], stale) + if err != nil { + t.Fatalf("normalizeNonExpandingPoolSessionBead: %v", err) + } + if got := result.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("result alias = %q, want canonical alias", got) + } + if got := result.Metadata[poolAliasConflictMetadataKey]; got != "" { + t.Fatalf("result pool_alias_conflict = %q, want cleared", got) + } + stored, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := stored.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("stored alias = %q, want canonical alias", got) + } + if got := stored.Metadata[poolAliasConflictMetadataKey]; got != "" { + t.Fatalf("stored pool_alias_conflict = %q, want cleared", got) + } + if got := stored.Metadata[poolAliasConflictCountMetadataKey]; got != "" { + t.Fatalf("stored pool_alias_conflict_count = %q, want cleared", got) + } + if got := stored.Metadata[poolAliasConflictAtMetadataKey]; got != "" { + t.Fatalf("stored pool_alias_conflict_at = %q, want cleared", got) + } +} + +func TestReconcilerClosesUnselectedCanonicalSingletonBeforeAliasReclaim(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + priority := 10 + if _, err := store.Create(beads.Bead{ + Title: "stale assigned work", + Type: "task", + Status: "in_progress", + Priority: &priority, + Assignee: "cashmaster/refinery-1", + Metadata: map[string]string{ + "gc.routed_to": "cashmaster/refinery", + }, + }); err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 0", + }}, + } + + var buildStderr bytes.Buffer + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &buildStderr) + if _, ok := dsResult.State[canonical.Metadata["session_name"]]; ok { + t.Fatalf("desired state includes unselected canonical singleton %q; keys=%v", canonical.Metadata["session_name"], mapKeys(dsResult.State)) + } + if _, ok := dsResult.State[stale.Metadata["session_name"]]; !ok { + t.Fatalf("desired state missing stale singleton resume %q; keys=%v stderr=%q", stale.Metadata["session_name"], mapKeys(dsResult.State), buildStderr.String()) + } + + sessions, err := loadSessionBeads(store) + if err != nil { + t.Fatal(err) + } + sp := runtime.NewFake() + clk := &clock.Fake{Time: time.Date(2026, 5, 6, 4, 0, 0, 0, time.UTC)} + var reconcileStdout, reconcileStderr bytes.Buffer + reconcileSessionBeads( + context.Background(), sessions, dsResult.State, configuredSessionNames(cfg, "", store), cfg, sp, + store, nil, nil, nil, newDrainTracker(), map[string]int{"cashmaster/refinery": 1}, false, nil, "", + nil, clk, events.Discard, 0, 0, &reconcileStdout, &reconcileStderr, + ) + + closedCanonical, err := store.Get(canonical.ID) + if err != nil { + t.Fatalf("Get(%s): %v", canonical.ID, err) + } + if closedCanonical.Status != "closed" { + t.Fatalf("canonical status = %q, want closed; stdout=%q stderr=%q", closedCanonical.Status, reconcileStdout.String(), reconcileStderr.String()) + } + if want := sessionpkg.CanonicalCloseReason("orphaned"); closedCanonical.Metadata["close_reason"] != want { + t.Fatalf("canonical close_reason = %q, want %q", closedCanonical.Metadata["close_reason"], want) + } + + var syncStderr bytes.Buffer + syncSessionBeads(cityPath, store, dsResult.State, sp, allConfiguredDS(dsResult.State), cfg, clk, &syncStderr, false) + reclaimed, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := reclaimed.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("reclaimed alias = %q, want canonical alias; sync stderr=%q", got, syncStderr.String()) + } + if got := reclaimed.Metadata[poolAliasConflictMetadataKey]; got != "" { + t.Fatalf("pool_alias_conflict = %q, want cleared", got) + } +} + +func TestProductionOrderDeferredSingletonAliasReclaimsOnSecondTick(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + priority := 10 + if _, err := store.Create(beads.Bead{ + Title: "stale assigned work", + Type: "task", + Status: "in_progress", + Priority: &priority, + Assignee: "cashmaster/refinery-1", + Metadata: map[string]string{ + "gc.routed_to": "cashmaster/refinery", + }, + }); err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 0", + }}, + } + sp := runtime.NewFake() + clk := &clock.Fake{Time: time.Date(2026, 5, 6, 4, 0, 0, 0, time.UTC)} + + var firstBuildStderr bytes.Buffer + firstTick := buildDesiredState("test-city", cityPath, clk.Now().UTC(), cfg, sp, store, &firstBuildStderr) + if _, ok := firstTick.State[stale.Metadata["session_name"]]; !ok { + t.Fatalf("first tick desired state missing stale singleton resume %q; keys=%v stderr=%q", stale.Metadata["session_name"], mapKeys(firstTick.State), firstBuildStderr.String()) + } + + var firstSyncStderr bytes.Buffer + _, updated := syncSessionBeadsWithSnapshotAndRigStores( + cityPath, + store, + nil, + firstTick.State, + sp, + configuredSessionNames(cfg, "", store), + cfg, + clk, + &firstSyncStderr, + true, + nil, + ) + stillDeferred, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := stillDeferred.Metadata["alias"]; got != "" { + t.Fatalf("first sync alias = %q, want deferred while canonical owner remains open", got) + } + if got := stillDeferred.Metadata[poolAliasConflictMetadataKey]; got != "cashmaster/refinery" { + t.Fatalf("first sync pool_alias_conflict = %q, want canonical alias; sync stderr=%q", got, firstSyncStderr.String()) + } + + open := updated.Open() + var reconcileStdout, reconcileStderr bytes.Buffer + reconcileSessionBeads( + context.Background(), open, firstTick.State, configuredSessionNames(cfg, "", store), cfg, sp, + store, nil, nil, nil, newDrainTracker(), map[string]int{"cashmaster/refinery": 1}, false, nil, "", + nil, clk, events.Discard, 0, 0, &reconcileStdout, &reconcileStderr, + ) + closedCanonical, err := store.Get(canonical.ID) + if err != nil { + t.Fatalf("Get(%s): %v", canonical.ID, err) + } + if closedCanonical.Status != "closed" { + t.Fatalf("canonical status = %q, want closed after first production-order reconcile; stdout=%q stderr=%q", closedCanonical.Status, reconcileStdout.String(), reconcileStderr.String()) + } + + clk.Advance(time.Minute) + var secondBuildStderr bytes.Buffer + secondTick := buildDesiredState("test-city", cityPath, clk.Now().UTC(), cfg, sp, store, &secondBuildStderr) + var secondSyncStderr bytes.Buffer + syncSessionBeads( + cityPath, + store, + secondTick.State, + sp, + allConfiguredDS(secondTick.State), + cfg, + clk, + &secondSyncStderr, + true, + ) + + reclaimed, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) + } + if got := reclaimed.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("second tick alias = %q, want canonical alias; build stderr=%q sync stderr=%q", got, secondBuildStderr.String(), secondSyncStderr.String()) + } + if got := reclaimed.Metadata["pool_slot"]; got != "" { + t.Fatalf("second tick pool_slot = %q, want empty after singleton recovery", got) + } + if got := reclaimed.Metadata[poolAliasConflictMetadataKey]; got != "" { + t.Fatalf("pool_alias_conflict = %q, want cleared", got) + } + if got := reclaimed.Metadata[poolAliasConflictCountMetadataKey]; got != "" { + t.Fatalf("pool_alias_conflict_count = %q, want cleared", got) + } + if got := reclaimed.Metadata[poolAliasConflictAtMetadataKey]; got != "" { + t.Fatalf("pool_alias_conflict_at = %q, want cleared", got) + } +} + +func TestDiscoverSessionBeadsSkipsStaleMaxOneWhenDependencyFloorDesired(t *testing.T) { + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "gascity/db-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:gascity/db-1", "template:gascity/db"}, + Metadata: map[string]string{ + "template": "gascity/db", + "agent_name": "gascity/db-1", + "alias": "gascity/db-1", + "session_name": "s-db-stale", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "db", + Dir: "gascity", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + }}, + } + desired := map[string]TemplateParams{ + "s-db-canonical": { + TemplateName: "gascity/db", + InstanceName: "s-db-canonical", + DependencyOnly: true, + }, + } + bp := &agentBuildParams{ + cityPath: t.TempDir(), + city: cfg, + beadStore: store, + sessionBeads: snapshot, + agents: cfg.Agents, + } + + discoverSessionBeadsWithRoots(bp, cfg, desired, nil, map[string]bool{"gascity/db": true}, nil, io.Discard) + + if _, ok := desired[stale.Metadata["session_name"]]; ok { + t.Fatalf("desired state includes stale duplicate dependency-floor sibling; keys=%v", mapKeys(desired)) + } +} + +func TestNonExpandingPoolIdentitySlotRecognizesOutOfRangeNumericSuffix(t *testing.T) { + cfgAgent := &config.Agent{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + } + + if got := nonExpandingPoolIdentitySlot(cfgAgent, "cashmaster/refinery-10"); got != 10 { + t.Fatalf("slot = %d, want out-of-range stale singleton suffix 10 recognized", got) + } +} + +func TestBuildDesiredState_MaxOneCanonicalBeadIsIdempotent(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-canonical", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + }, + }) + if err != nil { + t.Fatal(err) + } + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + + before, err := store.Get(canonical.ID) + if err != nil { + t.Fatalf("Get(%s): %v", canonical.ID, err) + } + var stderr bytes.Buffer + first := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + afterFirst, err := store.Get(canonical.ID) + if err != nil { + t.Fatalf("Get(%s) after first pass: %v", canonical.ID, err) + } + second := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, &stderr) + afterSecond, err := store.Get(canonical.ID) + if err != nil { + t.Fatalf("Get(%s) after second pass: %v", canonical.ID, err) + } + + if _, ok := first.State[canonical.Metadata["session_name"]]; !ok { + t.Fatalf("first desired state missing canonical singleton; keys=%v", mapKeys(first.State)) + } + if _, ok := second.State[canonical.Metadata["session_name"]]; !ok { + t.Fatalf("second desired state missing canonical singleton; keys=%v", mapKeys(second.State)) + } + if before.Title != afterFirst.Title || !reflect.DeepEqual(before.Metadata, afterFirst.Metadata) || !reflect.DeepEqual(before.Labels, afterFirst.Labels) { + t.Fatalf("first pass mutated canonical bead: before=%#v after=%#v", before, afterFirst) + } + if afterFirst.Title != afterSecond.Title || !reflect.DeepEqual(afterFirst.Metadata, afterSecond.Metadata) || !reflect.DeepEqual(afterFirst.Labels, afterSecond.Labels) { + t.Fatalf("second pass mutated canonical bead: first=%#v second=%#v", afterFirst, afterSecond) + } + if strings.Contains(stderr.String(), "collapsing phantom pool identity") { + t.Fatalf("stderr = %q, want no normalization diagnostic for canonical bead", stderr.String()) + } +} + +func TestBuildDesiredState_NamepoolMaxOneUsesNamepoolIdentity(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "worker", + Dir: "rig", + StartCommand: "true", + MaxActiveSessions: intPtr(1), + NamepoolNames: []string{"furiosa"}, + ScaleCheck: "printf 1", + }}, + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, io.Discard) + if len(dsResult.State) != 1 { + t.Fatalf("desired sessions = %d, want 1", len(dsResult.State)) + } + var tp TemplateParams + for _, candidate := range dsResult.State { + tp = candidate + } + if tp.InstanceName != "rig/furiosa" { + t.Fatalf("InstanceName = %q, want namepool identity", tp.InstanceName) + } + if tp.PoolSlot != 1 { + t.Fatalf("PoolSlot = %d, want namepool slot 1", tp.PoolSlot) + } + sessionBeads, err := loadSessionBeads(store) + if err != nil { + t.Fatalf("load session beads: %v", err) + } + if len(sessionBeads) != 1 { + t.Fatalf("session beads = %d, want 1", len(sessionBeads)) + } + got := sessionBeads[0] + if got.Metadata["agent_name"] != "rig/furiosa" { + t.Fatalf("agent_name = %q, want namepool identity", got.Metadata["agent_name"]) + } + if got.Metadata["alias"] != "rig/furiosa" { + t.Fatalf("alias = %q, want namepool identity", got.Metadata["alias"]) + } + if got.Metadata["pool_slot"] != "1" { + t.Fatalf("pool_slot = %q, want 1 for namepool singleton", got.Metadata["pool_slot"]) + } +} + func TestBuildDesiredState_NewPoolSessionBeadDefersAliasWhenConcreteAliasTaken(t *testing.T) { cityPath := t.TempDir() store := beads.NewMemStore() @@ -1825,7 +3211,7 @@ func TestCreatePoolSessionBeadWithGuardedAlias_LogsAliasLockSetupFailure(t *test stderr: &stderr, } - bead, err := createPoolSessionBeadWithGuardedAlias(bp, "claude", "claude-1", 1) + bead, err := createPoolSessionBeadWithGuardedAlias(bp, nil, "claude", "claude-1", 1) if err != nil { t.Fatalf("createPoolSessionBeadWithGuardedAlias: %v", err) } @@ -1837,6 +3223,44 @@ func TestCreatePoolSessionBeadWithGuardedAlias_LogsAliasLockSetupFailure(t *test } } +func TestCreatePoolSessionBeadWithGuardedAliasRejectsUnsupportedTransport(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city", Provider: "opencode"}, + Session: config.SessionConfig{Provider: config.SessionTransportACP}, + Providers: map[string]config.ProviderSpec{ + "opencode": { + Command: "echo", + Args: []string{"provider"}, + ACPCommand: "echo", + ACPArgs: []string{"acp"}, + PromptMode: "none", + SupportsACP: boolPtr(true), + }, + }, + Agents: []config.Agent{{ + Name: "worker", + Provider: "opencode", + Session: config.SessionTransportTmux, + MaxActiveSessions: intPtr(1), + }}, + } + store := beads.NewMemStore() + sp := &acpOnlyDesiredStateProvider{Fake: runtime.NewFake()} + bp := newAgentBuildParams("test-city", t.TempDir(), cfg, sp, time.Now().UTC(), store, io.Discard) + + _, err := createPoolSessionBeadWithGuardedAlias(bp, &cfg.Agents[0], "worker", "worker", 0) + if err == nil || !strings.Contains(err.Error(), "cannot route tmux sessions") { + t.Fatalf("createPoolSessionBeadWithGuardedAlias error = %v, want tmux routing rejection", err) + } + beads, listErr := store.ListByLabel(sessionBeadLabel, 0) + if listErr != nil { + t.Fatalf("ListByLabel(%q): %v", sessionBeadLabel, listErr) + } + if len(beads) != 0 { + t.Fatalf("session bead count = %d, want 0 after rejected transport: %#v", len(beads), beads) + } +} + func TestBuildDesiredState_MinZeroDefaultScaleCheckRoutedWorkCreatesPoolSession(t *testing.T) { skipSlowCmdGCTest(t, "uses real bd subprocesses for routed-work scale checks; run make test-cmd-gc-process for full coverage") bdPath, err := findPreferredBinary("bd", "/home/ubuntu/.local/bin/bd") @@ -4700,6 +6124,31 @@ func TestExistingPoolSlot_PreservesStampedOutOfBoundsLiveIdentity(t *testing.T) } } +func TestValidateAgentSessionTransportForBuild_ProductionShapeRunsTransportValidation(t *testing.T) { + bp := &agentBuildParams{ + workspace: &config.Workspace{}, + providers: map[string]config.ProviderSpec{ + "test-agent": { + Command: "test-agent", + SupportsACP: boolPtr(true), + }, + }, + lookPath: func(string) (string, error) { + return "/usr/bin/test-agent", nil + }, + sp: runtime.NewFake(), + } + cfgAgent := &config.Agent{Name: "worker", Provider: "test-agent", Session: config.SessionTransportACP} + + err := validateAgentSessionTransportForBuild(bp, cfgAgent, cfgAgent.QualifiedName()) + if err == nil { + t.Fatal("validateAgentSessionTransportForBuild returned nil, want transport routing error") + } + if !strings.Contains(err.Error(), "requires ACP transport") { + t.Fatalf("validateAgentSessionTransportForBuild error = %v, want ACP transport validation error", err) + } +} + func TestBuildDesiredState_DoesNotCreateDuplicatePoolBeadForDiscoveredSession(t *testing.T) { cityPath := t.TempDir() store := beads.NewMemStore() @@ -5208,106 +6657,412 @@ func TestSelectOrCreatePoolSessionBead_UsesFreshCreateTimeNotBeaconTime(t *testi if err != nil { t.Fatalf("parse pending_create_started_at %q: %v", result.Metadata["pending_create_started_at"], err) } - if startedAt.Before(beforeCreate) { - t.Fatalf("pending_create_started_at = %s, want current create time after %s", startedAt, beforeCreate) + if startedAt.Before(beforeCreate) { + t.Fatalf("pending_create_started_at = %s, want current create time after %s", startedAt, beforeCreate) + } + if !startedAt.After(oldBeacon.Add(staleCreatingStateTimeout)) { + t.Fatalf("pending_create_started_at = %s, want independent from stale beacon %s", startedAt, oldBeacon) + } + result.CreatedAt = oldBeacon + if staleCreatingState(result, &clock.Fake{Time: startedAt.Add(30 * time.Second)}) { + t.Fatal("fresh pool session was stale when row CreatedAt matched old controller beacon") + } +} + +func TestSelectOrCreatePoolSessionBead_ReusesPreferredDrained(t *testing.T) { + store := beads.NewMemStore() + drained, err := store.Create(beads.Bead{ + Title: "claude", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel}, + Metadata: map[string]string{ + "template": "claude", + "agent_name": "claude", + "session_name": "claude-drained", + "state": "drained", + "pool_slot": "4", + "pool_managed": "true", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(drained) + cfgAgent := config.Agent{Name: "claude", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(5)} + bp := &agentBuildParams{ + beadStore: store, + sessionBeads: snapshot, + agents: []config.Agent{cfgAgent}, + } + + result, slot, err := selectOrCreatePoolSessionBead(bp, &cfgAgent, "claude", &drained, map[string]bool{}, map[int]bool{}) + if err != nil { + t.Fatalf("selectOrCreatePoolSessionBead: %v", err) + } + if result.ID != drained.ID { + t.Fatal("resume tier should reuse preferred drained session bead") + } + if slot != 4 { + t.Fatalf("preferred reuse slot = %d, want 4", slot) + } +} + +func TestSelectOrCreateDependencyPoolSessionBead_SkipsDrained(t *testing.T) { + store := beads.NewMemStore() + drained, err := store.Create(beads.Bead{ + Title: "claude", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel}, + Metadata: map[string]string{ + "template": "claude", + "agent_name": "claude", + "session_name": "claude-dep-drained", + "state": "asleep", + "sleep_reason": "drained", + "dependency_only": "true", + "pool_managed": "true", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(drained) + cfgAgent := config.Agent{Name: "claude", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(5)} + bp := &agentBuildParams{ + beadStore: store, + sessionBeads: snapshot, + agents: []config.Agent{cfgAgent}, + } + + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "claude") + if err != nil { + t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) + } + if result.ID == drained.ID { + t.Fatal("should not reuse drained dependency session bead for generic dependency demand") + } + if got := result.Metadata["agent_name"]; got != "claude-1" { + t.Fatalf("dependency agent_name = %q, want claude-1", got) + } + if got := result.Metadata["alias"]; got != "claude-1" { + t.Fatalf("dependency alias = %q, want claude-1", got) + } + if got := result.Metadata["pool_slot"]; got != "1" { + t.Fatalf("dependency pool_slot = %q, want 1", got) + } + if got := result.Title; got != "claude-1" { + t.Fatalf("dependency title = %q, want claude-1", got) + } + if !containsString(result.Labels, "agent:claude-1") { + t.Fatalf("dependency labels = %#v, want concrete slot agent label", result.Labels) + } +} + +func TestSelectOrCreateDependencyPoolSessionBead_MaxOneUsesCanonicalIdentity(t *testing.T) { + store := beads.NewMemStore() + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + } + bp := &agentBuildParams{ + cityPath: t.TempDir(), + beadStore: store, + sessionBeads: &sessionBeadSnapshot{}, + agents: []config.Agent{cfgAgent}, + } + + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "cashmaster/refinery") + if err != nil { + t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) + } + if got := result.Metadata["agent_name"]; got != "cashmaster/refinery" { + t.Fatalf("dependency agent_name = %q, want canonical non-pool identity", got) + } + if got := result.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("dependency alias = %q, want canonical non-pool identity", got) + } + if got := result.Metadata["pool_slot"]; got != "" { + t.Fatalf("dependency pool_slot = %q, want empty for max_active_sessions=1", got) + } + if got := result.Title; got != "cashmaster/refinery" { + t.Fatalf("dependency title = %q, want canonical non-pool identity", got) + } + if containsString(result.Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("dependency labels = %#v, must not include phantom pool identity", result.Labels) + } + if !containsString(result.Labels, "agent:cashmaster/refinery") { + t.Fatalf("dependency labels = %#v, want canonical agent label", result.Labels) + } +} + +func TestSelectOrCreateDependencyPoolSessionBead_MaxOneNormalizesExistingStaleIdentity(t *testing.T) { + store := beads.NewMemStore() + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-dep-stale", + "state": "awake", + "dependency_only": boolMetadata(true), + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + poolAliasConflictMetadataKey: "cashmaster/refinery", + poolAliasConflictCountMetadataKey: "4", + poolAliasConflictAtMetadataKey: "2026-05-06T01:00:00Z", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(stale) + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + } + bp := &agentBuildParams{ + cityPath: t.TempDir(), + beadStore: store, + sessionBeads: snapshot, + agents: []config.Agent{cfgAgent}, + } + + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "cashmaster/refinery") + if err != nil { + t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) + } + if result.ID != stale.ID { + t.Fatalf("dependency reuse ID = %q, want stale bead %q", result.ID, stale.ID) + } + if got := result.Metadata["agent_name"]; got != "cashmaster/refinery" { + t.Fatalf("dependency agent_name = %q, want canonical non-pool identity", got) + } + if got := result.Metadata["alias"]; got != "cashmaster/refinery" { + t.Fatalf("dependency alias = %q, want canonical non-pool identity", got) + } + if got := result.Metadata["pool_slot"]; got != "" { + t.Fatalf("dependency pool_slot = %q, want empty after normalization", got) + } + if got := result.Metadata[poolAliasConflictMetadataKey]; got != "" { + t.Fatalf("dependency pool_alias_conflict = %q, want cleared after successful normalization", got) + } + if got := result.Metadata[poolAliasConflictCountMetadataKey]; got != "" { + t.Fatalf("dependency pool_alias_conflict_count = %q, want cleared after successful normalization", got) + } + if got := result.Metadata[poolAliasConflictAtMetadataKey]; got != "" { + t.Fatalf("dependency pool_alias_conflict_at = %q, want cleared after successful normalization", got) + } + if containsString(result.Labels, "agent:cashmaster/refinery-1") { + t.Fatalf("dependency labels = %#v, must not include stale label", result.Labels) + } + if !containsString(result.Labels, "agent:cashmaster/refinery") { + t.Fatalf("dependency labels = %#v, want canonical agent label", result.Labels) + } + stored, err := store.Get(stale.ID) + if err != nil { + t.Fatalf("Get(%s): %v", stale.ID, err) } - if !startedAt.After(oldBeacon.Add(staleCreatingStateTimeout)) { - t.Fatalf("pending_create_started_at = %s, want independent from stale beacon %s", startedAt, oldBeacon) + if stored.Metadata["agent_name"] != "cashmaster/refinery" || stored.Metadata["alias"] != "cashmaster/refinery" || stored.Metadata["pool_slot"] != "" { + t.Fatalf("stored dependency metadata = %#v, want normalized singleton identity", stored.Metadata) } - result.CreatedAt = oldBeacon - if staleCreatingState(result, &clock.Fake{Time: startedAt.Add(30 * time.Second)}) { - t.Fatal("fresh pool session was stale when row CreatedAt matched old controller beacon") + if stored.Metadata[poolAliasConflictMetadataKey] != "" || stored.Metadata[poolAliasConflictCountMetadataKey] != "" || stored.Metadata[poolAliasConflictAtMetadataKey] != "" { + t.Fatalf("stored dependency conflict metadata = %#v, want cleared after successful normalization", stored.Metadata) } } -func TestSelectOrCreatePoolSessionBead_ReusesPreferredDrained(t *testing.T) { +func TestSelectOrCreateDependencyPoolSessionBead_MaxOnePrefersCanonicalDependencyDuplicate(t *testing.T) { store := beads.NewMemStore() - drained, err := store.Create(beads.Bead{ - Title: "claude", + stale, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", Type: sessionBeadType, - Labels: []string{sessionBeadLabel}, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, Metadata: map[string]string{ - "template": "claude", - "agent_name": "claude", - "session_name": "claude-drained", - "state": "drained", - "pool_slot": "4", - "pool_managed": "true", + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-dep-stale", + "state": "awake", + "dependency_only": boolMetadata(true), + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) + } + canonical, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-dep-canonical", + "state": "awake", + "dependency_only": boolMetadata(true), + poolManagedMetadataKey: boolMetadata(true), }, }) if err != nil { t.Fatal(err) } snapshot := &sessionBeadSnapshot{} - snapshot.add(drained) - cfgAgent := config.Agent{Name: "claude", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(5)} + snapshot.add(stale) + snapshot.add(canonical) + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + } bp := &agentBuildParams{ + cityPath: t.TempDir(), beadStore: store, sessionBeads: snapshot, agents: []config.Agent{cfgAgent}, } - result, slot, err := selectOrCreatePoolSessionBead(bp, &cfgAgent, "claude", &drained, map[string]bool{}, map[int]bool{}) + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "cashmaster/refinery") if err != nil { - t.Fatalf("selectOrCreatePoolSessionBead: %v", err) + t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) } - if result.ID != drained.ID { - t.Fatal("resume tier should reuse preferred drained session bead") + if result.ID != canonical.ID { + t.Fatalf("dependency reuse ID = %q, want canonical bead %q instead of stale duplicate %q", result.ID, canonical.ID, stale.ID) } - if slot != 4 { - t.Fatalf("preferred reuse slot = %d, want 4", slot) + if got := result.Metadata["agent_name"]; got != "cashmaster/refinery" { + t.Fatalf("dependency agent_name = %q, want canonical non-pool identity", got) + } + if got := result.Metadata["pool_slot"]; got != "" { + t.Fatalf("dependency pool_slot = %q, want empty for canonical max-one bead", got) } } -func TestSelectOrCreateDependencyPoolSessionBead_SkipsDrained(t *testing.T) { - store := beads.NewMemStore() - drained, err := store.Create(beads.Bead{ - Title: "claude", - Type: sessionBeadType, - Labels: []string{sessionBeadLabel}, +func TestSelectOrCreateDependencyPoolSessionBead_MaxOnePicksEarliestCanonicalDuplicate(t *testing.T) { + base := time.Date(2026, 5, 16, 10, 20, 0, 0, time.UTC) + later := beads.Bead{ + ID: "session-later", + Title: "cashmaster/refinery", + Type: sessionBeadType, + Status: "open", + CreatedAt: base.Add(time.Minute), + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, Metadata: map[string]string{ - "template": "claude", - "agent_name": "claude", - "session_name": "claude-dep-drained", - "state": "asleep", - "sleep_reason": "drained", - "dependency_only": "true", - "pool_managed": "true", + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-dep-later", + "state": "awake", + "dependency_only": boolMetadata(true), + poolManagedMetadataKey: boolMetadata(true), }, - }) - if err != nil { - t.Fatal(err) } - snapshot := &sessionBeadSnapshot{} - snapshot.add(drained) - cfgAgent := config.Agent{Name: "claude", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(5)} + earliest := beads.Bead{ + ID: "session-earliest", + Title: "cashmaster/refinery", + Type: sessionBeadType, + Status: "open", + CreatedAt: base, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "alias": "cashmaster/refinery", + "session_name": "s-refinery-dep-earliest", + "state": "awake", + "dependency_only": boolMetadata(true), + poolManagedMetadataKey: boolMetadata(true), + }, + } + cfgAgent := config.Agent{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + } bp := &agentBuildParams{ - beadStore: store, - sessionBeads: snapshot, + sessionBeads: newSessionBeadSnapshot([]beads.Bead{later, earliest}), agents: []config.Agent{cfgAgent}, } - result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "claude") + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfgAgent, "cashmaster/refinery") if err != nil { t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) } - if result.ID == drained.ID { - t.Fatal("should not reuse drained dependency session bead for generic dependency demand") + if result.ID != earliest.ID { + t.Fatalf("dependency reuse ID = %q, want earliest canonical duplicate %q", result.ID, earliest.ID) } - if got := result.Metadata["agent_name"]; got != "claude-1" { - t.Fatalf("dependency agent_name = %q, want claude-1", got) +} + +func TestSelectOrCreatePoolSessionBeadPicksEarliestReusableSingletonCandidate(t *testing.T) { + cityPath := t.TempDir() + store := beads.NewMemStore() + earliest, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-2", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-2", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-2", + "alias": "cashmaster/refinery-2", + "session_name": "s-refinery-earliest", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "2", + }, + }) + if err != nil { + t.Fatal(err) } - if got := result.Metadata["alias"]; got != "claude-1" { - t.Fatalf("dependency alias = %q, want claude-1", got) + later, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery-1", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery-1", "template:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "session_name": "s-refinery-later", + "state": "awake", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }) + if err != nil { + t.Fatal(err) } - if got := result.Metadata["pool_slot"]; got != "1" { - t.Fatalf("dependency pool_slot = %q, want 1", got) + snapshot := &sessionBeadSnapshot{} + snapshot.add(later) + snapshot.add(earliest) + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + }}, } - if got := result.Title; got != "claude-1" { - t.Fatalf("dependency title = %q, want claude-1", got) + var stderr bytes.Buffer + bp := newAgentBuildParams("test-city", cityPath, cfg, runtime.NewFake(), time.Now().UTC(), store, &stderr) + bp.sessionBeads = snapshot + + result, _, err := selectOrCreatePoolSessionBead(bp, &cfg.Agents[0], "cashmaster/refinery", nil, map[string]bool{}, map[int]bool{}) + if err != nil { + t.Fatalf("selectOrCreatePoolSessionBead: %v", err) } - if !containsString(result.Labels, "agent:claude-1") { - t.Fatalf("dependency labels = %#v, want concrete slot agent label", result.Labels) + if result.ID != earliest.ID { + t.Fatalf("selected bead = %q, want earliest reusable candidate %q", result.ID, earliest.ID) } } @@ -5352,6 +7107,48 @@ func TestSelectOrCreateDependencyPoolSessionBead_DefersAliasWhenConcreteAliasTak } } +func TestSelectOrCreateDependencyPoolSessionBead_ReusesLegacyUnqualifiedTemplateWithFullConfig(t *testing.T) { + store := beads.NewMemStore() + legacy, err := store.Create(beads.Bead{ + Title: "legacy db dependency", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel}, + Metadata: map[string]string{ + "template": "db", + "session_name": "s-db-dep-legacy", + "state": "awake", + "dependency_only": "true", + "pool_managed": "true", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(legacy) + cfg := &config.City{Agents: []config.Agent{{ + Name: "db", + Dir: "gascity", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(3), + }}} + bp := &agentBuildParams{ + city: cfg, + cityPath: t.TempDir(), + beadStore: store, + sessionBeads: snapshot, + agents: cfg.Agents, + } + + result, err := selectOrCreateDependencyPoolSessionBead(bp, &cfg.Agents[0], "gascity/db") + if err != nil { + t.Fatalf("selectOrCreateDependencyPoolSessionBead: %v", err) + } + if result.ID != legacy.ID { + t.Fatalf("dependency reuse ID = %q, want legacy bead %q", result.ID, legacy.ID) + } +} + func TestSelectOrCreatePoolSessionBead_ReusesAvailableForNewTier(t *testing.T) { store := beads.NewMemStore() // Existing awake session bead without assigned work — should be reused @@ -5394,6 +7191,46 @@ func TestSelectOrCreatePoolSessionBead_ReusesAvailableForNewTier(t *testing.T) { } } +func TestSelectOrCreatePoolSessionBead_ReusesLegacyUnqualifiedTemplateWithFullConfig(t *testing.T) { + store := beads.NewMemStore() + legacy, err := store.Create(beads.Bead{ + Title: "legacy refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel}, + Metadata: map[string]string{ + "template": "refinery", + "session_name": "s-refinery-legacy", + "state": "awake", + "pool_managed": "true", + }, + }) + if err != nil { + t.Fatal(err) + } + snapshot := &sessionBeadSnapshot{} + snapshot.add(legacy) + cfg := &config.City{Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(3), + }}} + bp := &agentBuildParams{ + city: cfg, + beadStore: store, + sessionBeads: snapshot, + agents: cfg.Agents, + } + + result, _, err := selectOrCreatePoolSessionBead(bp, &cfg.Agents[0], "cashmaster/refinery", nil, map[string]bool{}, map[int]bool{}) + if err != nil { + t.Fatalf("selectOrCreatePoolSessionBead: %v", err) + } + if result.ID != legacy.ID { + t.Fatalf("pool reuse ID = %q, want legacy bead %q", result.ID, legacy.ID) + } +} + func TestSelectOrCreatePoolSessionBead_SkipsAssignedForNewTier(t *testing.T) { store := beads.NewMemStore() assigned, err := store.Create(beads.Bead{ @@ -5839,6 +7676,106 @@ func TestEnsureDependencyOnlyTemplate_StoreBackedUsesInstanceIdentity(t *testing } } +func TestEnsureDependencyOnlyTemplate_StoreBackedMaxOneUsesCanonicalIdentity(t *testing.T) { + cityPath := t.TempDir() + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{ + { + Name: "db", + Dir: "gascity", + StartCommand: "true", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 0", + }, + { + Name: "api", + Dir: "gascity", + StartCommand: "true", + MinActiveSessions: intPtr(0), + MaxActiveSessions: intPtr(3), + ScaleCheck: "printf 0", + DependsOn: []string{"gascity/db"}, + }, + }, + } + + store := beads.NewMemStore() + if _, err := store.Create(beads.Bead{ + Title: "api", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "template:gascity/api"}, + Metadata: map[string]string{ + "template": "gascity/api", + "agent_name": "gascity/api", + "session_name": "s-api-root", + "state": "active", + poolManagedMetadataKey: boolMetadata(true), + "pool_slot": "1", + }, + }); err != nil { + t.Fatalf("seed api root bead: %v", err) + } + + dsResult := buildDesiredState("test-city", cityPath, time.Now().UTC(), cfg, runtime.NewFake(), store, io.Discard) + + var tp TemplateParams + var found bool + for _, entry := range dsResult.State { + if entry.TemplateName == "gascity/db" && entry.DependencyOnly { + tp = entry + found = true + break + } + } + if !found { + entries := make([]string, 0, len(dsResult.State)) + for k, v := range dsResult.State { + entries = append(entries, fmt.Sprintf("%s{template=%s depOnly=%v alias=%s}", k, v.TemplateName, v.DependencyOnly, v.Env["GC_ALIAS"])) + } + t.Fatalf("store-backed dependency floor for db not found, desired = %v", entries) + } + if alias := tp.Env["GC_ALIAS"]; alias != "gascity/db" { + t.Fatalf("store-backed dep-floor GC_ALIAS = %q, want canonical identity %q", alias, "gascity/db") + } + + sessionBeads, err := loadSessionBeads(store) + if err != nil { + t.Fatalf("load session beads: %v", err) + } + var depBead beads.Bead + found = false + for _, candidate := range sessionBeads { + if candidate.Metadata["template"] == "gascity/db" && candidate.Metadata["agent_name"] == "gascity/db" { + depBead = candidate + found = true + break + } + } + if !found { + t.Fatalf("dependency-floor bead for db not found; beads=%#v", sessionBeads) + } + if got := depBead.Metadata["agent_name"]; got != "gascity/db" { + t.Fatalf("dependency agent_name = %q, want canonical non-pool identity", got) + } + if got := depBead.Metadata["alias"]; got != "gascity/db" { + t.Fatalf("dependency alias = %q, want canonical non-pool identity", got) + } + if got := depBead.Metadata["pool_slot"]; got != "" { + t.Fatalf("dependency pool_slot = %q, want empty for max_active_sessions=1", got) + } + if depBead.Title != "gascity/db" { + t.Fatalf("dependency title = %q, want canonical non-pool identity", depBead.Title) + } + if containsString(depBead.Labels, "agent:gascity/db-1") { + t.Fatalf("dependency labels = %#v, must not include phantom pool identity", depBead.Labels) + } + if !containsString(depBead.Labels, "agent:gascity/db") { + t.Fatalf("dependency labels = %#v, want canonical agent label", depBead.Labels) + } +} + func TestBuildDesiredState_DependencyFloorSkipsFailedCreate(t *testing.T) { cityPath := t.TempDir() cfg := &config.City{ diff --git a/cmd/gc/cmd_agent.go b/cmd/gc/cmd_agent.go index 9802832406..f368b28ce8 100644 --- a/cmd/gc/cmd_agent.go +++ b/cmd/gc/cmd_agent.go @@ -336,7 +336,7 @@ func resolveAgentIdentity(cfg *config.City, input, currentRigDir string) (config func resolvePoolInstance(cfg *config.City, input string) (config.Agent, bool) { for _, a := range cfg.Agents { sp := scaleParamsFor(&a) - if !a.SupportsInstanceExpansion() { + if !a.SupportsInstanceExpansion() || a.UsesCanonicalSingletonPoolIdentity() { continue } prefix := a.QualifiedName() + "-" @@ -362,7 +362,7 @@ func resolvePoolInstance(cfg *config.City, input string) (config.Agent, bool) { // pattern (e.g., "polecat-2" matches agent "polecat"). Returns the synthesized instance. func matchPoolInstance(a config.Agent, input string) (config.Agent, bool) { sp := scaleParamsFor(&a) - if !a.SupportsInstanceExpansion() { + if !a.SupportsInstanceExpansion() || a.UsesCanonicalSingletonPoolIdentity() { return config.Agent{}, false } prefix := a.Name + "-" diff --git a/cmd/gc/cmd_agent_test.go b/cmd/gc/cmd_agent_test.go index 88412ca631..e35ea909a3 100644 --- a/cmd/gc/cmd_agent_test.go +++ b/cmd/gc/cmd_agent_test.go @@ -239,6 +239,23 @@ schema = 2 } } +func TestResolveAgentIdentityRejectsCanonicalSingletonPoolSuffix(t *testing.T) { + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "worker", Dir: "frontend", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1)}, + }, + } + if a, ok := resolveAgentIdentity(cfg, "frontend/worker", ""); !ok || a.QualifiedName() != "frontend/worker" { + t.Fatalf("resolveAgentIdentity(frontend/worker) = (%q, %v), want canonical template", a.QualifiedName(), ok) + } + if _, ok := resolveAgentIdentity(cfg, "frontend/worker-1", ""); ok { + t.Fatal("resolveAgentIdentity(frontend/worker-1) = true, want false for canonical singleton pool") + } + if _, ok := resolveAgentIdentity(cfg, "worker-1", ""); ok { + t.Fatal("resolveAgentIdentity(worker-1) = true, want false for canonical singleton pool") + } +} + func TestEmitLoadCityConfigWarningsFiltersNonMigrationWarnings(t *testing.T) { var stderr bytes.Buffer emitLoadCityConfigWarnings(&stderr, &config.Provenance{ diff --git a/cmd/gc/cmd_citystatus_test.go b/cmd/gc/cmd_citystatus_test.go index 12b024d50a..d46e63eac7 100644 --- a/cmd/gc/cmd_citystatus_test.go +++ b/cmd/gc/cmd_citystatus_test.go @@ -164,7 +164,7 @@ func TestCityStatusSuspended(t *testing.T) { } var stdout, stderr bytes.Buffer - code := doCityStatus(sp, dops, cfg, "/tmp/city", &stdout, &stderr) + code := doCityStatus(sp, dops, cfg, t.TempDir(), &stdout, &stderr) if code != 0 { t.Fatalf("code = %d, want 0", code) } @@ -194,7 +194,7 @@ func TestCityStatusPoolExpansion(t *testing.T) { } var stdout, stderr bytes.Buffer - code := doCityStatus(sp, dops, cfg, "/tmp/city", &stdout, &stderr) + code := doCityStatus(sp, dops, cfg, t.TempDir(), &stdout, &stderr) if code != 0 { t.Fatalf("code = %d, want 0; stderr: %s", code, stderr.String()) } @@ -224,6 +224,36 @@ func TestCityStatusPoolExpansion(t *testing.T) { } } +func TestCityStatusCanonicalSingletonPoolUsesCanonicalName(t *testing.T) { + sp := runtime.NewFake() + if err := sp.Start(context.Background(), "hw--refinery", runtime.Config{Command: "echo"}); err != nil { + t.Fatal(err) + } + dops := newFakeDrainOps() + cfg := &config.City{ + Workspace: config.Workspace{Name: "city"}, + Agents: []config.Agent{ + {Name: "refinery", Dir: "hw", MaxActiveSessions: intPtr(1), ScaleCheck: "echo 1"}, + }, + } + + var stdout, stderr bytes.Buffer + code := doCityStatus(sp, dops, cfg, t.TempDir(), &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr: %s", code, stderr.String()) + } + out := stdout.String() + if !strings.Contains(out, "hw/refinery") || !strings.Contains(out, "running") { + t.Fatalf("stdout missing canonical running singleton status, got:\n%s", out) + } + if strings.Contains(out, "hw/refinery-1") { + t.Fatalf("stdout contains phantom singleton instance, got:\n%s", out) + } + if !strings.Contains(out, "1/1 agents running") { + t.Fatalf("stdout missing canonical singleton running summary, got:\n%s", out) + } +} + func TestCityStatusRigs(t *testing.T) { sp := runtime.NewFake() dops := newFakeDrainOps() @@ -237,7 +267,7 @@ func TestCityStatusRigs(t *testing.T) { } var stdout, stderr bytes.Buffer - code := doCityStatus(sp, dops, cfg, "/tmp/city", &stdout, &stderr) + code := doCityStatus(sp, dops, cfg, t.TempDir(), &stdout, &stderr) if code != 0 { t.Fatalf("code = %d, want 0", code) } @@ -535,7 +565,7 @@ func TestCityStatusAgentSuspendedByRig(t *testing.T) { } var stdout, stderr bytes.Buffer - code := doCityStatus(sp, dops, cfg, "/tmp/city", &stdout, &stderr) + code := doCityStatus(sp, dops, cfg, t.TempDir(), &stdout, &stderr) if code != 0 { t.Fatalf("code = %d, want 0", code) } diff --git a/cmd/gc/cmd_config.go b/cmd/gc/cmd_config.go index caa914c338..e2051fc712 100644 --- a/cmd/gc/cmd_config.go +++ b/cmd/gc/cmd_config.go @@ -319,14 +319,14 @@ func singletonSessionMigrationWarnings(cfg *config.City) []string { var warnings []string for i := range cfg.Agents { agentCfg := &cfg.Agents[i] - if m := agentCfg.EffectiveMaxActiveSessions(); m == nil || *m != 1 { + if !agentCfg.UsesCanonicalSingletonPoolIdentity() { continue } if namedByTemplate[agentCfg.QualifiedName()] { continue } warnings = append(warnings, - fmt.Sprintf("agent %q: max_active_sessions=1 now limits ephemeral capacity but does not create a persistent singleton; declare [[named_session]] for a canonical session identity", agentCfg.QualifiedName())) + fmt.Sprintf("agent %q: max_active_sessions=1 creates a canonical singleton that drains when scale_check returns 0; declare [[named_session]] only if you need a session that survives empty-demand windows", agentCfg.QualifiedName())) } sort.Strings(warnings) return warnings diff --git a/cmd/gc/cmd_config_validation_test.go b/cmd/gc/cmd_config_validation_test.go index 4c4bd85fc1..8dff1b1697 100644 --- a/cmd/gc/cmd_config_validation_test.go +++ b/cmd/gc/cmd_config_validation_test.go @@ -29,6 +29,25 @@ func TestSingletonSessionMigrationWarnings_SkipsNamedBackedTemplates(t *testing. if !strings.Contains(warnings[0], `agent "worker"`) { t.Fatalf("warning = %q, want worker guidance", warnings[0]) } + if !strings.Contains(warnings[0], "creates a canonical singleton") { + t.Fatalf("warning = %q, want canonical singleton guidance", warnings[0]) + } + if strings.Contains(warnings[0], "does not create a persistent singleton") { + t.Fatalf("warning = %q, contains stale singleton guidance", warnings[0]) + } +} + +func TestSingletonSessionMigrationWarnings_SkipsNamepoolSingleton(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{ + {Name: "worker", MaxActiveSessions: intPtr(1), NamepoolNames: []string{"alpha"}}, + }, + } + + if warnings := singletonSessionMigrationWarnings(cfg); len(warnings) != 0 { + t.Fatalf("warnings = %v, want none for namepool-backed max-one agent", warnings) + } } func TestValidateLegacyFormulaConfigRoutes_RejectsTemplateAssignee(t *testing.T) { diff --git a/cmd/gc/cmd_prime.go b/cmd/gc/cmd_prime.go index 16e44f058e..5157723f8a 100644 --- a/cmd/gc/cmd_prime.go +++ b/cmd/gc/cmd_prime.go @@ -580,7 +580,7 @@ func findAgentByName(cfg *config.City, name string) (config.Agent, bool) { } // Pool suffix stripping: "polecat-3" → try "polecat" if it's a pool. for _, a := range cfg.Agents { - if a.SupportsInstanceExpansion() { + if a.SupportsInstanceExpansion() && !a.UsesCanonicalSingletonPoolIdentity() { sp := scaleParamsFor(&a) prefix := a.Name + "-" if strings.HasPrefix(name, prefix) { diff --git a/cmd/gc/cmd_runtime_drain_test.go b/cmd/gc/cmd_runtime_drain_test.go index c3fd699ca4..d8f9077f13 100644 --- a/cmd/gc/cmd_runtime_drain_test.go +++ b/cmd/gc/cmd_runtime_drain_test.go @@ -1007,12 +1007,10 @@ func TestResolveAgentIdentity(t *testing.T) { // Pool instance negative (parsed as non-numeric due to dash). {"pool instance worker--1", "worker--1", false, "", false, ""}, - // Max=1 POOL agent (has MinActiveSessions or ScaleCheck): keeps the - // `-N` suffix per the new disambiguator. {name}-1 resolves to the - // pool instance with PoolName=base. Contrast with max=1 NAMED-session - // agents (no MinActiveSessions, no ScaleCheck) which collapse to the - // base identity — see TestBuildDesiredState_OnDemandNamedSession_NoPhantomPoolInstance. - {"singleton-1 matches max=1 pool", "singleton-1", true, "singleton-1", true, "singleton"}, + // Max=1 pool-shaped agents still run through pool reconciliation, but + // non-namepool singleton pools use the canonical configured identity. + // A numeric suffix would materialize a phantom concrete agent. + {"singleton-1 rejected for canonical max=1 pool", "singleton-1", false, "", false, ""}, // Nonexistent agent. {"nonexistent", "nobody", false, "", false, ""}, @@ -1216,23 +1214,14 @@ func TestFindAgentByNamePoolOutOfRange(t *testing.T) { } } -func TestFindAgentByNameSingletonPoolMatches(t *testing.T) { +func TestFindAgentByNameSingletonPoolRejectsSuffix(t *testing.T) { cfg := &config.City{ Agents: []config.Agent{ {Name: "singleton", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1), ScaleCheck: "echo 1"}, }, } - // Max=1 POOL agents (with MinActiveSessions or ScaleCheck set) keep the - // `-N` suffix per the disambiguator in SupportsInstanceExpansion. The - // instance name `singleton-1` resolves to the base agent. Contrast with - // max=1 NAMED-session agents (neither field set) which collapse to the - // base identity. - a, ok := findAgentByName(cfg, "singleton-1") - if !ok { - t.Fatal("singleton-1 should match the max=1 pool agent") - } - if a.Name != "singleton" { - t.Errorf("agent.Name = %q, want %q", a.Name, "singleton") + if _, ok := findAgentByName(cfg, "singleton-1"); ok { + t.Fatal("singleton-1 should not match a canonical singleton pool agent") } } diff --git a/cmd/gc/cmd_start.go b/cmd/gc/cmd_start.go index e048cc11b6..967ba72c8c 100644 --- a/cmd/gc/cmd_start.go +++ b/cmd/gc/cmd_start.go @@ -1282,14 +1282,9 @@ func countRunningPoolInstances(agentName, agentDir string, sp0 scaleParams, a *c return count } - // Bounded: build the set of expected pool instance session names. + // Bounded: build the set of expected pool runtime session names. expected := make(map[string]bool, sp0.Max) - for i := 1; i <= sp0.Max; i++ { - instanceName := poolInstanceName(agentName, i, a) - qualifiedInstance := instanceName - if agentDir != "" { - qualifiedInstance = agentDir + "/" + instanceName - } + for _, qualifiedInstance := range discoverPoolInstances(agentName, agentDir, sp0, a, cityName, sessionTemplate, sp) { expected[sessionName(nil, cityName, qualifiedInstance, sessionTemplate)] = true } diff --git a/cmd/gc/cmd_start_test.go b/cmd/gc/cmd_start_test.go index aeaa3fcfb3..98d1123c25 100644 --- a/cmd/gc/cmd_start_test.go +++ b/cmd/gc/cmd_start_test.go @@ -1,6 +1,7 @@ package main import ( + "context" "io" "os" "path" @@ -94,6 +95,153 @@ func TestComputePoolSessions_NamepoolMaxOneUsesPoolInstance(t *testing.T) { } } +func TestComputePoolSessions_CanonicalSingletonUsesCanonicalSessionName(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{}, + Agents: []config.Agent{ + { + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "echo 1", + DrainTimeout: "2m", + }, + }, + } + + got := computePoolSessions(cfg, "city", "", runtime.NewFake()) + want := startupSessionName("city", "cashmaster/refinery", cfg.Workspace.SessionTemplate) + if _, ok := got[want]; !ok { + t.Fatalf("computePoolSessions missing %q in %v", want, got) + } + if _, ok := got[startupSessionName("city", "cashmaster/refinery-1", cfg.Workspace.SessionTemplate)]; ok { + t.Fatalf("computePoolSessions registered phantom singleton instance in %v", got) + } + if len(got) != 1 { + t.Fatalf("computePoolSessions len = %d, want 1 (%v)", len(got), got) + } +} + +func TestBuildLifecycleTrackers_CanonicalSingletonUsesCanonicalSessionName(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{}, + Agents: []config.Agent{ + { + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "echo 1", + IdleTimeout: "5m", + MaxSessionAge: "1h", + }, + }, + } + canonical := startupSessionName("city", "cashmaster/refinery", cfg.Workspace.SessionTemplate) + phantom := startupSessionName("city", "cashmaster/refinery-1", cfg.Workspace.SessionTemplate) + + idle, ok := buildIdleTracker(cfg, "city", "", runtime.NewFake()).(*memoryIdleTracker) + if !ok { + t.Fatalf("buildIdleTracker returned %T, want *memoryIdleTracker", idle) + } + if _, ok := idle.timeouts[canonical]; !ok { + t.Fatalf("idle tracker missing canonical session %q in %v", canonical, idle.timeouts) + } + if _, ok := idle.timeouts[phantom]; ok { + t.Fatalf("idle tracker registered phantom singleton instance %q in %v", phantom, idle.timeouts) + } + + maxAge, ok := buildMaxSessionAgeTracker(cfg, "city", runtime.NewFake()).(*memoryMaxSessionAgeTracker) + if !ok { + t.Fatalf("buildMaxSessionAgeTracker returned %T, want *memoryMaxSessionAgeTracker", maxAge) + } + if _, ok := maxAge.configs[canonical]; !ok { + t.Fatalf("max-age tracker missing canonical session %q in %v", canonical, maxAge.configs) + } + if _, ok := maxAge.configs[phantom]; ok { + t.Fatalf("max-age tracker registered phantom singleton instance %q in %v", phantom, maxAge.configs) + } +} + +func TestBuildLifecycleTrackers_CanonicalSingletonIncludesLiveStaleSuffix(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{}, + Agents: []config.Agent{ + { + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "echo 1", + IdleTimeout: "5m", + MaxSessionAge: "1h", + }, + }, + } + stale := startupSessionName("city", "cashmaster/refinery-1", cfg.Workspace.SessionTemplate) + sp := runtime.NewFake() + if err := sp.Start(context.Background(), stale, runtime.Config{}); err != nil { + t.Fatalf("Start(%q): %v", stale, err) + } + + idle, ok := buildIdleTracker(cfg, "city", "", sp).(*memoryIdleTracker) + if !ok { + t.Fatalf("buildIdleTracker returned %T, want *memoryIdleTracker", idle) + } + if _, ok := idle.timeouts[stale]; !ok { + t.Fatalf("idle tracker missing live stale singleton suffix %q in %v", stale, idle.timeouts) + } + + maxAge, ok := buildMaxSessionAgeTracker(cfg, "city", sp).(*memoryMaxSessionAgeTracker) + if !ok { + t.Fatalf("buildMaxSessionAgeTracker returned %T, want *memoryMaxSessionAgeTracker", maxAge) + } + if _, ok := maxAge.configs[stale]; !ok { + t.Fatalf("max-age tracker missing live stale singleton suffix %q in %v", stale, maxAge.configs) + } +} + +func TestBuildLifecycleTrackers_CanonicalSingletonNamedSessionOverlayOneKey(t *testing.T) { + cfg := &config.City{ + Workspace: config.Workspace{}, + Agents: []config.Agent{ + { + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "echo 1", + IdleTimeout: "5m", + MaxSessionAge: "1h", + }, + }, + NamedSessions: []config.NamedSession{{ + Template: "refinery", + Dir: "cashmaster", + }}, + } + canonical := startupSessionName("city", "cashmaster/refinery", cfg.Workspace.SessionTemplate) + + idle, ok := buildIdleTracker(cfg, "city", "", runtime.NewFake()).(*memoryIdleTracker) + if !ok { + t.Fatalf("buildIdleTracker returned %T, want *memoryIdleTracker", idle) + } + if len(idle.timeouts) != 1 { + t.Fatalf("idle tracker registered %d keys, want 1: %v", len(idle.timeouts), idle.timeouts) + } + if _, ok := idle.timeouts[canonical]; !ok { + t.Fatalf("idle tracker missing canonical session %q in %v", canonical, idle.timeouts) + } + + maxAge, ok := buildMaxSessionAgeTracker(cfg, "city", runtime.NewFake()).(*memoryMaxSessionAgeTracker) + if !ok { + t.Fatalf("buildMaxSessionAgeTracker returned %T, want *memoryMaxSessionAgeTracker", maxAge) + } + if len(maxAge.configs) != 1 { + t.Fatalf("max-age tracker registered %d keys, want 1: %v", len(maxAge.configs), maxAge.configs) + } + if _, ok := maxAge.configs[canonical]; !ok { + t.Fatalf("max-age tracker missing canonical session %q in %v", canonical, maxAge.configs) + } +} + func TestStandaloneBuildAgentsFnWithSessionBeads_UsesRigStoresForAssignedWork(t *testing.T) { cityStore := beads.NewMemStore() rigStore := beads.NewMemStore() diff --git a/cmd/gc/cmd_status_test.go b/cmd/gc/cmd_status_test.go index f6c598e0fe..d5cab2fea2 100644 --- a/cmd/gc/cmd_status_test.go +++ b/cmd/gc/cmd_status_test.go @@ -123,6 +123,31 @@ func TestDoRigStatusWithDraining(t *testing.T) { } } +func TestDoRigStatusCanonicalSingletonPoolUsesCanonicalName(t *testing.T) { + sp := runtime.NewFake() + if err := sp.Start(context.Background(), "frontend--refinery", runtime.Config{Command: "echo"}); err != nil { + t.Fatal(err) + } + dops := newFakeDrainOps() + rig := config.Rig{Name: "frontend", Path: "/tmp/frontend"} + agents := []config.Agent{ + {Name: "refinery", Dir: "frontend", MaxActiveSessions: intPtr(1), ScaleCheck: "echo 1"}, + } + + var stdout, stderr bytes.Buffer + code := runDoRigStatus(sp, dops, rig, agents, "", &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr: %s", code, stderr.String()) + } + out := stdout.String() + if !strings.Contains(out, "frontend/refinery") || !strings.Contains(out, "running") { + t.Fatalf("stdout missing canonical running singleton status, got:\n%s", out) + } + if strings.Contains(out, "frontend/refinery-1") { + t.Fatalf("stdout contains phantom singleton instance, got:\n%s", out) + } +} + func TestDoRigStatusSuspendedAgent(t *testing.T) { sp := runtime.NewFake() dops := newFakeDrainOps() diff --git a/cmd/gc/main_test.go b/cmd/gc/main_test.go index c3b5bfb5b1..80842f588e 100644 --- a/cmd/gc/main_test.go +++ b/cmd/gc/main_test.go @@ -1190,6 +1190,28 @@ func TestFindSessionNameByTemplate_SkipsPoolSlotBeads(t *testing.T) { } } +func TestLookupSessionNameOrLegacy_CanonicalSingletonPoolManagedBead(t *testing.T) { + store := beads.NewMemStore() + if _, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "session_name": "s-canonical-refinery", + poolManagedMetadataKey: boolMetadata(true), + }, + }); err != nil { + t.Fatal(err) + } + + got := lookupSessionNameOrLegacy(store, "city", "cashmaster/refinery", "") + if got != "s-canonical-refinery" { + t.Fatalf("lookupSessionNameOrLegacy(canonical singleton pool bead) = %q, want s-canonical-refinery", got) + } +} + func TestFindSessionNameByTemplate_SkipsEmptySessionName(t *testing.T) { store := beads.NewMemStore() _, err := store.Create(beads.Bead{ @@ -1532,6 +1554,40 @@ func TestLookupPoolSessionNames_RejectsSharedPrefixSiblingTemplates(t *testing.T } } +func TestLookupPoolSessionNames_AcceptsCanonicalSingletonPoolManagedBead(t *testing.T) { + store := beads.NewMemStore() + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "refinery", Dir: "cashmaster", MaxActiveSessions: intPtr(1), ScaleCheck: "printf 1"}, + }, + } + cfgAgent := &cfg.Agents[0] + if _, err := store.Create(beads.Bead{ + Title: "cashmaster/refinery", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "session_name": "s-canonical-refinery", + poolManagedMetadataKey: boolMetadata(true), + }, + }); err != nil { + t.Fatal(err) + } + + got, err := lookupPoolSessionNames(store, cfg, cfgAgent) + if err != nil { + t.Fatalf("lookupPoolSessionNames: %v", err) + } + if got["cashmaster/refinery"] != "s-canonical-refinery" { + t.Fatalf("lookupPoolSessionNames(canonical singleton) = %#v, want canonical session", got) + } + if _, ok := got["cashmaster/refinery-1"]; ok { + t.Fatalf("lookupPoolSessionNames(canonical singleton) registered phantom slot: %#v", got) + } +} + func TestLookupPoolSessionNames_PreservesUniqueLegacyLocalSessionNameIdentity(t *testing.T) { store := beads.NewMemStore() cfg := &config.City{ @@ -2152,6 +2208,46 @@ func TestSelectRunningPoolSessionRefs_ReturnsAllLiveCandidatesForLogicalInstance } } +func TestSelectRunningPoolSessionRefs_CanonicalSingletonIncludesStaleBeadBackedSuffix(t *testing.T) { + store := beads.NewMemStore() + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "worker", Dir: "frontend", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1)}, + }, + } + agentCfg := cfg.Agents[0] + if _, err := store.Create(beads.Bead{ + Title: "stale singleton pool instance", + Type: sessionBeadType, + Labels: []string{sessionBeadLabel, "agent:frontend/worker-1", "template:frontend/worker"}, + Metadata: map[string]string{ + "template": "frontend/worker", + "agent_name": "frontend/worker-1", + "session_name": "s-stale-worker", + "pool_slot": "1", + poolManagedMetadataKey: boolMetadata(true), + "state": "active", + }, + }); err != nil { + t.Fatal(err) + } + sp := runtime.NewFake() + if err := sp.Start(context.Background(), "s-stale-worker", runtime.Config{Command: "echo"}); err != nil { + t.Fatal(err) + } + + refs, err := selectRunningPoolSessionRefs(store, sp, cfg, resolvePoolSessionRefs(store, cfg, agentCfg.Name, agentCfg.Dir, scaleParamsFor(&agentCfg), &agentCfg, "test-city", "", sp, io.Discard)) + if err != nil { + t.Fatalf("selectRunningPoolSessionRefs: %v", err) + } + for _, ref := range refs { + if ref.qualifiedInstance == "frontend/worker-1" && ref.sessionName == "s-stale-worker" { + return + } + } + t.Fatalf("selectRunningPoolSessionRefs() = %#v, want stale bead-backed singleton suffix ref", refs) +} + func TestSelectRunningPoolSessionRefs_ReportsConcreteSessionOnProbeFailure(t *testing.T) { cfg := &config.City{ Agents: []config.Agent{ diff --git a/cmd/gc/pool.go b/cmd/gc/pool.go index e784985d1f..b977907b1e 100644 --- a/cmd/gc/pool.go +++ b/cmd/gc/pool.go @@ -421,8 +421,9 @@ func runPoolOnBoot(cfg *config.City, cityPath string, runner ScaleCheckRunner, s } } -// discoverPoolInstances returns qualified instance names for a multi-instance pool. -// For bounded pools (max > 1), generates static names {name}-1..{name}-{max}. +// discoverPoolInstances returns qualified runtime identities for a pool-shaped +// agent. Canonical singleton pools use the configured qualified name. Bounded +// multi-instance pools generate static names {name}-1..{name}-{max}. // For unlimited pools (max < 0), discovers running instances via session provider // prefix matching. func discoverPoolInstances(agentName, agentDir string, sp0 scaleParams, a *config.Agent, @@ -430,6 +431,9 @@ func discoverPoolInstances(agentName, agentDir string, sp0 scaleParams, a *confi ) []string { isUnlimited := sp0.Max < 0 if !isUnlimited { + if a.UsesCanonicalSingletonPoolIdentity() { + return discoverCanonicalSingletonPoolInstances(a, cityName, st, sp) + } // Bounded pool: static enumeration. var names []string for i := 1; i <= sp0.Max; i++ { @@ -481,6 +485,42 @@ func discoverPoolInstances(agentName, agentDir string, sp0 scaleParams, a *confi return names } +func discoverCanonicalSingletonPoolInstances(a *config.Agent, cityName, st string, sp runtime.Provider) []string { + if a == nil { + return nil + } + canonical := a.QualifiedName() + names := []string{canonical} + if sp == nil { + return names + } + prefix := agent.SessionNameFor(cityName, canonical+"-", st) + running, err := sp.ListRunning("") + if err != nil { + return names + } + templatePrefix := agent.SessionNameFor(cityName, "", st) + stale := make([]string, 0, len(running)) + seen := map[string]bool{canonical: true} + for _, sn := range running { + if !strings.HasPrefix(sn, prefix) { + continue + } + qnSanitized := sn + if templatePrefix != "" && strings.HasPrefix(qnSanitized, templatePrefix) { + qnSanitized = qnSanitized[len(templatePrefix):] + } + qn := agent.UnsanitizeQualifiedNameFromSession(qnSanitized) + if seen[qn] || nonExpandingPoolIdentitySlot(a, qn) <= 0 { + continue + } + seen[qn] = true + stale = append(stale, qn) + } + sort.Strings(stale) + return append(names, stale...) +} + func resolvePoolSessionRefs( store beads.Store, cfg *config.City, diff --git a/cmd/gc/pool_test.go b/cmd/gc/pool_test.go index f050083db3..6b9056985f 100644 --- a/cmd/gc/pool_test.go +++ b/cmd/gc/pool_test.go @@ -363,6 +363,22 @@ func TestDiscoverPoolInstancesBoundedWithNamepool(t *testing.T) { } } +func TestDiscoverPoolInstancesCanonicalSingletonUsesBaseName(t *testing.T) { + sp := runtime.NewFake() + a := &config.Agent{ + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "echo 1", + } + pool := scaleParams{Min: 0, Max: 1} + instances := discoverPoolInstances("refinery", "cashmaster", pool, a, "city", "", sp) + want := []string{"cashmaster/refinery"} + if !reflect.DeepEqual(instances, want) { + t.Fatalf("instances = %v, want %v", instances, want) + } +} + func TestDiscoverPoolInstancesUnlimited(t *testing.T) { sp := runtime.NewFake() // Start some instances that look like pool members. @@ -1112,15 +1128,14 @@ func TestComputePoolDeathHandlers(t *testing.T) { Agents: []config.Agent{ {Name: "mayor", MaxActiveSessions: intPtr(1)}, // not a pool (no MinActiveSessions/ScaleCheck) {Name: "dog", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(3), OnDeath: "echo death"}, - {Name: "cat", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1), OnDeath: "echo death"}, // max=1 POOL (MinActiveSessions set) — keeps cat-1 per the disambiguator + {Name: "cat", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1), OnDeath: "echo death"}, // max=1 canonical singleton pool }, } handlers := computePoolDeathHandlers(cfg, "test", t.TempDir(), runtime.NewFake(), nil) // dog has max=3, so 3 handlers (dog-1, dog-2, dog-3). - // cat is a max=1 POOL agent (MinActiveSessions set), so cat-1 keeps the - // `-N` suffix per SupportsInstanceExpansion's disambiguator — 1 handler. + // cat is a max=1 canonical singleton pool agent, so the handler uses cat. // mayor is not a pool (neither MinActiveSessions nor ScaleCheck set). if len(handlers) != 4 { t.Fatalf("len(handlers) = %d, want 4", len(handlers)) @@ -1139,11 +1154,10 @@ func TestComputePoolDeathHandlers(t *testing.T) { } } - // cat-1 handler (max=1 pool keeps the suffix) - if info, ok := handlers["cat-1"]; !ok { - t.Errorf("missing handler for cat-1 (have keys: %v)", handlerKeys(handlers)) + if info, ok := handlers["cat"]; !ok { + t.Errorf("missing handler for cat (have keys: %v)", handlerKeys(handlers)) } else if !strings.Contains(info.Command, "echo death") { - t.Errorf("handler[cat-1].Command = %q, want configured on_death command", info.Command) + t.Errorf("handler[cat].Command = %q, want configured on_death command", info.Command) } } diff --git a/cmd/gc/session_bead_snapshot.go b/cmd/gc/session_bead_snapshot.go index 89f73f60d2..d6ff5b8bee 100644 --- a/cmd/gc/session_bead_snapshot.go +++ b/cmd/gc/session_bead_snapshot.go @@ -91,7 +91,11 @@ func newSessionBeadSnapshot(beadsIn []beads.Bead) *sessionBeadSnapshot { isCanonicalNamed := strings.TrimSpace(b.Metadata["configured_named_identity"]) != "" if agentName := sessionBeadAgentName(b); agentName != "" { if isPoolManagedSessionBead(b) && agentName == b.Metadata["template"] { - agentName = stampedPoolQualifiedIdentity(b) + if stamped := stampedPoolQualifiedIdentity(b); stamped != "" { + agentName = stamped + } else if !isCanonicalPoolManagedSessionBeadForTemplate(b, agentName) { + agentName = "" + } } if agentName == "" { continue diff --git a/cmd/gc/session_bead_snapshot_test.go b/cmd/gc/session_bead_snapshot_test.go index a3b9aa5d52..36327683cf 100644 --- a/cmd/gc/session_bead_snapshot_test.go +++ b/cmd/gc/session_bead_snapshot_test.go @@ -84,3 +84,30 @@ func BenchmarkLoadSessionBeadSnapshot_OpenOnlyBaseline(b *testing.B) { } } } + +func TestSessionBeadSnapshotIndexesCanonicalSingletonPoolManagedBead(t *testing.T) { + snapshot := newSessionBeadSnapshot([]beads.Bead{{ + ID: "refinery-session", + Title: "cashmaster/refinery", + Type: sessionBeadType, + Status: "open", + Labels: []string{sessionBeadLabel, "agent:cashmaster/refinery"}, + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery", + "session_name": "s-canonical-refinery", + poolManagedMetadataKey: boolMetadata(true), + }, + }}) + + if got := snapshot.FindSessionNameByTemplate("cashmaster/refinery"); got != "s-canonical-refinery" { + t.Fatalf("FindSessionNameByTemplate(canonical singleton pool bead) = %q, want s-canonical-refinery", got) + } + bead, ok := snapshot.FindSessionBeadByTemplate("cashmaster/refinery") + if !ok { + t.Fatal("FindSessionBeadByTemplate(canonical singleton pool bead) = false") + } + if bead.ID != "refinery-session" { + t.Fatalf("FindSessionBeadByTemplate ID = %q, want refinery-session", bead.ID) + } +} diff --git a/cmd/gc/session_beads.go b/cmd/gc/session_beads.go index 58257d989d..2834bf8681 100644 --- a/cmd/gc/session_beads.go +++ b/cmd/gc/session_beads.go @@ -1156,6 +1156,12 @@ func syncSessionBeadsWithSnapshotAndRigStores( queueMeta("pool_slot", "") } } + if managedAlias == "" && isManagedPool && !isPoolInstance && isPoolManagedSessionBead(b) { + conflictAlias := strings.TrimSpace(b.Metadata[poolAliasConflictMetadataKey]) + if cfgAgent := findAgentByTemplate(cfg, tp.TemplateName); cfgAgent != nil && cfgAgent.UsesCanonicalSingletonPoolIdentity() && conflictAlias == cfgAgent.QualifiedName() { + managedAlias = conflictAlias + } + } needsAliasSync := b.Metadata["alias"] != managedAlias if b.Metadata["pool_slot"] == "" { queuePoolSlotMeta := queueMeta @@ -1298,6 +1304,9 @@ func syncSessionBeadsWithSnapshotAndRigStores( } } clearAliasConflict := func() { + wasConflicted := b.Metadata[poolAliasConflictMetadataKey] != "" || + b.Metadata[poolAliasConflictCountMetadataKey] != "" || + b.Metadata[poolAliasConflictAtMetadataKey] != "" if b.Metadata[poolAliasConflictMetadataKey] != "" { queueMeta(poolAliasConflictMetadataKey, "") } @@ -1307,12 +1316,17 @@ func syncSessionBeadsWithSnapshotAndRigStores( if b.Metadata[poolAliasConflictAtMetadataKey] != "" { queueMeta(poolAliasConflictAtMetadataKey, "") } + if wasConflicted { + fmt.Fprintf(stderr, "session beads: clearing alias conflict for %s\n", agentName) //nolint:errcheck + } } recordAliasConflict := func() { count := 0 if existing, err := strconv.Atoi(strings.TrimSpace(b.Metadata[poolAliasConflictCountMetadataKey])); err == nil && existing > 0 { count = existing } + // This is a retry counter across build-time normalization and + // sync-time alias recovery, not a one-increment-per-tick gauge. queueMeta(poolAliasConflictMetadataKey, managedAlias) queueMeta(poolAliasConflictCountMetadataKey, strconv.Itoa(count+1)) queueMeta(poolAliasConflictAtMetadataKey, now.Format(time.RFC3339)) @@ -1465,6 +1479,9 @@ func syncDesiredPoolSlots( if agentCfg == nil || !agentCfg.SupportsInstanceExpansion() { continue } + if agentCfg.UsesCanonicalSingletonPoolIdentity() { + continue + } desiredByTemplate[tp.TemplateName] = append(desiredByTemplate[tp.TemplateName], sn) } diff --git a/cmd/gc/session_name_lookup.go b/cmd/gc/session_name_lookup.go index 74daa0d496..1f43b53572 100644 --- a/cmd/gc/session_name_lookup.go +++ b/cmd/gc/session_name_lookup.go @@ -31,6 +31,20 @@ func isPoolManagedSessionBead(bead beads.Bead) bool { return strings.TrimSpace(bead.Metadata["pool_slot"]) != "" } +// isCanonicalPoolManagedSessionBeadForTemplate is the bead-shape companion to +// config.Agent.UsesCanonicalSingletonPoolIdentity: pool-managed, no pool slot, +// and canonical identity according to beadIdentifiesAsCanonical. +func isCanonicalPoolManagedSessionBeadForTemplate(bead beads.Bead, template string) bool { + template = strings.TrimSpace(template) + if template == "" || !isPoolManagedSessionBead(bead) { + return false + } + if strings.TrimSpace(bead.Metadata["pool_slot"]) != "" { + return false + } + return beadIdentifiesAsCanonical(bead, template) +} + func resolveLegacyPoolTemplate(cfg *config.City, storedTemplate string) string { storedTemplate = strings.TrimSpace(storedTemplate) if cfg == nil || storedTemplate == "" { @@ -288,7 +302,7 @@ func findSessionNameByAgentLabel(store beads.Store, template string) string { if err != nil { return "" } - return chooseSessionNameForTemplate(store, items, true, "", "") + return chooseSessionNameForTemplate(store, items, true, "", "", template) } func findSessionNameByMetadata(store beads.Store, key, value string, agentNameMatch bool) string { @@ -296,11 +310,12 @@ func findSessionNameByMetadata(store beads.Store, key, value string, agentNameMa if err != nil { return "" } - return chooseSessionNameForTemplate(store, items, agentNameMatch, key, value) + return chooseSessionNameForTemplate(store, items, agentNameMatch, key, value, value) } -func chooseSessionNameForTemplate(store beads.Store, items []beads.Bead, agentNameMatch bool, key, value string) string { +func chooseSessionNameForTemplate(store beads.Store, items []beads.Bead, agentNameMatch bool, key, value, queryTemplate string) string { var fallback string + var canonicalPoolFallback string for _, b := range items { if !sessionpkg.IsSessionBeadOrRepairable(b) || b.Status == "closed" { continue @@ -309,7 +324,8 @@ func chooseSessionNameForTemplate(store beads.Store, items []beads.Bead, agentNa if key != "" && strings.TrimSpace(b.Metadata[key]) != value { continue } - if agentNameMatch && isPoolManagedSessionBead(b) && sessionBeadAgentName(b) == b.Metadata["template"] { + canonicalPoolManaged := isCanonicalPoolManagedSessionBeadForTemplate(b, queryTemplate) + if agentNameMatch && isPoolManagedSessionBead(b) && sessionBeadAgentName(b) == b.Metadata["template"] && !canonicalPoolManaged { continue } if !agentNameMatch && isPoolManagedSessionBead(b) { @@ -322,10 +338,19 @@ func chooseSessionNameForTemplate(store beads.Store, items []beads.Bead, agentNa if strings.TrimSpace(b.Metadata["configured_named_identity"]) != "" { return sessionName } + if canonicalPoolManaged { + if canonicalPoolFallback == "" { + canonicalPoolFallback = sessionName + } + continue + } if fallback == "" { fallback = sessionName } } + if fallback == "" { + return canonicalPoolFallback + } return fallback } @@ -428,7 +453,7 @@ func lookupPoolSessionNameCandidates(store beads.Store, template string, cfg *co if storedTemplateMatches && strings.TrimSpace(b.Metadata["alias"]) == "" && !beadOwnsPoolSessionName(b) { sessionNameSlot = resolveSlot(sessionName) } - if cfgAgent != nil && poolSlotHasConfiguredBound(cfgAgent) { + if cfgAgent != nil && poolSlotHasConfiguredBound(cfgAgent) && !cfgAgent.UsesCanonicalSingletonPoolIdentity() { if agentSlot > 0 && !inBoundsPoolSlot(cfgAgent, agentSlot) { agentSlot = 0 } @@ -446,7 +471,31 @@ func lookupPoolSessionNameCandidates(store beads.Store, template string, cfg *co continue } agentName := sessionBeadAgentName(b) - if storedTemplateMatches && (agentName == template || agentName == targetBasename(template)) { + canonicalPoolManaged := cfgAgent.UsesCanonicalSingletonPoolIdentity() && isCanonicalPoolManagedSessionBeadForTemplate(b, template) + staleCanonicalSingletonSlot := 0 + if cfgAgent.UsesCanonicalSingletonPoolIdentity() && isPoolManagedSessionBead(b) && !canonicalPoolManaged { + switch { + case agentSlot > 0: + staleCanonicalSingletonSlot = agentSlot + case aliasSlot > 0: + staleCanonicalSingletonSlot = aliasSlot + case sessionNameSlot > 0: + staleCanonicalSingletonSlot = sessionNameSlot + default: + if slot, err := strconv.Atoi(strings.TrimSpace(b.Metadata["pool_slot"])); err == nil && slot > 0 { + staleCanonicalSingletonSlot = slot + } + } + if staleCanonicalSingletonSlot == 0 { + continue + } + } + switch { + case canonicalPoolManaged: + agentName = template + case staleCanonicalSingletonSlot > 0: + agentName = qualifiedInstanceName(staleCanonicalSingletonSlot) + case storedTemplateMatches && (agentName == template || agentName == targetBasename(template)): agentName = "" } switch { diff --git a/cmd/gc/session_name_lookup_test.go b/cmd/gc/session_name_lookup_test.go index 4b1e9c5b66..8f44490aac 100644 --- a/cmd/gc/session_name_lookup_test.go +++ b/cmd/gc/session_name_lookup_test.go @@ -107,3 +107,30 @@ func TestExistingPoolSlotWithConfig_PrefersConcreteAgentIdentityOverStaleSlot(t t.Fatalf("existingPoolSlotWithConfig = %d, want concrete agent slot 3 over stale slot/foreign alias", got) } } + +func TestExistingPoolSlot_CanonicalSingletonReturnsZero(t *testing.T) { + cfg := &config.City{ + Agents: []config.Agent{{ + Name: "refinery", + Dir: "cashmaster", + MaxActiveSessions: intPtr(1), + ScaleCheck: "printf 1", + }}, + } + cfgAgent := &cfg.Agents[0] + bead := beads.Bead{ + Metadata: map[string]string{ + "template": "cashmaster/refinery", + "agent_name": "cashmaster/refinery-1", + "alias": "cashmaster/refinery-1", + "pool_slot": "1", + }, + } + + if got := existingPoolSlot(cfgAgent, bead); got != 0 { + t.Fatalf("existingPoolSlot(canonical singleton) = %d, want 0", got) + } + if got := existingPoolSlotWithConfig(cfg, cfgAgent, bead); got != 0 { + t.Fatalf("existingPoolSlotWithConfig(canonical singleton) = %d, want 0", got) + } +} diff --git a/internal/agentutil/pool.go b/internal/agentutil/pool.go index b2b2e83f49..54d044cfd4 100644 --- a/internal/agentutil/pool.go +++ b/internal/agentutil/pool.go @@ -93,7 +93,7 @@ func ExpandAgents(agents []config.Agent, cityName, sessTmpl string, sp SessionLi } func expandSingleAgent(a config.Agent, cityName, sessTmpl string, sp SessionLister) []ExpandedAgent { - if !a.SupportsInstanceExpansion() { + if !a.SupportsExpandedSessionIdentities() { return []ExpandedAgent{{ QualifiedName: a.QualifiedName(), Rig: a.Dir, @@ -170,7 +170,7 @@ func discoverUnlimitedPool(a config.Agent, poolName, cityName, sessTmpl string, // PoolInstanceName returns the display name for a pool member at the given slot. // Uses namepool_names if configured, otherwise "{base}-{slot}". func PoolInstanceName(base string, slot int, a config.Agent) string { - if !a.SupportsInstanceExpansion() { + if !a.SupportsInstanceExpansion() || a.UsesCanonicalSingletonPoolIdentity() { return base } if slot >= 1 && slot <= len(a.NamepoolNames) { diff --git a/internal/agentutil/pool_test.go b/internal/agentutil/pool_test.go index 67490f5f4d..2cc242c676 100644 --- a/internal/agentutil/pool_test.go +++ b/internal/agentutil/pool_test.go @@ -58,6 +58,19 @@ func TestExpandAgentsBoundedPool(t *testing.T) { } } +func TestExpandAgentsCanonicalSingletonPoolUsesBaseName(t *testing.T) { + agents := []config.Agent{ + {Name: "worker", Dir: "myrig", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1)}, + } + result := ExpandAgents(agents, "city", "", nil) + if len(result) != 1 { + t.Fatalf("got %d agents, want 1", len(result)) + } + if result[0].QualifiedName != "myrig/worker" { + t.Errorf("name = %q, want myrig/worker", result[0].QualifiedName) + } +} + func TestExpandAgentsNamepool(t *testing.T) { agents := []config.Agent{ { @@ -124,6 +137,11 @@ func TestPoolInstanceName(t *testing.T) { t.Errorf("single instance: got %q, want polecat", got) } + a2.MinActiveSessions = intPtr(0) + if got := PoolInstanceName("polecat", 1, a2); got != "polecat" { + t.Errorf("canonical singleton pool: got %q, want polecat", got) + } + a3 := config.Agent{ Name: "polecat", MaxActiveSessions: intPtr(2), NamepoolNames: []string{"alpha", "beta"}, diff --git a/internal/agentutil/resolve.go b/internal/agentutil/resolve.go index d62134e024..9ad556a01e 100644 --- a/internal/agentutil/resolve.go +++ b/internal/agentutil/resolve.go @@ -170,7 +170,7 @@ func matchPoolInstanceBare(a config.Agent, input string) (config.Agent, bool) { // IsMultiSessionAgent reports whether a config agent supports multiple // concurrent sessions. func IsMultiSessionAgent(a *config.Agent) bool { - return a != nil && a.SupportsInstanceExpansion() + return a.SupportsExpandedSessionIdentities() } // DeepCopyAgent creates a deep copy of a config.Agent with a new name and dir. diff --git a/internal/agentutil/resolve_test.go b/internal/agentutil/resolve_test.go index 32d9065cac..34b602d1cc 100644 --- a/internal/agentutil/resolve_test.go +++ b/internal/agentutil/resolve_test.go @@ -125,6 +125,23 @@ func TestResolveAgentPoolMemberAllowed(t *testing.T) { } } +func TestResolveAgentPoolMemberRejectsCanonicalSingletonPoolSuffix(t *testing.T) { + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "worker", Dir: "myrig", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1)}, + }, + } + if a, ok := ResolveAgent(cfg, "myrig/worker", ResolveOpts{AllowPoolMembers: true}); !ok || a.QualifiedName() != "myrig/worker" { + t.Fatalf("ResolveAgent(myrig/worker) = (%q, %v), want canonical template", a.QualifiedName(), ok) + } + if _, ok := ResolveAgent(cfg, "myrig/worker-1", ResolveOpts{AllowPoolMembers: true}); ok { + t.Fatal("ResolveAgent(myrig/worker-1) = true, want false for canonical singleton pool") + } + if _, ok := ResolveAgent(cfg, "worker-1", ResolveOpts{AllowPoolMembers: true}); ok { + t.Fatal("ResolveAgent(worker-1) = true, want false for canonical singleton pool") + } +} + func TestResolveAgentCityScopedBindingQualifiedPoolMemberAllowed(t *testing.T) { cfg := &config.City{ Agents: []config.Agent{ diff --git a/internal/api/handler_agents.go b/internal/api/handler_agents.go index 3f925066d6..20ce4da1b4 100644 --- a/internal/api/handler_agents.go +++ b/internal/api/handler_agents.go @@ -85,9 +85,8 @@ type expandedAgent struct { // provider prefix matching — the same approach as discoverPoolInstances. func expandAgent(a config.Agent, cityName, sessTmpl string, sp sessionLister) []expandedAgent { maxSess := a.EffectiveMaxActiveSessions() - isMultiSession := maxSess == nil || *maxSess != 1 - if !isMultiSession { + if !isMultiSessionAgent(a) { return []expandedAgent{{ qualifiedName: a.QualifiedName(), rig: a.Dir, @@ -220,8 +219,7 @@ func findAgent(cfg *config.City, name string) (config.Agent, bool) { } // Check multi-session instance members. maxSess := a.EffectiveMaxActiveSessions() - isMultiSession := maxSess == nil || *maxSess != 1 - if isMultiSession && a.Dir == dir { + if isMultiSessionAgent(a) && a.Dir == dir { isUnlimited := maxSess == nil || *maxSess < 0 if isUnlimited { // Unlimited: match "{name}-{N}" or "{binding.name}-{N}" where N >= 1. @@ -539,8 +537,7 @@ func poolQualifiedNameForSlot(a config.Agent, slot int) string { // isMultiSessionAgent reports whether the agent can have more than one // concurrent session. This is the replacement for the removed IsPool() method. func isMultiSessionAgent(a config.Agent) bool { - maxSess := a.EffectiveMaxActiveSessions() - return maxSess == nil || *maxSess != 1 + return a.SupportsExpandedSessionIdentities() } // classifyAgentKind labels an agent so dashboards can route its sessions to @@ -569,13 +566,16 @@ func isCrewDir(dir string) bool { } func poolInstanceNameForAPI(base string, slot int, a config.Agent) string { - maxSess := a.EffectiveMaxActiveSessions() - isMultiInstance := maxSess != nil && (*maxSess > 1 || *maxSess < 0) - if !isMultiInstance { + if a.UsesCanonicalSingletonPoolIdentity() { return base } if slot >= 1 && slot <= len(a.NamepoolNames) { return a.NamepoolNames[slot-1] } + maxSess := a.EffectiveMaxActiveSessions() + isMultiInstance := maxSess != nil && (*maxSess > 1 || *maxSess < 0) + if !isMultiInstance { + return base + } return fmt.Sprintf("%s-%d", base, slot) } diff --git a/internal/api/handler_agents_test.go b/internal/api/handler_agents_test.go index b08503b273..efb2bfa1a1 100644 --- a/internal/api/handler_agents_test.go +++ b/internal/api/handler_agents_test.go @@ -253,6 +253,48 @@ func TestFindAgentUnlimitedPoolMember(t *testing.T) { } } +func TestFindAgentCanonicalSingletonPoolRejectsSuffix(t *testing.T) { + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "worker", Dir: "myrig", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(1)}, + }, + } + if a, ok := findAgent(cfg, "myrig/worker"); !ok || a.QualifiedName() != "myrig/worker" { + t.Fatalf("findAgent(myrig/worker) = (%q, %v), want canonical template", a.QualifiedName(), ok) + } + if _, ok := findAgent(cfg, "myrig/worker-1"); ok { + t.Fatal("findAgent(myrig/worker-1) = true, want false for canonical singleton pool") + } + expanded := expandAgent(cfg.Agents[0], "city", "", nil) + if len(expanded) != 1 { + t.Fatalf("expandAgent() returned %d entries, want 1", len(expanded)) + } + if expanded[0].qualifiedName != "myrig/worker" { + t.Fatalf("expandAgent()[0].qualifiedName = %q, want myrig/worker", expanded[0].qualifiedName) + } +} + +func TestExpandAgentDisabledAgentUsesConfiguredIdentity(t *testing.T) { + cfg := &config.City{ + Agents: []config.Agent{ + {Name: "worker", Dir: "myrig", MinActiveSessions: intPtr(0), MaxActiveSessions: intPtr(0)}, + }, + } + if isMultiSessionAgent(cfg.Agents[0]) { + t.Fatal("isMultiSessionAgent(max=0) = true, want false") + } + expanded := expandAgent(cfg.Agents[0], "city", "", nil) + if len(expanded) != 1 { + t.Fatalf("expandAgent() returned %d entries, want 1", len(expanded)) + } + if expanded[0].qualifiedName != "myrig/worker" { + t.Fatalf("expandAgent()[0].qualifiedName = %q, want myrig/worker", expanded[0].qualifiedName) + } + if expanded[0].pool != "" { + t.Fatalf("expandAgent()[0].pool = %q, want empty", expanded[0].pool) + } +} + // TestFindAgentBoundedPoolMember covers bounded multi-session pools — both // V1 (no BindingName) and V2 (with BindingName), with and without // NamepoolNames. The V2 cases regressed silently because the bounded diff --git a/internal/config/config.go b/internal/config/config.go index 1493fa4fc7..f2a14fc8f4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2541,22 +2541,49 @@ func (a *Agent) SupportsMultipleSessions() bool { return maxSessions == nil || *maxSessions != 1 } +// UsesCanonicalSingletonPoolIdentity reports whether singleton pool-shaped +// surfaces should use the configured agent identity instead of synthesizing a +// slot identity such as "{name}-1". +func (a *Agent) UsesCanonicalSingletonPoolIdentity() bool { + if a == nil { + return false + } + if strings.TrimSpace(a.Namepool) != "" || len(a.NamepoolNames) > 0 { + return false + } + maxSessions := a.EffectiveMaxActiveSessions() + return maxSessions != nil && *maxSessions == 1 +} + +// SupportsExpandedSessionIdentities reports whether callers should expose or +// discover concrete member identities instead of only the configured identity. +func (a *Agent) SupportsExpandedSessionIdentities() bool { + if a == nil { + return false + } + if m := a.EffectiveMaxActiveSessions(); m != nil && *m == 0 { + return false + } + return a.SupportsInstanceExpansion() && !a.UsesCanonicalSingletonPoolIdentity() +} + // SupportsInstanceExpansion reports whether the template may have multiple // simultaneously addressable concrete instances and therefore needs instance // discovery / synthetic member naming. // // max_active_sessions=1 has two distinct flavors: // -// - Pool agents (MinActiveSessions or ScaleCheck set) addressed as -// "{name}-1" — they participate in pool semantics even at capacity 1. +// - Pool agents (MinActiveSessions or ScaleCheck set) keep pool controller +// semantics. Non-namepool singleton pools still use the canonical +// configured identity; see UsesCanonicalSingletonPoolIdentity. // - Named-session agents (MaxActiveSessions=1 with a [[named_session]] // entry, no Min/ScaleCheck) addressed as just "{name}" — they have a // stable canonical identity and a phantom "-1" suffix breaks tools that -// resolve by qualified name (e.g. refinery). +// resolve by qualified name. // -// We keep instance expansion on for the pool flavor so existing identities -// stay stable, and turn it off for the named-session flavor so the bare name -// resolves correctly. +// We keep instance expansion on for the pool flavor so controller paths still +// run pool reconciliation, and turn it off for the named-session flavor so the +// bare name resolves correctly. func (a *Agent) SupportsInstanceExpansion() bool { if a == nil { return false diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e5c3fc2099..049881e4ce 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1832,6 +1832,85 @@ func TestEffectiveScalingExplicit(t *testing.T) { } } +func TestAgentUsesCanonicalSingletonPoolIdentity(t *testing.T) { + tests := []struct { + name string + a Agent + want bool + }{ + { + name: "max one pool flavor uses canonical identity", + a: Agent{Name: "worker", MinActiveSessions: ptrInt(0), MaxActiveSessions: ptrInt(1)}, + want: true, + }, + { + name: "namepool max one keeps instance identity", + a: Agent{Name: "worker", MaxActiveSessions: ptrInt(1), NamepoolNames: []string{"alpha"}}, + }, + { + name: "multi session pool keeps instance identity", + a: Agent{Name: "worker", MaxActiveSessions: ptrInt(2)}, + }, + { + name: "unbounded pool keeps instance identity", + a: Agent{Name: "worker"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.a.UsesCanonicalSingletonPoolIdentity(); got != tt.want { + t.Fatalf("UsesCanonicalSingletonPoolIdentity() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestAgentSupportsExpandedSessionIdentities(t *testing.T) { + tests := []struct { + name string + a *Agent + want bool + }{ + { + name: "nil", + }, + { + name: "disabled max zero", + a: &Agent{Name: "worker", MinActiveSessions: ptrInt(0), MaxActiveSessions: ptrInt(0)}, + }, + { + name: "canonical singleton pool", + a: &Agent{Name: "worker", MinActiveSessions: ptrInt(0), MaxActiveSessions: ptrInt(1)}, + }, + { + name: "fixed singleton", + a: &Agent{Name: "worker", MaxActiveSessions: ptrInt(1)}, + }, + { + name: "bounded pool", + a: &Agent{Name: "worker", MaxActiveSessions: ptrInt(2)}, + want: true, + }, + { + name: "unbounded pool", + a: &Agent{Name: "worker"}, + want: true, + }, + { + name: "namepool max one", + a: &Agent{Name: "worker", MaxActiveSessions: ptrInt(1), NamepoolNames: []string{"alpha", "beta"}}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.a.SupportsExpandedSessionIdentities(); got != tt.want { + t.Fatalf("SupportsExpandedSessionIdentities() = %v, want %v", got, tt.want) + } + }) + } +} + func TestEffectiveScaleCheckDefaults(t *testing.T) { // Check empty → default uses qualified name. a := Agent{ diff --git a/internal/graphroute/graphroute_test.go b/internal/graphroute/graphroute_test.go index 95c41d8302..27379becb3 100644 --- a/internal/graphroute/graphroute_test.go +++ b/internal/graphroute/graphroute_test.go @@ -525,6 +525,71 @@ func TestResolveGraphStepBinding_AssigneeConcreteSessionBeatsTemplateCollision(t } } +func TestResolveGraphStepBinding_CanonicalSingletonPoolUsesConcreteSession(t *testing.T) { + zero := 0 + one := 1 + cfg := &config.City{ + Workspace: config.Workspace{Name: "test-city"}, + Agents: []config.Agent{ + {Name: "worker", Dir: "frontend", MinActiveSessions: &zero, MaxActiveSessions: &one}, + }, + } + stepByID := map[string]*formula.RecipeStep{ + "demo.work": { + ID: "demo.work", + Title: "Work", + Metadata: map[string]string{"gc.run_target": "worker"}, + }, + } + cache := make(map[string]GraphRouteBinding) + resolving := make(map[string]bool) + + binding, err := ResolveGraphStepBinding("demo.work", stepByID, nil, nil, cache, resolving, GraphRouteBinding{}, "frontend", beads.NewMemStore(), cfg.Workspace.Name, cfg, Deps{Resolver: testAgentResolver{}}) + if err != nil { + t.Fatalf("ResolveGraphStepBinding: %v", err) + } + if binding.QualifiedName != "frontend/worker" { + t.Fatalf("QualifiedName = %q, want frontend/worker", binding.QualifiedName) + } + if binding.SessionName != "frontend--worker" { + t.Fatalf("SessionName = %q, want frontend--worker", binding.SessionName) + } + if binding.MetadataOnly { + t.Fatal("MetadataOnly = true, want false for canonical singleton pool") + } +} + +func TestResolveGraphStepBinding_CanonicalSingletonPoolReportsMissingSessionName(t *testing.T) { + zero := 0 + one := 1 + cfg := &config.City{ + Workspace: config.Workspace{ + Name: "test-city", + SessionTemplate: `{{""}}`, + }, + Agents: []config.Agent{ + {Name: "worker", Dir: "frontend", MinActiveSessions: &zero, MaxActiveSessions: &one}, + }, + } + stepByID := map[string]*formula.RecipeStep{ + "demo.work": { + ID: "demo.work", + Title: "Work", + Metadata: map[string]string{"gc.run_target": "worker"}, + }, + } + cache := make(map[string]GraphRouteBinding) + resolving := make(map[string]bool) + + _, err := ResolveGraphStepBinding("demo.work", stepByID, nil, nil, cache, resolving, GraphRouteBinding{}, "frontend", beads.NewMemStore(), cfg.Workspace.Name, cfg, Deps{Resolver: testAgentResolver{}}) + if err == nil { + t.Fatal("ResolveGraphStepBinding succeeded; want missing session-name error") + } + if !strings.Contains(err.Error(), `could not resolve session name for "frontend/worker"`) { + t.Fatalf("ResolveGraphStepBinding error = %q, want missing session-name guidance", err) + } +} + func TestControlDispatcherBinding_NilConfig(t *testing.T) { _, err := ControlDispatcherBinding(nil, "city", nil, "", Deps{}) if err == nil { diff --git a/internal/sling/sling_env_test.go b/internal/sling/sling_env_test.go index 20d6c58eb1..9b62a993ac 100644 --- a/internal/sling/sling_env_test.go +++ b/internal/sling/sling_env_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/gastownhall/gascity/internal/agent" "github.com/gastownhall/gascity/internal/beads" "github.com/gastownhall/gascity/internal/config" ) @@ -29,6 +30,17 @@ func multiSessionAgent(name string) config.Agent { } } +func canonicalSingletonPoolAgent() config.Agent { + zero := 0 + one := 1 + return config.Agent{ + Name: "worker", + Dir: "rig", + MinActiveSessions: &zero, + MaxActiveSessions: &one, + } +} + func slingEnvTestDeps(t *testing.T, cityPath string) SlingDeps { t.Helper() return SlingDeps{ @@ -57,6 +69,25 @@ func TestResolveSlingEnv_SingleSession_NoMolecule(t *testing.T) { } } +func TestResolveSlingEnv_CanonicalSingletonPoolSetsTarget(t *testing.T) { + deps := slingEnvTestDeps(t, t.TempDir()) + bead, err := deps.Store.Create(beads.Bead{Title: "standalone"}) + if err != nil { + t.Fatal(err) + } + a := canonicalSingletonPoolAgent() + + env := ResolveSlingEnv(a, deps, bead.ID) + + want := agent.SessionNameFor(deps.CityName, a.QualifiedName(), deps.Cfg.Workspace.SessionTemplate) + if got := env["GC_SLING_TARGET"]; got != want { + t.Fatalf("GC_SLING_TARGET = %q, want %q", got, want) + } + if _, ok := env["GC_ARTIFACT_DIR"]; ok { + t.Errorf("GC_ARTIFACT_DIR should not be set for non-molecule bead, got %q", env["GC_ARTIFACT_DIR"]) + } +} + func TestResolveSlingEnv_SingleSession_WithMolecule(t *testing.T) { cityPath := t.TempDir() deps := slingEnvTestDeps(t, cityPath) diff --git a/test/integration/e2e_pool_test.go b/test/integration/e2e_pool_test.go index ffacd3148c..72026da954 100644 --- a/test/integration/e2e_pool_test.go +++ b/test/integration/e2e_pool_test.go @@ -32,10 +32,10 @@ func TestE2E_Pool_InstanceNaming(t *testing.T) { } } -// TestE2E_Pool_MaxOneStillUsesPoolIdentity verifies that max=1 pool configs -// still use concrete pool instance naming rather than collapsing into a -// singleton template identity. -func TestE2E_Pool_MaxOneStillUsesPoolIdentity(t *testing.T) { +// TestE2E_Pool_MaxOneUsesCanonicalIdentity verifies that max=1 pool configs +// use the canonical singleton identity rather than concrete pool instance +// naming. +func TestE2E_Pool_MaxOneUsesCanonicalIdentity(t *testing.T) { city := e2eCity{ Agents: []e2eAgent{ { @@ -47,9 +47,9 @@ func TestE2E_Pool_MaxOneStillUsesPoolIdentity(t *testing.T) { } cityDir := setupE2ECity(t, nil, city) - report := waitForReport(t, cityDir, "singleton-1", e2eDefaultTimeout()) - if !report.has("GC_AGENT", "singleton-1") { - t.Errorf("singleton-1 GC_AGENT: got %v, want [singleton-1]", report.getAll("GC_AGENT")) + report := waitForReport(t, cityDir, "singleton", e2eDefaultTimeout()) + if !report.has("GC_AGENT", "singleton") { + t.Errorf("singleton GC_AGENT: got %v, want [singleton]", report.getAll("GC_AGENT")) } }