fix(office): stop config import from flattening the agent org chart (#2812)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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, "")
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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"]`)
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -80,13 +80,12 @@ export function applyImport(
|
||||
bundle: Record<string, unknown>,
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user