diff --git a/apps/backend/internal/backendapp/adapters_office.go b/apps/backend/internal/backendapp/adapters_office.go index 05e34cd8b7..4b06e421a5 100644 --- a/apps/backend/internal/backendapp/adapters_office.go +++ b/apps/backend/internal/backendapp/adapters_office.go @@ -230,6 +230,7 @@ func (a *configSyncerAdapter) ApplyIncoming(ctx context.Context, workspaceID str return &officeonboarding.ApplyResult{ CreatedCount: result.CreatedCount, UpdatedCount: result.UpdatedCount, + Warnings: result.Warnings, }, nil } diff --git a/apps/backend/internal/office/config/dto.go b/apps/backend/internal/office/config/dto.go index a80ac0d398..41c90b38d8 100644 --- a/apps/backend/internal/office/config/dto.go +++ b/apps/backend/internal/office/config/dto.go @@ -75,8 +75,9 @@ type ImportPreview struct { // ImportResult reports what was actually changed. type ImportResult struct { - CreatedCount int `json:"created_count"` - UpdatedCount int `json:"updated_count"` + CreatedCount int `json:"created_count"` + UpdatedCount int `json:"updated_count"` + Warnings []string `json:"warnings,omitempty"` } // ParseError describes a single file that failed to parse during a FS scan. diff --git a/apps/backend/internal/office/config/helpers_test.go b/apps/backend/internal/office/config/helpers_test.go index 3da20cb895..209e9ca0b0 100644 --- a/apps/backend/internal/office/config/helpers_test.go +++ b/apps/backend/internal/office/config/helpers_test.go @@ -288,6 +288,19 @@ func fullBundle() *ConfigBundle { } } +// hierarchyAgent returns a minimal AgentConfig for reports_to resolution +// tests. Role and counts are irrelevant to hierarchy resolution but keep the +// fixture realistic. +func hierarchyAgent(name, reportsTo string) AgentConfig { + return AgentConfig{ + Name: name, + Role: "engineer", + ReportsTo: reportsTo, + BudgetMonthlyCents: 100, + MaxConcurrentSessions: 1, + } +} + // seedAgent inserts an office agent row and returns it. func seedAgent(t *testing.T, e *testEnv, wsID, name string) *models.AgentInstance { t.Helper() diff --git a/apps/backend/internal/office/config/import.go b/apps/backend/internal/office/config/import.go index 411e3fafbf..bee70a84fa 100644 --- a/apps/backend/internal/office/config/import.go +++ b/apps/backend/internal/office/config/import.go @@ -114,10 +114,22 @@ func (s *ConfigService) previewProjects( // ApplyImport applies a config bundle to the workspace, deduplicating by name. func (s *ConfigService) ApplyImport( ctx context.Context, workspaceID string, bundle *ConfigBundle, +) (*ImportResult, error) { + return s.applyImport(ctx, workspaceID, bundle, true) +} + +// applyImport applies a bundle with the reports_to lookup policy for its +// caller. Direct bundle imports may reference an existing manager that is not +// in the bundle. Filesystem syncs are authoritative snapshots, so they must +// resolve only against agents present in the snapshot before stale rows are +// pruned. +func (s *ConfigService) applyImport( + ctx context.Context, workspaceID string, bundle *ConfigBundle, + allowExternalManagers bool, ) (*ImportResult, error) { result := &ImportResult{} - if err := s.applyAgents(ctx, workspaceID, bundle.Agents, result); err != nil { + if err := s.applyAgents(ctx, workspaceID, bundle.Agents, result, allowExternalManagers); err != nil { return nil, fmt.Errorf("apply agents: %w", err) } if err := s.applySkills(ctx, workspaceID, bundle.Skills, result); err != nil { @@ -139,6 +151,7 @@ func (s *ConfigService) ApplyImport( func (s *ConfigService) applyAgents( ctx context.Context, wsID string, incoming []AgentConfig, result *ImportResult, + allowExternalManagers bool, ) error { existing, err := s.repo.ListAgentInstances(ctx, wsID) if err != nil { @@ -178,9 +191,73 @@ func (s *ConfigService) applyAgents( result.CreatedCount++ } } + return s.applyAgentReportsTo(ctx, wsID, incoming, result, allowExternalManagers) +} + +// applyAgentReportsTo resolves each incoming agent's reports_to name to the +// target agent's ID and persists it. It runs after every agent in the bundle +// has been created or updated above, so it re-lists the workspace to pick up +// IDs assigned to newly-created rows — this makes resolution independent of +// bundle order. When allowExternalManagers is true, a reports_to name may +// also reference an agent that is only in the workspace. Filesystem syncs pass +// false because the snapshot is authoritative and rows absent from it are +// pruned after this import. A name that resolves to nothing (dangling +// reference) or to the agent itself is recorded as a warning and left empty +// rather than failing the import. +func (s *ConfigService) applyAgentReportsTo( + ctx context.Context, wsID string, incoming []AgentConfig, result *ImportResult, + allowExternalManagers bool, +) error { + current, err := s.repo.ListAgentInstances(ctx, wsID) + if err != nil { + return fmt.Errorf("resolve reports_to: list agents: %w", err) + } + byName := make(map[string]*models.AgentInstance, len(current)) + incomingNames := bundleAgentSet(incoming) + for _, a := range current { + if !allowExternalManagers && !incomingNames[a.Name] { + continue + } + byName[a.Name] = a + } + for _, cfg := range incoming { + agent, ok := byName[cfg.Name] + if !ok { + continue + } + reportsTo, warning := resolveReportsTo(cfg, byName) + if warning != "" { + result.Warnings = append(result.Warnings, warning) + } + if agent.ReportsTo == reportsTo { + continue + } + agent.ReportsTo = reportsTo + if err := s.repo.UpdateAgentInstance(ctx, agent); err != nil { + return fmt.Errorf("resolve reports_to: update %q: %w", agent.Name, err) + } + } return nil } +// resolveReportsTo resolves cfg.ReportsTo (a name) against byName (a name -> +// agent index built from the target workspace after apply). It returns the +// resolved manager ID, or an empty string plus a warning when the name is a +// self-reference or does not match any known agent. +func resolveReportsTo(cfg AgentConfig, byName map[string]*models.AgentInstance) (string, string) { + if cfg.ReportsTo == "" { + return "", "" + } + if cfg.ReportsTo == cfg.Name { + return "", fmt.Sprintf("agent %q cannot report to itself", cfg.Name) + } + manager, ok := byName[cfg.ReportsTo] + if !ok { + return "", fmt.Sprintf("agent %q reports_to %q, which was not found", cfg.Name, cfg.ReportsTo) + } + return manager.ID, "" +} + func (s *ConfigService) applySkills( ctx context.Context, wsID string, incoming []SkillConfig, result *ImportResult, ) error { diff --git a/apps/backend/internal/office/config/import_reports_to_test.go b/apps/backend/internal/office/config/import_reports_to_test.go new file mode 100644 index 0000000000..5673defb83 --- /dev/null +++ b/apps/backend/internal/office/config/import_reports_to_test.go @@ -0,0 +1,210 @@ +package config + +import ( + "context" + "strings" + "testing" +) + +// TestApplyImport_ReportsToResolvesRegardlessOfBundleOrder imports a manager +// that appears after its report in the bundle slice and asserts resolution +// still succeeds. The resolver re-lists agents after every create/update in +// the bundle has run, rather than relying on encounter order. +func TestApplyImport_ReportsToResolvesRegardlessOfBundleOrder(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + bundle := &ConfigBundle{Agents: []AgentConfig{ + hierarchyAgent("bob", "carol"), + hierarchyAgent("carol", ""), + }} + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle); err != nil { + t.Fatalf("ApplyImport: %v", err) + } + + carol := agentByName(t, env, testWorkspaceID, "carol") + bob := agentByName(t, env, testWorkspaceID, "bob") + assertEqual(t, "bob reports_to", bob.ReportsTo, carol.ID) +} + +// TestApplyImport_ReportsToDanglingReferenceWarns asserts a reports_to naming +// nobody leaves the field empty, does not fail the import, and records +// exactly one warning naming both the agent and the missing manager. +func TestApplyImport_ReportsToDanglingReferenceWarns(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + bundle := &ConfigBundle{Agents: []AgentConfig{hierarchyAgent("bob", "nobody")}} + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 1) + assertEqual(t, "updated count", result.UpdatedCount, 0) + if len(result.Warnings) != 1 { + t.Fatalf("warnings: got %d, want 1: %v", len(result.Warnings), result.Warnings) + } + if !strings.Contains(result.Warnings[0], "bob") || !strings.Contains(result.Warnings[0], "nobody") { + t.Errorf("warning %q does not name both the agent and the missing manager", result.Warnings[0]) + } + assertEqual(t, "bob reports_to", agentByName(t, env, testWorkspaceID, "bob").ReportsTo, "") +} + +// TestApplyImport_ReportsToSelfReferenceWarns asserts an agent naming itself +// as its own manager is skipped with a warning rather than persisted or +// treated as an error. +func TestApplyImport_ReportsToSelfReferenceWarns(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + bundle := &ConfigBundle{Agents: []AgentConfig{hierarchyAgent("bob", "bob")}} + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + if len(result.Warnings) != 1 { + t.Fatalf("warnings: got %d, want 1: %v", len(result.Warnings), result.Warnings) + } + if !strings.Contains(result.Warnings[0], "bob") { + t.Errorf("warning %q does not name the self-referencing agent", result.Warnings[0]) + } + assertEqual(t, "bob reports_to", agentByName(t, env, testWorkspaceID, "bob").ReportsTo, "") +} + +// TestApplyImport_ReportsToIsIdempotent imports the same hierarchy bundle +// twice and asserts the second pass leaves reports_to unchanged and does not +// duplicate rows. +func TestApplyImport_ReportsToIsIdempotent(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + bundle := &ConfigBundle{Agents: []AgentConfig{ + hierarchyAgent("bob", "carol"), + hierarchyAgent("carol", ""), + }} + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle); err != nil { + t.Fatalf("first import: %v", err) + } + firstBobID := agentByName(t, env, testWorkspaceID, "bob").ID + firstCarolID := agentByName(t, env, testWorkspaceID, "carol").ID + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle); err != nil { + t.Fatalf("second import: %v", err) + } + + agents, err := env.repo.ListAgentInstances(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "agent row count", len(agents), 2) + + bob := agentByName(t, env, testWorkspaceID, "bob") + carol := agentByName(t, env, testWorkspaceID, "carol") + assertEqual(t, "bob id unchanged", bob.ID, firstBobID) + assertEqual(t, "carol id unchanged", carol.ID, firstCarolID) + assertEqual(t, "bob reports_to stable", bob.ReportsTo, carol.ID) +} + +// TestApplyImport_ReportsToClearsOnOmission asserts that re-importing an +// agent with a bundle omitting reports_to clears a previously-set value, +// matching every other field's overwrite-on-update semantics. +func TestApplyImport_ReportsToClearsOnOmission(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seeded := &ConfigBundle{Agents: []AgentConfig{ + hierarchyAgent("bob", "carol"), + hierarchyAgent("carol", ""), + }} + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, seeded); err != nil { + t.Fatalf("seed import: %v", err) + } + if agentByName(t, env, testWorkspaceID, "bob").ReportsTo == "" { + t.Fatal("setup: bob should already report to carol") + } + + cleared := &ConfigBundle{Agents: []AgentConfig{ + hierarchyAgent("bob", ""), + hierarchyAgent("carol", ""), + }} + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, cleared); err != nil { + t.Fatalf("clearing import: %v", err) + } + assertEqual(t, "bob reports_to cleared", agentByName(t, env, testWorkspaceID, "bob").ReportsTo, "") +} + +// TestApplyImport_ReportsToRoundTripsAcrossWorkspaces exports a workspace +// with a hierarchy and imports the resulting bundle into a separate +// workspace (its own database, mirroring a real export/import across +// installs), asserting the hierarchy survives the round trip. +func TestApplyImport_ReportsToRoundTripsAcrossWorkspaces(t *testing.T) { + source := newTestEnv(t) + ctx := context.Background() + seeded := &ConfigBundle{Agents: []AgentConfig{ + hierarchyAgent("bob", "carol"), + hierarchyAgent("carol", ""), + }} + if _, err := source.svc.ApplyImport(ctx, testWorkspaceID, seeded); err != nil { + t.Fatalf("seed source workspace: %v", err) + } + + bundle, err := source.svc.ExportBundle(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ExportBundle: %v", err) + } + + target := newTestEnv(t) + if _, err := target.svc.ApplyImport(ctx, testWorkspaceID, bundle); err != nil { + t.Fatalf("ApplyImport into target workspace: %v", err) + } + + bob := agentByName(t, target, testWorkspaceID, "bob") + carol := agentByName(t, target, testWorkspaceID, "carol") + assertEqual(t, "bob reports_to carol in target workspace", bob.ReportsTo, carol.ID) +} + +// TestApplyIncoming_PreservesReportsToFromFilesystem covers the +// filesystem-config-sync path (ApplyIncoming -> ScanFilesystem -> +// ApplyImport), not just the bundle-upload import endpoint. reports_to +// written to an on-disk agent YAML file must survive the scan and resolve to +// the manager's ID, the same as it does for a directly-uploaded bundle. +func TestApplyIncoming_PreservesReportsToFromFilesystem(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + env.writeFSAgent(t, "grace", "name: grace\nrole: manager\n") + env.writeFSAgent(t, "bob", "name: bob\nrole: worker\nreports_to: grace\n") + + if _, err := env.svc.ApplyIncoming(ctx, testWorkspaceID); err != nil { + t.Fatalf("ApplyIncoming: %v", err) + } + + grace := agentByName(t, env, testWorkspaceID, "grace") + bob := agentByName(t, env, testWorkspaceID, "bob") + assertEqual(t, "bob reports_to grace after fs sync", bob.ReportsTo, grace.ID) +} + +// TestApplyIncoming_DoesNotKeepReportsToForPrunedManager verifies that the +// incoming sync does not resolve hierarchy names against DB-only agents. +// A manager that is absent from the filesystem must not leave a dangling ID +// on an agent that remains in the imported bundle. +func TestApplyIncoming_DoesNotKeepReportsToForPrunedManager(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + manager := seedAgent(t, env, testWorkspaceID, "grace") + seedAgent(t, env, testWorkspaceID, "bob") + env.writeFSAgent(t, "bob", "name: bob\nrole: worker\nreports_to: grace\n") + + result, err := env.svc.ApplyIncoming(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ApplyIncoming: %v", err) + } + if len(result.Warnings) != 1 { + t.Fatalf("warnings: got %d, want 1: %v", len(result.Warnings), result.Warnings) + } + if !strings.Contains(result.Warnings[0], "bob") || !strings.Contains(result.Warnings[0], "grace") { + t.Errorf("warning %q does not name both the agent and the pruned manager", result.Warnings[0]) + } + assertEqual(t, "bob reports_to after manager prune", agentByName(t, env, testWorkspaceID, "bob").ReportsTo, "") + if _, err := env.repo.GetAgentInstance(ctx, manager.ID); err == nil { + t.Fatalf("pruned manager %q is still visible in the repository", manager.ID) + } +} diff --git a/apps/backend/internal/office/config/import_test.go b/apps/backend/internal/office/config/import_test.go index 07ffe90c8e..824b5cb08b 100644 --- a/apps/backend/internal/office/config/import_test.go +++ b/apps/backend/internal/office/config/import_test.go @@ -57,16 +57,18 @@ func TestApplyImport_CreatesEveryFieldFromBundle(t *testing.T) { assertEqual(t, "project executor config", project.ExecutorConfig, `{"type":"local_docker"}`) } -// TestApplyImport_NameReferencesAreNotResolved pins the deliberate gap in the -// importer: the three cross-entity references in the DTO (agent reports_to, -// routine assignee_name, project lead_agent_name) name an entity but the -// import never resolves them to an ID. They are export-only fields today. -// Wiring resolution in must update this test. +// TestApplyImport_NameReferencesAreNotResolved documents the current state of +// the three cross-entity references in the DTO (agent reports_to, routine +// assignee_name, project lead_agent_name): agent reports_to IS resolved to +// the target agent's ID (see the reports_to-specific tests in +// import_reports_to_test.go for the full behavior), but routine assignee_name +// and project lead_agent_name remain export-only fields that import never +// resolves. Wiring resolution in for those must update this test. func TestApplyImport_NameReferencesAreNotResolved(t *testing.T) { env := newTestEnv(t) ctx := context.Background() - // Seed the referenced agent so a resolver would have something to find. + // Seed the referenced agent so the resolver has something to find. lead := seedAgent(t, env, testWorkspaceID, "grace") if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()); err != nil { @@ -74,10 +76,7 @@ func TestApplyImport_NameReferencesAreNotResolved(t *testing.T) { } agent := agentByName(t, env, testWorkspaceID, "ada") - assertEqual(t, "agent reports_to", agent.ReportsTo, "") - if agent.ReportsTo == lead.ID { - t.Error("reports_to resolution appears implemented; update this test") - } + assertEqual(t, "agent reports_to", agent.ReportsTo, lead.ID) assertEqual(t, "routine assignee", routineByName(t, env, testWorkspaceID, "standup").AssigneeAgentProfileID, "") assertEqual(t, "project lead", diff --git a/apps/backend/internal/office/config/sync.go b/apps/backend/internal/office/config/sync.go index 7911d5214a..8980338168 100644 --- a/apps/backend/internal/office/config/sync.go +++ b/apps/backend/internal/office/config/sync.go @@ -19,7 +19,7 @@ func (s *ConfigService) ScanFilesystem(_ context.Context, _ string) (*ConfigBund bundle := &ConfigBundle{Settings: SettingsConfig{Name: defaultWorkspaceName}} for _, a := range s.cfgLoader.GetAgents(defaultWorkspaceName) { bundle.Agents = append(bundle.Agents, AgentConfig{ - Name: a.Name, Role: string(a.Role), Icon: a.Icon, + Name: a.Name, Role: string(a.Role), Icon: a.Icon, ReportsTo: a.ReportsTo, BudgetMonthlyCents: a.BudgetMonthlyCents, MaxConcurrentSessions: a.MaxConcurrentSessions, DesiredSkills: a.DesiredSkills, ExecutorPreference: a.ExecutorPreference, }) @@ -97,7 +97,9 @@ func (s *ConfigService) ApplyIncoming(ctx context.Context, workspaceID string) ( if err != nil { return nil, err } - result, err := s.ApplyImport(ctx, workspaceID, bundle) + // The filesystem is an authoritative snapshot for this direction. Do not + // resolve reports_to against DB-only managers because they are pruned below. + result, err := s.applyImport(ctx, workspaceID, bundle, false) if err != nil { return nil, err } diff --git a/apps/backend/internal/office/config/sync_test.go b/apps/backend/internal/office/config/sync_test.go index a31e92e955..233fb29dc4 100644 --- a/apps/backend/internal/office/config/sync_test.go +++ b/apps/backend/internal/office/config/sync_test.go @@ -81,6 +81,7 @@ func TestScanFilesystem_CarriesEveryFieldFromDisk(t *testing.T) { assertEqual(t, "agent name", agent.Name, "ada") assertEqual(t, "agent role", agent.Role, "cto") assertEqual(t, "agent icon", agent.Icon, "🤖") + assertEqual(t, "agent reports_to", agent.ReportsTo, "grace") assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) assertEqual(t, "agent max sessions", agent.MaxConcurrentSessions, 3) assertEqual(t, "agent desired skills", agent.DesiredSkills, `["go","sql"]`) @@ -117,15 +118,16 @@ func TestScanFilesystem_CarriesEveryFieldFromDisk(t *testing.T) { `{"type":"local_docker"}`) } -// TestScanFilesystem_DropsNameReferences records the same gap the importer -// has: the scan does not carry reports_to, assignee_name or lead_agent_name -// off disk even though all three are present in the fixture files. The result -// is a lossy sync — applying a checked-in workspace config erases the org -// chart, routine assignees and project leads rather than resolving the -// portable names to IDs. +// TestScanFilesystem_DropsNameReferences records a gap that remains for +// routines and projects: the scan does not carry assignee_name or +// lead_agent_name off disk even though both are present in the fixture +// files. The result is a lossy sync for those two fields — applying a +// checked-in workspace config erases routine assignees and project leads +// rather than resolving the portable names to IDs. Agent reports_to no +// longer drops (see TestScanFilesystem_CarriesEveryFieldFromDisk). // -// Recorded rather than fixed (test-only change); carrying the names through -// the scan and resolving them on import must update this test. +// Recorded rather than fixed (test-only change); carrying the remaining +// names through the scan and resolving them on import must update this test. func TestScanFilesystem_DropsNameReferences(t *testing.T) { env := newTestEnv(t) seedFullFSState(t, env) @@ -134,7 +136,6 @@ func TestScanFilesystem_DropsNameReferences(t *testing.T) { if err != nil { t.Fatalf("ScanFilesystem: %v", err) } - assertEqual(t, "agent reports_to", bundle.Agents[0].ReportsTo, "") assertEqual(t, "routine assignee", bundle.Routines[0].AssigneeName, "") assertEqual(t, "project lead", bundle.Projects[0].LeadAgentName, "") } diff --git a/apps/backend/internal/office/config/sync_util.go b/apps/backend/internal/office/config/sync_util.go index 804fc326d7..41fb15a105 100644 --- a/apps/backend/internal/office/config/sync_util.go +++ b/apps/backend/internal/office/config/sync_util.go @@ -126,6 +126,7 @@ func writeBundleEntities(w *configloader.FileWriter, bundle *ConfigBundle) error for _, a := range bundle.Agents { agent := &models.AgentInstance{ Name: a.Name, Role: models.AgentRole(a.Role), Icon: a.Icon, + ReportsTo: a.ReportsTo, BudgetMonthlyCents: a.BudgetMonthlyCents, MaxConcurrentSessions: a.MaxConcurrentSessions, DesiredSkills: a.DesiredSkills, ExecutorPreference: a.ExecutorPreference, } diff --git a/apps/backend/internal/office/config/sync_util_test.go b/apps/backend/internal/office/config/sync_util_test.go index d7ad4e5db8..461873ed36 100644 --- a/apps/backend/internal/office/config/sync_util_test.go +++ b/apps/backend/internal/office/config/sync_util_test.go @@ -143,6 +143,7 @@ func TestWriteBundleToFS_AllFieldsReachDisk(t *testing.T) { assertEqual(t, "agent name", got.Name, "ada") assertEqual(t, "agent role", string(got.Role), "cto") assertEqual(t, "agent icon", got.Icon, "🤖") + assertEqual(t, "agent reports_to", got.ReportsTo, "grace") assertEqual(t, "agent budget", got.BudgetMonthlyCents, 4200) assertEqual(t, "agent max sessions", got.MaxConcurrentSessions, 3) assertEqual(t, "agent desired skills", got.DesiredSkills, `["go","sql"]`) diff --git a/apps/backend/internal/office/onboarding/dto.go b/apps/backend/internal/office/onboarding/dto.go index 5e5380d5c9..1b0fefbb26 100644 --- a/apps/backend/internal/office/onboarding/dto.go +++ b/apps/backend/internal/office/onboarding/dto.go @@ -18,6 +18,7 @@ type OnboardingStateResponse struct { type OnboardingImportFSResponse struct { WorkspaceIDs []string `json:"workspaceIds"` ImportedCount int `json:"importedCount"` + Warnings []string `json:"warnings,omitempty"` } // OnboardingCompleteRequest is the request body for POST /onboarding/complete. diff --git a/apps/backend/internal/office/onboarding/handler.go b/apps/backend/internal/office/onboarding/handler.go index 27a79bceb4..816d88d9f2 100644 --- a/apps/backend/internal/office/onboarding/handler.go +++ b/apps/backend/internal/office/onboarding/handler.go @@ -91,5 +91,6 @@ func (h *Handler) importFromFS(c *gin.Context) { c.JSON(http.StatusOK, OnboardingImportFSResponse{ WorkspaceIDs: result.WorkspaceIDs, ImportedCount: result.ImportedCount, + Warnings: result.Warnings, }) } diff --git a/apps/backend/internal/office/onboarding/service.go b/apps/backend/internal/office/onboarding/service.go index a085fc92e0..88ca65afbf 100644 --- a/apps/backend/internal/office/onboarding/service.go +++ b/apps/backend/internal/office/onboarding/service.go @@ -102,6 +102,7 @@ type ConfigSyncer interface { type ApplyResult struct { CreatedCount int UpdatedCount int + Warnings []string } // RunReason for a newly assigned task. @@ -211,6 +212,7 @@ type CompleteResult struct { type ImportFromFSResult struct { WorkspaceIDs []string ImportedCount int + Warnings []string } // GetOnboardingState checks whether onboarding has been completed. @@ -440,6 +442,7 @@ func (s *OnboardingService) importSingleWorkspace( return "", false } result.ImportedCount += importResult.CreatedCount + importResult.UpdatedCount + result.Warnings = append(result.Warnings, importResult.Warnings...) } return wsID, true } diff --git a/apps/backend/internal/office/onboarding/service_test.go b/apps/backend/internal/office/onboarding/service_test.go index e138fbf3c0..7e55cec3cc 100644 --- a/apps/backend/internal/office/onboarding/service_test.go +++ b/apps/backend/internal/office/onboarding/service_test.go @@ -84,6 +84,14 @@ type mockWorkflowEnsurer struct { defaultID string } +type mockConfigSyncer struct { + result *ApplyResult +} + +func (m *mockConfigSyncer) ApplyIncoming(_ context.Context, _ string) (*ApplyResult, error) { + return m.result, nil +} + func (m *mockWorkflowEnsurer) EnsureOfficeWorkflow(_ context.Context, _ string) (string, error) { return m.officeID, nil } @@ -575,6 +583,29 @@ func TestImportFromFS_SkipsAlreadyImported(t *testing.T) { } } +func TestImportFromFS_PropagatesSyncWarnings(t *testing.T) { + svc, _, _ := newTestOnboardingService(t) + svc.configSyncer = &mockConfigSyncer{result: &ApplyResult{ + CreatedCount: 2, + UpdatedCount: 1, + Warnings: []string{"agent \"bob\" reports_to \"grace\", which was not found"}, + }} + + result, err := svc.ImportFromFS(context.Background()) + if err != nil { + t.Fatalf("import from fs: %v", err) + } + if result.ImportedCount != 3 { + t.Fatalf("imported count: got %d, want 3", result.ImportedCount) + } + if len(result.Warnings) != 1 { + t.Fatalf("warnings: got %d, want 1: %v", len(result.Warnings), result.Warnings) + } + if result.Warnings[0] != "agent \"bob\" reports_to \"grace\", which was not found" { + t.Errorf("warning: got %q", result.Warnings[0]) + } +} + // onboardingRoutingScenario is one row in the default-tier seeding // table. It captures the wizard's tier input plus the expected // outcome on the persisted office_workspace_routing row. diff --git a/apps/web/app/office/setup/setup-wizard.tsx b/apps/web/app/office/setup/setup-wizard.tsx index 49c2b9d075..d9fb6857a4 100644 --- a/apps/web/app/office/setup/setup-wizard.tsx +++ b/apps/web/app/office/setup/setup-wizard.tsx @@ -233,6 +233,9 @@ export function SetupWizard({ try { const result = await importFromFS(); toast.success(t("office:importedConfigEntries", { importedCount: result.importedCount })); + if (result.warnings?.length) { + toast.warning(result.warnings.join("\n")); + } router.push("/office"); router.refresh(); } catch (err) { diff --git a/apps/web/app/office/workspace/settings/sync/use-sync-state.ts b/apps/web/app/office/workspace/settings/sync/use-sync-state.ts index 7e3d9bcf4b..af4ce0e317 100644 --- a/apps/web/app/office/workspace/settings/sync/use-sync-state.ts +++ b/apps/web/app/office/workspace/settings/sync/use-sync-state.ts @@ -51,6 +51,9 @@ export function useSyncState(activeWorkspaceId: string) { updated: res.result.updated_count, }), ); + if (res.result.warnings?.length) { + toast.warning(res.result.warnings.join("\n")); + } await refresh(); } catch (err) { toast.error(err instanceof Error ? err.message : t(MSG_IMPORT_FAIL)); diff --git a/apps/web/lib/api/domains/office-extended-api.ts b/apps/web/lib/api/domains/office-extended-api.ts index ff899726d4..4b80625980 100644 --- a/apps/web/lib/api/domains/office-extended-api.ts +++ b/apps/web/lib/api/domains/office-extended-api.ts @@ -80,13 +80,12 @@ export function applyImport( bundle: Record, options?: ApiRequestOptions, ) { - return fetchJson<{ result: { created_count: number; updated_count: number } }>( - `${BASE}/workspaces/${workspaceId}/config/import`, - { - ...options, - init: { method: "POST", body: JSON.stringify(bundle), ...options?.init }, - }, - ); + return fetchJson<{ + result: { created_count: number; updated_count: number; warnings?: string[] }; + }>(`${BASE}/workspaces/${workspaceId}/config/import`, { + ...options, + init: { method: "POST", body: JSON.stringify(bundle), ...options?.init }, + }); } // --- Config Sync (FS <-> DB) --- @@ -131,10 +130,12 @@ export function getOutgoingDiff(workspaceId: string, options?: ApiRequestOptions } export function applyIncomingSync(workspaceId: string, options?: ApiRequestOptions) { - return fetchJson<{ result: { created_count: number; updated_count: number } }>( - `${BASE}/workspaces/${workspaceId}/config/sync/import-fs`, - { ...options, init: { method: "POST", ...options?.init } }, - ); + return fetchJson<{ + result: { created_count: number; updated_count: number; warnings?: string[] }; + }>(`${BASE}/workspaces/${workspaceId}/config/sync/import-fs`, { + ...options, + init: { method: "POST", ...options?.init }, + }); } export function applyOutgoingSync(workspaceId: string, options?: ApiRequestOptions) { @@ -493,6 +494,7 @@ export function completeOnboarding(data: OnboardingCompletePayload, options?: Ap export type ImportFromFSResult = { workspaceIds: string[]; importedCount: number; + warnings?: string[]; }; export function importFromFS(options?: ApiRequestOptions) {