From 67a16732ce688b12fe60fbe5cdd28d95470295e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 6 Jul 2026 19:10:12 +0100 Subject: [PATCH] fix: repair office agent launch context (#1601) --- apps/backend/cmd/agentctl/kandev_client.go | 10 +++- apps/backend/cmd/agentctl/kandev_test.go | 14 +++++ .../agent/runtime/lifecycle/skill/delivery.go | 10 ++++ .../runtime/lifecycle/skill/deployer_test.go | 6 ++- .../agent/runtime/lifecycle/skill/inject.go | 14 ++++- .../runtime/lifecycle/skill/inject_test.go | 43 +++++++++++++++ .../handlers/get_task_conversation_test.go | 54 +++++++++++++++++++ .../backend/internal/mcp/handlers/handlers.go | 17 ++++-- .../internal/mcp/server/config_handlers.go | 2 +- .../web/e2e/tests/office/agentctl-cli.spec.ts | 31 +++++++++++ .../e2e/tests/office/skill-injection.spec.ts | 6 ++- docs/specs/office/agents.md | 6 +-- docs/specs/office/overview.md | 4 +- 13 files changed, 202 insertions(+), 15 deletions(-) diff --git a/apps/backend/cmd/agentctl/kandev_client.go b/apps/backend/cmd/agentctl/kandev_client.go index 879f6c33c1..f138d366ba 100644 --- a/apps/backend/cmd/agentctl/kandev_client.go +++ b/apps/backend/cmd/agentctl/kandev_client.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "os" + "strings" "time" ) @@ -26,7 +27,7 @@ type kandevClient struct { // newKandevClient creates a client from environment variables. // Returns an error if required variables (KANDEV_API_URL, KANDEV_API_KEY) are missing. func newKandevClient() (*kandevClient, error) { - apiURL := os.Getenv("KANDEV_API_URL") + apiURL := normalizeKandevAPIURL(os.Getenv("KANDEV_API_URL")) apiKey := os.Getenv("KANDEV_API_KEY") if apiURL == "" || apiKey == "" { return nil, fmt.Errorf("KANDEV_API_URL and KANDEV_API_KEY must be set") @@ -42,6 +43,13 @@ func newKandevClient() (*kandevClient, error) { }, nil } +func normalizeKandevAPIURL(raw string) string { + base := strings.TrimRight(strings.TrimSpace(raw), "/") + // Runtime injection includes /api/v1, while this client appends + // /api/v1/ itself. Strip that single expected suffix. + return strings.TrimSuffix(base, "/api/v1") +} + // isMutating returns true for methods that modify server state. func isMutating(method string) bool { switch method { diff --git a/apps/backend/cmd/agentctl/kandev_test.go b/apps/backend/cmd/agentctl/kandev_test.go index 55e08c93af..8271b32e23 100644 --- a/apps/backend/cmd/agentctl/kandev_test.go +++ b/apps/backend/cmd/agentctl/kandev_test.go @@ -50,6 +50,20 @@ func setEnvVars(t *testing.T, srv *httptest.Server) { t.Setenv("KANDEV_WORKSPACE_ID", "ws-def") } +func TestNewKandevClient_NormalizesVersionedAPIURL(t *testing.T) { + srv, captured := setupMockServer(t, 200, `{"task":{"id":"task-abc"}}`) + setEnvVars(t, srv) + t.Setenv("KANDEV_API_URL", srv.URL+"/api/v1") + + code := runKandevCLI([]string{"task", "get"}) + if code != 0 { + t.Fatalf("expected exit 0, got %d", code) + } + if captured.Path != "/api/v1/office/tasks/task-abc" { + t.Errorf("unexpected path: %s", captured.Path) + } +} + // --- Task Tests --- func TestTaskGet_CallsCorrectEndpoint(t *testing.T) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/skill/delivery.go b/apps/backend/internal/agent/runtime/lifecycle/skill/delivery.go index 67ad6663cc..192e8febb4 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/skill/delivery.go +++ b/apps/backend/internal/agent/runtime/lifecycle/skill/delivery.go @@ -57,6 +57,7 @@ func (d *Deployer) deliverLocal(manifest *Manifest, worktreePath string) DeployR func (d *Deployer) deliverSprites(manifest *Manifest) DeployResult { dir := spritesInstructionsDir(manifest.WorkspaceSlug, manifest.AgentID) rewriteManifestRefs(manifest, dir) + normalizeManifestSkills(manifest) data, err := json.Marshal(manifest) if err != nil { d.logger.Warn("failed to marshal skill manifest for sprites", zap.Error(err)) @@ -70,6 +71,15 @@ func (d *Deployer) deliverSprites(manifest *Manifest) DeployResult { } } +func normalizeManifestSkills(manifest *Manifest) { + if manifest == nil { + return + } + for i := range manifest.Skills { + manifest.Skills[i].Content = renderSkillMarkdown(manifest.Skills[i]) + } +} + // rewriteManifestRefs canonicalises sibling instruction references // (./HEARTBEAT.md, ./SOUL.md, ...) inside each instruction file's // content to absolute paths under instructionsDir. Used by both the diff --git a/apps/backend/internal/agent/runtime/lifecycle/skill/deployer_test.go b/apps/backend/internal/agent/runtime/lifecycle/skill/deployer_test.go index f94036c4d1..d0fed96c75 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/skill/deployer_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/skill/deployer_test.go @@ -99,7 +99,7 @@ func TestDeploy_Local_WritesSkillsAndInstructions(t *testing.T) { skillPath := filepath.Join(worktree, ".claude", "skills", "kandev-sk-foo", "SKILL.md") if data, err := os.ReadFile(skillPath); err != nil { t.Fatalf("read SKILL.md: %v", err) - } else if string(data) != "# foo skill" { + } else if string(data) != "---\nname: sk-foo\ndescription: sk-foo\n---\n# foo skill" { t.Errorf("SKILL.md content = %q", string(data)) } // The legacy host runtime layout for skills must NOT be populated. @@ -220,7 +220,7 @@ func TestDeploy_Docker_WritesSkillsToWorktree(t *testing.T) { skillPath := filepath.Join(worktree, ".claude", "skills", "kandev-sk-foo", "SKILL.md") if data, err := os.ReadFile(skillPath); err != nil { t.Fatalf("skill file missing in worktree: %v", err) - } else if string(data) != "# foo" { + } else if string(data) != "---\nname: sk-foo\ndescription: sk-foo\n---\n# foo" { t.Errorf("SKILL.md content = %q", string(data)) } } @@ -261,6 +261,8 @@ func TestDeploy_Sprites_SetsManifestJSONMetadata(t *testing.T) { } if len(decoded.Skills) != 1 || decoded.Skills[0].Slug != "sk-foo" { t.Errorf("manifest skills = %+v", decoded.Skills) + } else if decoded.Skills[0].Content != "---\nname: sk-foo\ndescription: sk-foo\n---\n# foo" { + t.Errorf("manifest skill content = %q", decoded.Skills[0].Content) } if len(decoded.Instructions) != 1 || decoded.Instructions[0].Filename != "AGENTS.md" { t.Errorf("manifest instructions = %+v", decoded.Instructions) diff --git a/apps/backend/internal/agent/runtime/lifecycle/skill/inject.go b/apps/backend/internal/agent/runtime/lifecycle/skill/inject.go index e2504e5b02..3b67894334 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/skill/inject.go +++ b/apps/backend/internal/agent/runtime/lifecycle/skill/inject.go @@ -52,7 +52,7 @@ func injectSkills(worktreePath, projectSkillDir string, skills []Skill) error { if err := os.MkdirAll(dir, 0o755); err != nil { return fmt.Errorf("mkdir skill %s: %w", sk.Slug, err) } - if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(sk.Content), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(renderSkillMarkdown(sk)), 0o644); err != nil { return fmt.Errorf("write SKILL.md for %s: %w", sk.Slug, err) } } @@ -130,3 +130,15 @@ func resolveGitDir(worktreePath string) (string, error) { func SpritesProjectSkillPath(projectSkillDir, slug string) string { return "/workspace/" + projectSkillDir + "/kandev-" + slug + "/SKILL.md" } + +func renderSkillMarkdown(sk Skill) string { + content := strings.TrimLeft(sk.Content, "\r\n") + if hasYAMLFrontmatter(content) { + return content + } + return fmt.Sprintf("---\nname: %s\ndescription: %s\n---\n%s", sk.Slug, sk.Slug, content) +} + +func hasYAMLFrontmatter(content string) bool { + return strings.HasPrefix(content, "---\n") || strings.HasPrefix(content, "---\r\n") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/skill/inject_test.go b/apps/backend/internal/agent/runtime/lifecycle/skill/inject_test.go index 3a2315ef76..8d56c4ba89 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/skill/inject_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/skill/inject_test.go @@ -35,6 +35,49 @@ func TestInjectSkills_WritesUnderProjectSkillDir(t *testing.T) { } } +func TestInjectSkills_AddsFrontmatterWhenMissing(t *testing.T) { + worktree := t.TempDir() + ensureGit(t, worktree) + + if err := injectSkills(worktree, ".agents/skills", []Skill{ + {Slug: "kandev-team", Content: "# Team\n\nUse the team commands."}, + }); err != nil { + t.Fatalf("injectSkills: %v", err) + } + + data, err := os.ReadFile(filepath.Join(worktree, ".agents", "skills", "kandev-kandev-team", "SKILL.md")) + if err != nil { + t.Fatalf("read SKILL.md: %v", err) + } + got := string(data) + if !strings.HasPrefix(got, "---\nname: kandev-team\ndescription: kandev-team\n---\n") { + t.Fatalf("SKILL.md missing synthesized frontmatter:\n%s", got) + } + if !strings.Contains(got, "# Team\n\nUse the team commands.") { + t.Errorf("SKILL.md missing original body:\n%s", got) + } +} + +func TestInjectSkills_PreservesExistingFrontmatter(t *testing.T) { + worktree := t.TempDir() + ensureGit(t, worktree) + + content := "---\nname: custom\ndescription: existing\n---\n# Body" + if err := injectSkills(worktree, ".agents/skills", []Skill{ + {Slug: "custom", Content: content}, + }); err != nil { + t.Fatalf("injectSkills: %v", err) + } + + data, err := os.ReadFile(filepath.Join(worktree, ".agents", "skills", "kandev-custom", "SKILL.md")) + if err != nil { + t.Fatalf("read SKILL.md: %v", err) + } + if string(data) != content { + t.Errorf("existing frontmatter should be preserved, got %q", string(data)) + } +} + func TestInjectSkills_CleanSlateRemovesPreviousKandevDirs(t *testing.T) { worktree := t.TempDir() ensureGit(t, worktree) diff --git a/apps/backend/internal/mcp/handlers/get_task_conversation_test.go b/apps/backend/internal/mcp/handlers/get_task_conversation_test.go index dd80a0c771..462165bc58 100644 --- a/apps/backend/internal/mcp/handlers/get_task_conversation_test.go +++ b/apps/backend/internal/mcp/handlers/get_task_conversation_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "testing" + "time" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/service" @@ -49,6 +50,59 @@ func TestHandleGetTaskConversation_UsesPrimarySession(t *testing.T) { assert.Equal(t, float64(1), payload["total"]) } +func TestHandleGetTaskConversation_FallsBackToLatestTaskSession(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Test"})) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "Board"})) + task, err := svc.CreateTask(ctx, &service.CreateTaskRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf-1", + Title: "Office task", + }) + require.NoError(t, err) + older := &models.TaskSession{ + ID: "older-office-session", + TaskID: task.ID, + AgentProfileID: "agent-profile-1", + IsPrimary: false, + State: models.TaskSessionStateWaitingForInput, + StartedAt: time.Now().Add(-time.Hour), + } + require.NoError(t, repo.CreateTaskSession(ctx, older)) + newer := &models.TaskSession{ + ID: "newer-office-session", + TaskID: task.ID, + AgentProfileID: "agent-profile-1", + IsPrimary: false, + State: models.TaskSessionStateWaitingForInput, + StartedAt: time.Now(), + } + require.NoError(t, repo.CreateTaskSession(ctx, newer)) + _, err = svc.CreateMessage(ctx, &service.CreateMessageRequest{ + TaskSessionID: newer.ID, + TaskID: task.ID, + AuthorType: "agent", + Content: "office message", + }) + require.NoError(t, err) + + h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()} + msg := makeWSMessage(t, ws.ActionMCPGetTaskConversation, map[string]interface{}{ + "task_id": task.ID, + "limit": 10, + }) + + resp, err := h.handleGetTaskConversation(ctx, msg) + require.NoError(t, err) + assert.Equal(t, ws.MessageTypeResponse, resp.Type) + + var payload map[string]interface{} + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + assert.Equal(t, newer.ID, payload["session_id"]) + assert.Equal(t, float64(1), payload["total"]) +} + func TestHandleGetTaskConversation_SessionMustBelongToTask(t *testing.T) { svc, repo := newTestTaskService(t) _, taskA, _ := seedTaskWithSession(t, svc, repo, models.TaskSessionStateWaitingForInput) diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 8b10121aa6..17a9c4e0f6 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -1459,7 +1459,8 @@ func (h *Handlers) handleMessageTask(ctx context.Context, msg *ws.Message) (*ws. } // handleGetTaskConversation returns paginated conversation history for a task. -// If session_id is omitted, it uses the task's primary session. +// If session_id is omitted, it uses the task's primary session and falls back +// to the latest session when no primary session exists. func (h *Handlers) handleGetTaskConversation(ctx context.Context, msg *ws.Message) (*ws.Message, error) { req, errResp := parseTaskConversationRequest(msg) if errResp != nil { @@ -1538,10 +1539,20 @@ func (h *Handlers) resolveConversationSession(ctx context.Context, msg *ws.Messa return session, nil } session, err := h.taskSvc.GetPrimarySession(ctx, taskID) - if err != nil || session == nil { + if err == nil && session != nil { + return session, nil + } + if err != nil && !errors.Is(err, taskrepo.ErrNoPrimarySession) { + return nil, wsError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to get task session") + } + sessions, listErr := h.taskSvc.ListTaskSessions(ctx, taskID) + if listErr != nil { + return nil, wsError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to list task sessions") + } + if len(sessions) == 0 { return nil, wsError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "task has no session") } - return session, nil + return sessions[0], nil } func wsError(id, action, code, message string) *ws.Message { diff --git a/apps/backend/internal/mcp/server/config_handlers.go b/apps/backend/internal/mcp/server/config_handlers.go index 9d59ea2d43..d60ed7954b 100644 --- a/apps/backend/internal/mcp/server/config_handlers.go +++ b/apps/backend/internal/mcp/server/config_handlers.go @@ -295,7 +295,7 @@ func (s *Server) registerConfigTaskTools() { ) s.mcpServer.AddTool( mcp.NewTool("get_task_conversation_kandev", - mcp.WithDescription("Get conversation history for a task. If session_id is omitted, the primary session is used."), + mcp.WithDescription("Get conversation history for a task. If session_id is omitted, the primary session is used, falling back to the latest task session."), mcp.WithString("task_id", mcp.Required(), mcp.Description("The task ID")), mcp.WithString("session_id", mcp.Description("Optional session ID (must belong to task_id)")), mcp.WithNumber("limit", mcp.Description("Optional page size (defaults to backend setting, max backend-capped)")), diff --git a/apps/web/e2e/tests/office/agentctl-cli.spec.ts b/apps/web/e2e/tests/office/agentctl-cli.spec.ts index 13a6efedb1..0293514710 100644 --- a/apps/web/e2e/tests/office/agentctl-cli.spec.ts +++ b/apps/web/e2e/tests/office/agentctl-cli.spec.ts @@ -77,6 +77,37 @@ test.describe("agentctl kandev CLI", () => { expect(agents.some((a) => a.id === officeSeed.agentId)).toBe(true); }); + test("agents list accepts a versioned API base URL", async ({ + apiClient, + backend, + officeSeed, + }) => { + const run = await apiClient.seedRun({ + agentProfileId: officeSeed.agentId, + reason: "task_assigned", + status: "claimed", + }); + const { token } = await apiClient.mintRuntimeToken({ + agentProfileId: officeSeed.agentId, + workspaceId: officeSeed.workspaceId, + runId: run.run_id, + }); + + const env: NodeJS.ProcessEnv = { + KANDEV_API_URL: `${backend.baseUrl}/api/v1`, + KANDEV_API_KEY: token, + KANDEV_WORKSPACE_ID: officeSeed.workspaceId, + KANDEV_AGENT_ID: officeSeed.agentId, + KANDEV_RUN_ID: run.run_id, + }; + + const res = runCLI(env, ["agents", "list"]); + expect(res.exitCode, `stderr=${res.stderr}`).toBe(0); + const parsed = JSON.parse(res.stdout); + const agents = (parsed.agents ?? []) as Array<{ id: string }>; + expect(agents.some((a) => a.id === officeSeed.agentId)).toBe(true); + }); + test("tasks list returns workspace tasks after one is created", async ({ apiClient, backend, diff --git a/apps/web/e2e/tests/office/skill-injection.spec.ts b/apps/web/e2e/tests/office/skill-injection.spec.ts index 2e1b97eb1a..9f932464f1 100644 --- a/apps/web/e2e/tests/office/skill-injection.spec.ts +++ b/apps/web/e2e/tests/office/skill-injection.spec.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { test, expect } from "../../fixtures/test-base"; import { OfficeApiClient } from "../../helpers/office-api-client"; -// Verifies the spec-aligned skill injection (docs/specs/office-skills/spec.md): +// Verifies the spec-aligned skill injection (docs/specs/office/agents.md): // when a skill is assigned to an agent profile, launching a session writes // it to //kandev-/SKILL.md and appends // the kandev-* glob to .git/info/exclude. @@ -69,7 +69,9 @@ test("skill injection writes assigned skill to session worktree on launch", asyn await expect .poll(() => fs.existsSync(skillFile), { timeout: 15_000, message: skillFile }) .toBe(true); - expect(fs.readFileSync(skillFile, "utf8")).toContain(`Marker: ${slug}-content`); + const skillFileContent = fs.readFileSync(skillFile, "utf8"); + expect(skillFileContent).toContain(`---\nname: ${slug}\ndescription: ${slug}\n---\n`); + expect(skillFileContent).toContain(`Marker: ${slug}-content`); // 6. .git/info/exclude (resolved through .git file for linked worktrees) // contains the kandev-* glob so injected skills never get committed. diff --git a/docs/specs/office/agents.md b/docs/specs/office/agents.md index 0f7b15a5cc..86d49ccc10 100644 --- a/docs/specs/office/agents.md +++ b/docs/specs/office/agents.md @@ -119,7 +119,7 @@ Before each session, instructions are written to `~/.kandev/runtime/` directory), `description`, `source_type` (`inline` | `local_path` | `git`), `source_locator` (path/URL), `content` (SKILL.md text for inline, null otherwise), `file_inventory` (JSON list of `{name, size}`), `workspace_id` FK, `created_by_agent_instance_id` (nullable; agents only edit skills they created), `is_system` (bool), `system_version` (kandev release). @@ -255,7 +255,7 @@ Read them when referenced in these instructions. ### Skill injection -Skill content is stored in the DB (source of truth). Before each session, each desired skill's `SKILL.md` is written into the agent's worktree CWD. +Skill content is stored in the DB (source of truth). Before each session, each desired skill's `SKILL.md` is written into the agent's worktree CWD. If the stored content does not already begin with YAML frontmatter delimited by `---`, runtime materialization prepends generated `name` and `description` frontmatter from the skill slug so agent CLIs can load the file as a native skill. Each agent type defines `ProjectSkillDir` in its `RuntimeConfig`: @@ -556,7 +556,7 @@ There are no TTLs on agent rows, runtime rows, instructions, skills, run history - **GIVEN** a user on `/office/workspace/skills`, **WHEN** they click "Add Skill" and enter a name, description, and SKILL.md content, **THEN** the skill appears in the registry and is available for assignment. -- **GIVEN** a skill assigned to a Claude Code worker, **WHEN** the worker starts a new session, **THEN** the skill's `SKILL.md` is written to `/.claude/skills/kandev-/SKILL.md` (Claude's `ProjectSkillDir`). For non-Claude agents, the path is `/.agents/skills/kandev-/SKILL.md`. +- **GIVEN** a skill assigned to a Claude Code worker, **WHEN** the worker starts a new session, **THEN** the skill's `SKILL.md` is written to `/.claude/skills/kandev-/SKILL.md` (Claude's `ProjectSkillDir`) and begins with YAML frontmatter. For non-Claude agents, the path is `/.agents/skills/kandev-/SKILL.md`. - **GIVEN** a skill sourced from a git URL, **WHEN** the user creates the skill entry, **THEN** the repository is cloned and cached and the file inventory displays in the UI. diff --git a/docs/specs/office/overview.md b/docs/specs/office/overview.md index 6244c67d2d..fa4d11f11a 100644 --- a/docs/specs/office/overview.md +++ b/docs/specs/office/overview.md @@ -129,7 +129,7 @@ Filesystem layout: **Git integration** for repo-backed workspaces: setup via `git clone ~/.kandev/workspaces//`; pulling new config -> Sync UI shows incoming diff -> user applies; conflicts are resolved in the terminal then imported via Sync UI. -**Skill injection**: skills for agent sessions are written into the agent's worktree (CWD) before each session. Skill content can come from the DB (inline skills created via UI), the filesystem (imported from GitHub/skills.sh), or bundled (shipped with the kandev binary). All routes inject into the agent-specific skill path under the worktree. See [office-skills](../office-skills/spec.md). +**Skill injection**: skills for agent sessions are written into the agent's worktree (CWD) before each session. Skill content can come from the DB (inline skills created via UI), the filesystem (imported from GitHub/skills.sh), or bundled (shipped with the kandev binary). All routes inject into the agent-specific skill path under the worktree. See [office agents](./agents.md#skill-injection). **Agent profiles** (model, CLI flags, MCP servers) stay in the existing kandev `agent_profiles` DB table. Office agent instances reference profiles by ID. Filesystem export format: @@ -367,7 +367,7 @@ The CEO and workers move tasks between states by calling the same API the UI use ## Related specs - [office-agents](../office-agents/spec.md) - agent instances, hierarchy, permissions -- [office-skills](../office-skills/spec.md) - skill registry and CWD injection +- [office agents](./agents.md#skill-injection) - skill registry and CWD injection - [office-scheduler](../office-scheduler/spec.md) - wakeup queue and heartbeat scheduler - [office-costs](../office-costs/spec.md) - cost tracking and budget management - [office-routines](../office-routines/spec.md) - recurring scheduled tasks