diff --git a/cmd/agents.go b/cmd/agents.go new file mode 100644 index 0000000..3734b67 --- /dev/null +++ b/cmd/agents.go @@ -0,0 +1,158 @@ +package cmd + +import ( + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + + "github.com/spf13/cobra" + "github.com/warrenronsiek/ctask/internal/agent" + "github.com/warrenronsiek/ctask/internal/config" + "github.com/warrenronsiek/ctask/internal/workspace" +) + +var agentsCmd = &cobra.Command{ + Use: "agents", + Short: "Inspect and validate agent configuration", + SilenceUsage: true, +} + +var agentsCheckCmd = &cobra.Command{ + Use: "check [workspace]", + Short: "Validate the agent configuration for a workspace", + Long: `Validate that the configured agent for a workspace can be launched. +Does NOT launch the agent. Checks: agent type known, command resolvable +on PATH, launch_dir valid, AGENTS.md present, CLAUDE.md shim present +(WARN, claude type only). Displays agent.env keys informationally. + +If [workspace] is omitted, checks the most-recently-active workspace.`, + Args: cobra.MaximumNArgs(1), + SilenceUsage: true, + RunE: runAgentsCheck, +} + +func init() { + agentsCheckCmd.ValidArgsFunction = completeWorkspaces(completionAny) + agentsCmd.AddCommand(agentsCheckCmd) + rootCmd.AddCommand(agentsCmd) +} + +func runAgentsCheck(cmd *cobra.Command, args []string) error { + roots := config.SearchRoots() + var ws *workspace.QueryResult + if len(args) == 1 { + ws = resolveOne(roots, args[0], true) + } else { + best, err := workspace.MostRecentActive(roots) + if err != nil || best == nil { + return fmt.Errorf("agents check: no workspace specified and no active workspace found") + } + ws = best + } + + return runAgentsCheckOnWorkspace(cmd.OutOrStdout(), ws) +} + +// runAgentsCheckOnWorkspace performs the checks and prints results. +// Returns a non-nil error iff any check is FAIL (so doctor can surface +// the failure count). Extracted from runAgentsCheck so doctor can reuse. +func runAgentsCheckOnWorkspace(out io.Writer, ws *workspace.QueryResult) error { + fmt.Fprintf(out, "── Agent Check: %s ─────────────────\n", ws.Meta.Slug) + + failed := 0 + spec := ws.Meta.Agent + + // 1. Agent type known. (ValidateAgentSpec already enforced this on read, + // but we re-display for symmetry. If type is empty, fall through to + // default_agent and label as such.) + typ := spec.Type + typeLabel := typ + if typ == "" { + typ = config.LoadResolver().DefaultAgent().Value + typeLabel = typ + " (from default_agent)" + } + fmt.Fprintf(out, "Agent type: %s\n", typeLabel) + if !agent.IsKnownType(typ) { + fmt.Fprintf(out, " [FAIL] unknown agent type %q\n", typ) + failed++ + } else { + fmt.Fprintln(out, " [PASS]") + } + + // 2. Command resolvable. + resolved, rerr := agent.Resolve(spec, typ) + if rerr != nil { + fmt.Fprintf(out, "Command: [FAIL] %v\n", rerr) + failed++ + } else { + path, lerr := exec.LookPath(resolved.Command) + fmt.Fprintf(out, "Command: %s", resolved.Command) + if lerr != nil { + fmt.Fprintf(out, "\n [FAIL] not found on PATH: %v\n", lerr) + failed++ + } else { + fmt.Fprintf(out, " (%s)\n", path) + fmt.Fprintln(out, " [PASS]") + } + } + + // 3. launch_dir valid. + launchAbs, _, lderr := workspace.ResolveLaunch(ws.Path, ws.Meta.LaunchDir) + fmt.Fprintf(out, "Launch dir: %s\n", launchAbs) + if lderr != nil { + fmt.Fprintf(out, " [FAIL] %v\n", lderr) + failed++ + } else { + fmt.Fprintln(out, " [PASS]") + } + + // 4. AGENTS.md present. + if _, err := os.Stat(filepath.Join(ws.Path, "AGENTS.md")); err == nil { + fmt.Fprintln(out, "AGENTS.md: found\n [PASS]") + } else { + fmt.Fprintln(out, "AGENTS.md: [FAIL] missing") + failed++ + } + + // 5. CLAUDE.md shim — WARN only, claude type only. + if typ == "claude" { + if _, err := os.Stat(filepath.Join(ws.Path, "CLAUDE.md")); err == nil { + fmt.Fprintln(out, "CLAUDE.md: found\n [PASS]") + } else { + fmt.Fprintln(out, "CLAUDE.md: [WARN] missing (shim is optional)") + } + } + + // 6. agent.env keys (informational + CTASK_* shadow WARN). + if len(spec.Env) > 0 { + keys := make([]string, 0, len(spec.Env)) + for k := range spec.Env { + keys = append(keys, k) + } + sort.Strings(keys) + fmt.Fprintf(out, "Agent env: %d keys configured (%s)\n", len(keys), strings.Join(keys, ", ")) + // agent.env merges AFTER ctask's own exported vars, so a key + // matching CTASK_* shadows what ctask exports. The user is allowed + // to do this (spec §5); we surface the surprise. WARN does not + // bump the failed counter. + var shadowed []string + for _, k := range keys { + if strings.HasPrefix(k, "CTASK_") { + shadowed = append(shadowed, k) + } + } + if len(shadowed) > 0 { + fmt.Fprintf(out, " [WARN] agent.env overrides ctask-exported vars: %s\n", + strings.Join(shadowed, ", ")) + } + } + + if failed > 0 { + return fmt.Errorf("agents check: %d failures", failed) + } + return nil +} diff --git a/cmd/agents_check_test.go b/cmd/agents_check_test.go new file mode 100644 index 0000000..6a5147d --- /dev/null +++ b/cmd/agents_check_test.go @@ -0,0 +1,256 @@ +package cmd + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/config" + "github.com/warrenronsiek/ctask/internal/workspace" +) + +// captureRootCmd redirects rootCmd's output to a buffer for the duration +// of t and restores the process defaults (and clears SetArgs) on cleanup. +// rootCmd is a package global; tests that drive it via Execute() must +// restore it so later tests are not affected. +func captureRootCmd(t *testing.T) *bytes.Buffer { + t.Helper() + var buf bytes.Buffer + rootCmd.SetOut(&buf) + rootCmd.SetErr(&buf) + t.Cleanup(func() { + rootCmd.SetOut(os.Stdout) + rootCmd.SetErr(os.Stderr) + rootCmd.SetArgs(nil) + }) + return &buf +} + +func TestAgentsCheckPassAllChecks(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no command on PATH for fixture") + } + res, err := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, + Title: "agents-check-pass", + Category: "general", + AgentSpec: workspace.AgentSpec{Type: "custom", Command: "go"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + out := buf.String() + for _, want := range []string{"[PASS]", "Agent type:", "Command:", "AGENTS.md:"} { + if !strings.Contains(out, want) { + t.Errorf("output missing %q\n%s", want, out) + } + } +} + +func TestAgentsCheckCommandNotFound(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, + Title: "no-cmd", + Category: "general", + AgentSpec: workspace.AgentSpec{Type: "custom", Command: "definitely-not-on-path-zzz"}, + }) + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + err := rootCmd.Execute() + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(buf.String(), "[FAIL]") { + t.Errorf("output missing [FAIL]:\n%s", buf.String()) + } +} + +func TestAgentsCheckMissingAgentsMD(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "no-agents-md", Category: "general", + AgentSpec: workspace.AgentSpec{Type: "custom", Command: "go"}, + }) + // Delete the AGENTS.md that workspace.Create just wrote. + _ = os.Remove(filepath.Join(res.Path, "AGENTS.md")) + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + err := rootCmd.Execute() + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(buf.String(), "AGENTS.md: [FAIL]") { + t.Errorf("output missing AGENTS.md FAIL:\n%s", buf.String()) + } +} + +func TestAgentsCheckMissingShimWarnsForClaude(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + // Use claude as the type but a known-on-PATH command override so the + // command check passes regardless of whether claude is installed. + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "claude-no-shim", Category: "general", + AgentSpec: workspace.AgentSpec{Type: "claude", Command: "go"}, + }) + _ = os.Remove(filepath.Join(res.Path, "CLAUDE.md")) + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("unexpected error (WARN should not bump failure count): %v", err) + } + out := buf.String() + if !strings.Contains(out, "CLAUDE.md: [WARN]") { + t.Errorf("output missing CLAUDE.md WARN line:\n%s", out) + } + if strings.Contains(out, "CLAUDE.md: [FAIL]") { + t.Errorf("CLAUDE.md missing must be WARN, not FAIL:\n%s", out) + } +} + +func TestAgentsCheckShimNotRequiredForOpencode(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "opencode-no-shim-needed", Category: "general", + // type: custom is a stand-in for opencode so the test does not + // need opencode on PATH. The shim check is gated on type=="claude", + // so any non-claude type must NOT emit a CLAUDE.md line. + AgentSpec: workspace.AgentSpec{Type: "custom", Command: "go"}, + }) + if _, err := os.Stat(filepath.Join(res.Path, "CLAUDE.md")); !os.IsNotExist(err) { + t.Fatalf("precondition: CLAUDE.md should not exist for non-claude, got %v", err) + } + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + _ = rootCmd.Execute() + if strings.Contains(buf.String(), "CLAUDE.md:") { + t.Errorf("non-claude workspace must not produce a CLAUDE.md row:\n%s", buf.String()) + } +} + +func TestAgentsCheckCustomNoCommand(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + // Create a normal workspace, then build a QueryResult with + // agent.type=custom and no command directly — ReadMeta would reject + // such a task.yaml, so we exercise the agents-check diagnostic itself. + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "custom-no-cmd-direct", Category: "general", + AgentSpec: workspace.AgentSpec{Type: "claude"}, + }) + qr := &workspace.QueryResult{ + Path: res.Path, + Root: tmpRoot, + Meta: &workspace.TaskMeta{ + Slug: res.Meta.Slug, + Agent: workspace.AgentSpec{Type: "custom"}, // no command + }, + } + var buf bytes.Buffer + err := runAgentsCheckOnWorkspace(&buf, qr) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(buf.String(), "requires command") { + t.Errorf("output must mention requires command:\n%s", buf.String()) + } +} + +func TestAgentsCheckShowsEnvKeysInfo(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "env-keys-info", Category: "general", + AgentSpec: workspace.AgentSpec{ + Type: "custom", + Command: "go", + Env: map[string]string{ + "OPENAI_API_KEY": "x", + "OPENAI_BASE_URL": "y", + }, + }, + }) + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + _ = rootCmd.Execute() + out := buf.String() + if !strings.Contains(out, "Agent env: 2 keys configured") { + t.Errorf("output missing env-keys count:\n%s", out) + } + if !strings.Contains(out, "OPENAI_API_KEY, OPENAI_BASE_URL") { + t.Errorf("output missing sorted key list:\n%s", out) + } +} + +func TestAgentsCheckWarnsOnCtaskEnvShadow(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + res, _ := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, Title: "ctask-env-shadow", Category: "general", + AgentSpec: workspace.AgentSpec{ + Type: "custom", + Command: "go", + Env: map[string]string{"CTASK_WORKSPACE": "shadowed"}, + }, + }) + + buf := captureRootCmd(t) + rootCmd.SetArgs([]string{"agents", "check", filepath.Base(res.Path)}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("unexpected error (shadow is WARN, not FAIL): %v", err) + } + if !strings.Contains(buf.String(), "[WARN] agent.env overrides ctask-exported vars") { + t.Errorf("output missing CTASK_* shadow WARN:\n%s", buf.String()) + } +} diff --git a/cmd/agents_doctor_test.go b/cmd/agents_doctor_test.go new file mode 100644 index 0000000..3c0c37f --- /dev/null +++ b/cmd/agents_doctor_test.go @@ -0,0 +1,49 @@ +package cmd + +import ( + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/config" + "github.com/warrenronsiek/ctask/internal/workspace" +) + +// This file swaps process-global os.Stdout and env vars via +// runDoctorCapture. Do not call t.Parallel() in this file. + +func TestDoctorIncludesAgentCheck(t *testing.T) { + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no PATH command") + } + tmpRoot := t.TempDir() + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + if _, err := workspace.Create(workspace.CreateOpts{ + Root: tmpRoot, + Title: "doctor-agent-test", + Category: "general", + AgentSpec: workspace.AgentSpec{Type: "custom", Command: "go"}, + }); err != nil { + t.Fatalf("Create: %v", err) + } + + // doctor returns an error if any check failed; assert on the printed + // block, not the exit code. + out, _ := runDoctorCapture(t, tmpRoot, "", "", false, false) + if !strings.Contains(out, "── Agent Check:") { + t.Errorf("doctor output missing Agent Check block:\n%s", out) + } +} + +func TestDoctorSkipsAgentCheckWhenNoWorkspace(t *testing.T) { + tmpRoot := t.TempDir() + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + // Do NOT create any workspaces. + + out, _ := runDoctorCapture(t, tmpRoot, "", "", false, false) + if !strings.Contains(out, "Agent check: skipped") { + t.Errorf("doctor output missing skip line:\n%s", out) + } +} diff --git a/cmd/archive_test.go b/cmd/archive_test.go index b99acb2..d45595e 100644 --- a/cmd/archive_test.go +++ b/cmd/archive_test.go @@ -22,16 +22,16 @@ func makeArchiveWs(t *testing.T, root, category, dirName string) string { now := time.Now().UTC().Truncate(time.Second) slug := dirName[11:] meta := &workspace.TaskMeta{ - ID: "test", - Slug: slug, - Title: slug, - CreatedAt: now, - UpdatedAt: now, - Status: "active", - Category: category, - Type: "task", - Mode: "local", - Agent: "claude", + ID: "test", + Slug: slug, + Title: slug, + CreatedAt: now, + UpdatedAt: now, + Status: "active", + Category: category, + Type: "task", + Mode: "local", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(dir, "task.yaml"), meta) return dir diff --git a/cmd/attach.go b/cmd/attach.go index 39a7cf5..a2be156 100644 --- a/cmd/attach.go +++ b/cmd/attach.go @@ -43,16 +43,16 @@ func runAttach(cmd *cobra.Command, args []string) error { return fmt.Errorf("updating metadata: %w", err) } - agent := attachAgent - if agent == "" { - agent = ws.Meta.Agent + resolved, err := resolveEntryAgent(ws.Meta.Agent, attachAgent) + if err != nil { + return err } return runWorkspaceEntry(WorkspaceEntryOptions{ WsPath: ws.Path, WsRoot: ws.Root, WsMeta: ws.Meta, - Agent: agent, + ResolvedAgent: resolved, Shell: false, // attach defaults to agent Force: attachForce, AlwaysPersistent: true, // attach is always tmux, regardless of env diff --git a/cmd/attach_test.go b/cmd/attach_test.go index c6903b9..1fdba09 100644 --- a/cmd/attach_test.go +++ b/cmd/attach_test.go @@ -56,7 +56,7 @@ func TestAttachForwardsToEntryHelperWithAlwaysPersistent(t *testing.T) { ID: "t", Slug: "attach-fwd-demo", Title: "attach-fwd-demo", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) diff --git a/cmd/completion_test.go b/cmd/completion_test.go index e9bab8d..f9bdf1a 100644 --- a/cmd/completion_test.go +++ b/cmd/completion_test.go @@ -27,7 +27,7 @@ func completionTestEnv(t *testing.T) string { ID: "t", Slug: slug, Title: slug, CreatedAt: now, UpdatedAt: now, Status: status, Category: category, Type: taskType, - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if status == "archived" { meta.ArchivedAt = &now @@ -197,6 +197,17 @@ func TestCompletionSubcommandViaExecute(t *testing.T) { defer rootCmd.SetOut(os.Stdout) defer rootCmd.SetErr(os.Stderr) + // Cobra's default `completion` command captures the root's output + // writer once, when it is first created on the first Execute() anywhere + // in the process (see InitDefaultCompletionCmd). Drop any previously + // created instance so the Execute() below re-creates it bound to buf — + // keeps this test independent of which other test ran Execute() first. + for _, c := range rootCmd.Commands() { + if c.Name() == "completion" { + rootCmd.RemoveCommand(c) + } + } + rootCmd.SetArgs([]string{"completion", "bash"}) defer rootCmd.SetArgs(nil) if err := rootCmd.Execute(); err != nil { diff --git a/cmd/delete_test.go b/cmd/delete_test.go index 0ac92eb..9a23480 100644 --- a/cmd/delete_test.go +++ b/cmd/delete_test.go @@ -21,15 +21,15 @@ func createTestWs(t *testing.T, root, category, dirName, status string) string { now := time.Now().UTC().Truncate(time.Second) slug := dirName[11:] // skip "YYYY-MM-DD_" meta := &workspace.TaskMeta{ - ID: "test", - Slug: slug, - Title: slug, - CreatedAt: now, - UpdatedAt: now, - Status: status, - Category: category, - Mode: "local", - Agent: "claude", + ID: "test", + Slug: slug, + Title: slug, + CreatedAt: now, + UpdatedAt: now, + Status: status, + Category: category, + Mode: "local", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(dir, "task.yaml"), meta) diff --git a/cmd/doctor.go b/cmd/doctor.go index 7dc9972..653f00b 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -207,6 +207,17 @@ func runDoctor(cmd *cobra.Command, args []string) error { // Check 9: tmux availability for persistent session mode (v0.5.3). checkTmux(&passed, &failed) + // Check 10: v0.6 — global config file + per-setting source attribution. + // Loaded once and reused across the whole settings block so doctor + // stays internally consistent (no re-reading the config file mid-render). + resolver := config.LoadResolver() + checkSettings(resolver, &passed, &failed) + + // Check 11: v0.6 — agent check when a workspace context is resolvable. + // Uses the most-recently-active workspace (mirrors `ctask last`); skips + // with an INFO line when no active workspace exists anywhere. + checkAgentForDoctor(&passed, &failed) + // Summary fmt.Println() fmt.Printf("%d checks passed, %d failed\n", passed, failed) @@ -259,6 +270,125 @@ func checkSeedDir(label, envValue, resolved, envName string, passed, failed *int *failed++ } +// checkSettings prints the v0.6 ── Settings ── block. The block shows +// the effective value of every user-level default tracked by the +// resolver, along with the source it was drawn from (built-in default, +// config file, env var, or platform override). When an env var +// overrides a config value, the override line displays both values so +// the user can see what they would inherit if they unset the env. +// +// The block also reports the config-file lifecycle state: +// - not found → INFO line only (no counters bumped) +// - found, valid → INFO line listing the path +// - found, invalid → FAIL line naming the offending key (failed++) +// +// Increments failed only for the invalid-config case. Passed is +// reserved for future per-setting checks (e.g., "ctask_root exists +// and is writable" — currently covered by Check 1). +// +// All values must come from the resolver, never re-read from env or +// disk inside this function — the resolver is the single source of +// truth and was already loaded once by runDoctor. +func checkSettings(r *config.Resolver, passed, failed *int) { + _ = passed // reserved (see doc comment) + fmt.Println() + fmt.Println("── Settings ──────────────────────────────────") + + // Config-file lifecycle line. + switch { + case r.ConfigErr != nil: + fmt.Printf("Config file: %s\n", r.ConfigPath) + fmt.Printf(" [FAIL] %s\n", r.ConfigErr) + fmt.Println(" Config file settings not applied — using env vars and built-in defaults only.") + *failed++ + case r.ConfigPath != "": + if _, err := os.Stat(r.ConfigPath); err == nil { + fmt.Printf("Config file: %s\n", r.ConfigPath) + } else { + fmt.Printf("Config file: not found — using built-in defaults (%s)\n", r.ConfigPath) + } + default: + fmt.Println("Config file: not found — using built-in defaults") + } + + // Per-setting lines. Each line: `: ` then a `source:` + // child line. When Override is non-nil the source line names the + // overridden source and value, matching the spec example + // `CTASK_AGENT env var (overrides config file: claude)`. + for _, s := range []config.ResolvedSetting{ + r.CtaskRoot(), + r.ProjectRoot(), + r.SeedDir(), + r.DefaultAgent(), + r.DefaultCategory(), + r.SessionMode(), + r.Editor(), + } { + fmt.Println() + printSettingLine(s) + } +} + +// printSettingLine renders one ResolvedSetting as a key/value/source +// trio, including override-chain context. Extracted so future settings +// (e.g., per-workspace launch session mode in info) can reuse it. +func printSettingLine(s config.ResolvedSetting) { + fmt.Printf("%s: %s\n", s.Key, s.Value) + fmt.Printf(" source: %s\n", formatSettingSource(s)) + if s.Source == config.PlatformOverride && s.Override != nil { + // Spec section 1.8 calls for an extra "configured: " + // row so the user sees what they asked for in addition to + // what's in effect. + fmt.Printf(" configured: %s\n", s.Override.Value) + } +} + +// formatSettingSource builds the "source: ..." annotation. The base +// label comes from the SettingSource.String(); env-var sources also +// surface the actual env-var name; PlatformOverride spells out why +// it fired; and when an override chain is present, the next-lower +// source's value is appended in parentheses so doctor renders lines +// like "CTASK_AGENT env var (overrides config file: claude)". +func formatSettingSource(s config.ResolvedSetting) string { + var base string + switch s.Source { + case config.EnvVar: + if s.EnvName != "" { + base = s.EnvName + " env var" + } else { + base = "env var" + } + case config.PlatformOverride: + base = "platform override (persistent mode requires tmux; not available on native Windows)" + default: + base = s.Source.String() + } + if s.Override != nil && s.Source != config.PlatformOverride { + base += fmt.Sprintf(" (overrides %s: %s)", s.Override.Source, s.Override.Value) + } + return base +} + +// checkAgentForDoctor runs the same validation as `ctask agents check` +// against the most-recently-active workspace. When no active workspace +// exists, prints a skip line. FAIL outcomes increment the failed counter +// (so doctor's overall exit code reflects agent breakage); WARN does not. +func checkAgentForDoctor(passed, failed *int) { + _ = passed // reserved for future symmetry with the other checks + fmt.Println() + roots := config.SearchRoots() + best, err := workspace.MostRecentActive(roots) + if err != nil || best == nil { + fmt.Println("Agent check: skipped (no workspace context)") + return + } + if checkErr := runAgentsCheckOnWorkspace(os.Stdout, best); checkErr != nil { + // runAgentsCheckOnWorkspace already printed the [FAIL] lines; just + // increment the counter so the summary reflects the breakage. + *failed++ + } +} + // checkTmux reports the three-state tmux check (v0.5.3): // - CTASK_SESSION_MODE != "persistent" -> INFO (direct mode, tmux optional) // - persistent + tmux on PATH + version OK -> two INFO lines diff --git a/cmd/doctor_settings_test.go b/cmd/doctor_settings_test.go new file mode 100644 index 0000000..185f0eb --- /dev/null +++ b/cmd/doctor_settings_test.go @@ -0,0 +1,179 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/config" +) + +// captureCheckSettings runs checkSettings with a fresh resolver state +// and returns (stdout, passed, failed). Each test supplies its own +// resolver via the config-path test seam. +func captureCheckSettings(t *testing.T) (string, int, int) { + t.Helper() + passed, failed := 0, 0 + out := captureStdout(t, func() { + r := config.LoadResolver() + checkSettings(r, &passed, &failed) + }) + return out, passed, failed +} + +// TestDoctorShowsSettingsSection — the new ── Settings ── header is +// printed and each resolver-tracked key surfaces. +func TestDoctorShowsSettingsSection(t *testing.T) { + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + clearResolverEnv(t) + + out, _, failed := captureCheckSettings(t) + if failed != 0 { + t.Errorf("no-config path should not increment failed counter, got %d", failed) + } + if !strings.Contains(out, "Settings") { + t.Errorf("expected Settings header in output, got %q", out) + } + // Each documented setting should appear by its config key name. + for _, key := range []string{"ctask_root", "default_agent", "session_mode", "seed_dir"} { + if !strings.Contains(out, key) { + t.Errorf("expected key %q in settings output, got %q", key, out) + } + } +} + +// TestDoctorShowsSourceAttribution — every settings line carries a +// "source:" annotation. +func TestDoctorShowsSourceAttribution(t *testing.T) { + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + clearResolverEnv(t) + + out, _, _ := captureCheckSettings(t) + // In the no-config / no-env state every setting falls back to its + // built-in default. Each source line must label that fact. + if strings.Count(out, "source:") < 4 { + t.Errorf("expected source: lines for at least 4 settings, got %q", out) + } + if !strings.Contains(out, "built-in default") { + t.Errorf("expected 'built-in default' source attribution, got %q", out) + } +} + +// TestDoctorShowsOverrides — when an env var overrides a config value, +// both surface in the doctor output (with the overridden value in +// parentheses). +func TestDoctorShowsOverrides(t *testing.T) { + clearResolverEnv(t) + // Plant a config file with default_agent: opencode. + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(cfgPath, []byte("default_agent: opencode\n"), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + config.SetConfigPathForTest(t, cfgPath) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + t.Setenv("CTASK_AGENT", "aider") + + out, _, _ := captureCheckSettings(t) + if !strings.Contains(out, "aider") { + t.Errorf("expected env-var value 'aider' in output, got %q", out) + } + if !strings.Contains(out, "CTASK_AGENT") { + t.Errorf("expected env-var name 'CTASK_AGENT' in source line, got %q", out) + } + if !strings.Contains(out, "overrides") { + t.Errorf("expected 'overrides' wording when env beats config, got %q", out) + } + if !strings.Contains(out, "opencode") { + t.Errorf("expected overridden config value 'opencode' in output, got %q", out) + } +} + +// TestDoctorConfigNotFound — missing config file is INFO, not FAIL. +func TestDoctorConfigNotFound(t *testing.T) { + clearResolverEnv(t) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "absent.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + out, _, failed := captureCheckSettings(t) + if failed != 0 { + t.Errorf("missing config should not fail, got failed=%d", failed) + } + if !strings.Contains(out, "Config file") { + t.Errorf("expected 'Config file' line, got %q", out) + } + if !strings.Contains(out, "not found") { + t.Errorf("expected 'not found' wording, got %q", out) + } + // "built-in defaults" should also be mentioned so the user knows + // what's actually in effect. + if !strings.Contains(out, "built-in") { + t.Errorf("expected mention of built-in defaults, got %q", out) + } +} + +// TestDoctorConfigInvalid — unknown key in config file marks the +// settings section as failed and names the offending key. +func TestDoctorConfigInvalid(t *testing.T) { + clearResolverEnv(t) + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(cfgPath, []byte("default_agnet: foo\n"), 0644); err != nil { + t.Fatalf("write: %v", err) + } + config.SetConfigPathForTest(t, cfgPath) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + out, _, failed := captureCheckSettings(t) + if failed < 1 { + t.Errorf("invalid config should increment failed counter; got %d", failed) + } + if !strings.Contains(out, "default_agnet") { + t.Errorf("expected unknown key name in output, got %q", out) + } + if !strings.Contains(out, "[FAIL]") { + t.Errorf("expected [FAIL] label on the invalid-config line, got %q", out) + } +} + +// TestDoctorSessionModePlatformOverrideRendered — when the platform +// override kicks in, the Settings section renders the final value +// (direct) plus the configured-before-override value. +func TestDoctorSessionModePlatformOverrideRendered(t *testing.T) { + clearResolverEnv(t) + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(cfgPath, []byte("session_mode: persistent\n"), 0644); err != nil { + t.Fatalf("write: %v", err) + } + config.SetConfigPathForTest(t, cfgPath) + config.SetIsNativeWindowsForTest(t, func() bool { return true }) + + out, _, _ := captureCheckSettings(t) + if !strings.Contains(out, "session_mode: direct") { + t.Errorf("expected 'session_mode: direct' after platform override, got %q", out) + } + if !strings.Contains(out, "platform override") { + t.Errorf("expected 'platform override' wording, got %q", out) + } + if !strings.Contains(out, "configured: persistent") { + t.Errorf("expected 'configured: persistent' line, got %q", out) + } +} + +// clearResolverEnv is the cmd-package counterpart to the helper in +// internal/config; resets every env var the resolver reads so a +// developer-host shell state cannot leak into the test. +func clearResolverEnv(t *testing.T) { + t.Helper() + for _, name := range []string{ + "CTASK_ROOT", "CTASK_PROJECT_ROOT", "CTASK_SEED_DIR", + "CTASK_AGENT", "CTASK_SESSION_MODE", "EDITOR", + } { + t.Setenv(name, "") + os.Unsetenv(name) + } +} diff --git a/cmd/doctor_test.go b/cmd/doctor_test.go index 054de11..8f6f1ed 100644 --- a/cmd/doctor_test.go +++ b/cmd/doctor_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/warrenronsiek/ctask/internal/config" ) // captureStdout runs fn while capturing os.Stdout and returns the output. @@ -196,6 +198,10 @@ func TestCheckTmuxConfiguredAndPresent(t *testing.T) { if _, err := exec.LookPath("tmux"); err != nil { t.Skip("tmux not on PATH") } + // Disable the v0.6 platform override so this test can verify the + // post-resolver doctor output for persistent mode without being + // short-circuited on native Windows. + config.SetIsNativeWindowsForTest(t, func() bool { return false }) os.Setenv("CTASK_SESSION_MODE", "persistent") defer os.Unsetenv("CTASK_SESSION_MODE") out := captureStdout(t, func() { @@ -211,6 +217,10 @@ func TestCheckTmuxConfiguredAndPresent(t *testing.T) { } func TestCheckTmuxConfiguredAndMissing(t *testing.T) { + // Disable the v0.6 platform override so the test reaches the + // tmux lookup path (rather than being short-circuited on native + // Windows hosts). + config.SetIsNativeWindowsForTest(t, func() bool { return false }) orig := os.Getenv("PATH") defer os.Setenv("PATH", orig) os.Setenv("PATH", "") diff --git a/cmd/entry.go b/cmd/entry.go index 91927c7..33725c4 100644 --- a/cmd/entry.go +++ b/cmd/entry.go @@ -7,6 +7,7 @@ import ( "path/filepath" "runtime" + "github.com/warrenronsiek/ctask/internal/agent" "github.com/warrenronsiek/ctask/internal/config" "github.com/warrenronsiek/ctask/internal/session" "github.com/warrenronsiek/ctask/internal/shell" @@ -23,14 +24,14 @@ type WorkspaceEntryOptions struct { WsPath string // absolute workspace directory WsRoot string // top-level root (used for CTASK_ROOT env var) WsMeta *workspace.TaskMeta // workspace metadata - Agent string - Shell bool // launch interactive shell (open / new --shell) - Force bool // bypass v0.4 Layer 1/3 prompts (owner-create only) - Direct bool // user passed --direct - AlwaysPersistent bool // ctask attach: ignore CTASK_SESSION_MODE - CommandName string // for hint rendering: "new" | "resume" | "open" | "attach" - TmuxPath string // pre-resolved tmux path; if empty in persistent mode, runWorkspaceEntry resolves - NewlyCreated bool // forwarded to LaunchOpts.NewlyCreated + ResolvedAgent *agent.Resolved // launch-ready agent (command + args + env) + Shell bool // launch interactive shell (open / new --shell) + Force bool // bypass v0.4 Layer 1/3 prompts (owner-create only) + Direct bool // user passed --direct + AlwaysPersistent bool // ctask attach: ignore CTASK_SESSION_MODE + CommandName string // for hint rendering: "new" | "resume" | "open" | "attach" + TmuxPath string // pre-resolved tmux path; if empty in persistent mode, runWorkspaceEntry resolves + NewlyCreated bool // forwarded to LaunchOpts.NewlyCreated } // runWorkspaceEntry is the test seam for the persistent-mode dispatcher. @@ -63,6 +64,13 @@ func dispatchPersistent(hasTmuxSession bool, leaseState session.LeaseState) disp } func defaultRunWorkspaceEntry(opts WorkspaceEntryOptions) error { + // v0.6: if the resolver downgraded a configured persistent mode to + // direct via the native-Windows platform override, surface the fact + // to the user once before any launch work happens. attach + // (AlwaysPersistent=true) is excluded — it has no direct-mode + // fallback and refuses on native Windows via preflightPersistentEntry. + emitPlatformOverrideWarningIfNeeded(opts.AlwaysPersistent) + mode := config.ResolveSessionMode() persistent := opts.AlwaysPersistent || (mode == "persistent" && !opts.Direct) @@ -107,6 +115,18 @@ func defaultRunWorkspaceEntry(opts WorkspaceEntryOptions) error { return fmt.Errorf("internal: unreachable persistent dispatch") } +// resolveEntryAgent builds the launch-ready agent for an entry command +// (resume / last / open / attach). It starts from the workspace's +// AgentSpec, applies an optional one-shot agent.command override (the +// --agent flag — a command override, NOT a type selector, per v0.6 +// Open Question 1), then resolves against the user-level default_agent. +func resolveEntryAgent(spec workspace.AgentSpec, commandOverride string) (*agent.Resolved, error) { + if commandOverride != "" { + spec.Command = commandOverride + } + return agent.Resolve(spec, config.LoadResolver().DefaultAgent().Value) +} + func entryEnvVars(opts WorkspaceEntryOptions) map[string]string { return config.EnvVars( opts.WsMeta.Slug, opts.WsMeta.Mode, @@ -120,7 +140,7 @@ func invokeDirectRun(opts WorkspaceEntryOptions) error { return session.Run(session.LaunchOpts{ WsDir: opts.WsPath, EnvVars: entryEnvVars(opts), - Agent: opts.Agent, + ResolvedAgent: opts.ResolvedAgent, Mode: opts.WsMeta.Mode, Slug: opts.WsMeta.Slug, Shell: opts.Shell, @@ -168,35 +188,35 @@ func formatDirectModeTmuxHint(slug string) string { func invokePersistentRun(opts WorkspaceEntryOptions, tmuxPath, sessionName string) error { return session.Run(session.LaunchOpts{ - WsDir: opts.WsPath, - EnvVars: entryEnvVars(opts), - Agent: opts.Agent, - Mode: opts.WsMeta.Mode, - Slug: opts.WsMeta.Slug, - Shell: opts.Shell, - LaunchDir: opts.WsMeta.LaunchDir, - Category: opts.WsMeta.Category, - SessionMode: "persistent", - SessionName: sessionName, - TmuxPath: tmuxPath, - Force: opts.Force, - NewlyCreated: opts.NewlyCreated, + WsDir: opts.WsPath, + EnvVars: entryEnvVars(opts), + ResolvedAgent: opts.ResolvedAgent, + Mode: opts.WsMeta.Mode, + Slug: opts.WsMeta.Slug, + Shell: opts.Shell, + LaunchDir: opts.WsMeta.LaunchDir, + Category: opts.WsMeta.Category, + SessionMode: "persistent", + SessionName: sessionName, + TmuxPath: tmuxPath, + Force: opts.Force, + NewlyCreated: opts.NewlyCreated, }) } func invokePersistentAdoption(opts WorkspaceEntryOptions, tmuxPath, sessionName string) error { return session.AdoptExistingPersistentSession(tmuxPath, sessionName, opts.WsPath, session.LaunchOpts{ - WsDir: opts.WsPath, - EnvVars: entryEnvVars(opts), - Agent: opts.Agent, - Mode: opts.WsMeta.Mode, - Slug: opts.WsMeta.Slug, - Shell: opts.Shell, - LaunchDir: opts.WsMeta.LaunchDir, - Category: opts.WsMeta.Category, - SessionMode: "persistent", - SessionName: sessionName, - TmuxPath: tmuxPath, + WsDir: opts.WsPath, + EnvVars: entryEnvVars(opts), + ResolvedAgent: opts.ResolvedAgent, + Mode: opts.WsMeta.Mode, + Slug: opts.WsMeta.Slug, + Shell: opts.Shell, + LaunchDir: opts.WsMeta.LaunchDir, + Category: opts.WsMeta.Category, + SessionMode: "persistent", + SessionName: sessionName, + TmuxPath: tmuxPath, }) } diff --git a/cmd/entry_test.go b/cmd/entry_test.go index e276009..bf69a1e 100644 --- a/cmd/entry_test.go +++ b/cmd/entry_test.go @@ -4,6 +4,7 @@ import ( "path/filepath" "testing" + "github.com/warrenronsiek/ctask/internal/agent" "github.com/warrenronsiek/ctask/internal/session" "github.com/warrenronsiek/ctask/internal/workspace" ) @@ -63,12 +64,12 @@ func TestRunWorkspaceEntryIsInjectable(t *testing.T) { t.Cleanup(func() { runWorkspaceEntry = orig }) want := WorkspaceEntryOptions{ - WsPath: "/tmp/ws", - WsRoot: "/tmp", - WsMeta: &workspace.TaskMeta{Slug: "demo", Category: "projects", Mode: "local", Agent: "claude"}, - Agent: "claude", - Shell: true, - CommandName: "test", + WsPath: "/tmp/ws", + WsRoot: "/tmp", + WsMeta: &workspace.TaskMeta{Slug: "demo", Category: "projects", Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}}, + ResolvedAgent: &agent.Resolved{Command: "claude"}, + Shell: true, + CommandName: "test", } if err := runWorkspaceEntry(want); err != nil { t.Fatalf("runWorkspaceEntry: %v", err) diff --git a/cmd/info.go b/cmd/info.go index cc2579d..a6a0668 100644 --- a/cmd/info.go +++ b/cmd/info.go @@ -32,12 +32,20 @@ func runInfo(cmd *cobra.Command, args []string) error { ws := resolveOne(roots, args[0], true) m := ws.Meta + // v0.6: load the resolver once and reuse it across this info + // invocation. The Agent line gains a workspace-vs-default source + // label; a new Launch session mode line surfaces the configured + // launch default with its own source attribution. + resolver := config.LoadResolver() + fmt.Printf("Task: %s\n", m.Slug) fmt.Printf("Title: %s\n", m.Title) fmt.Printf("Category: %s\n", m.Category) fmt.Printf("Status: %s\n", m.Status) fmt.Printf("Mode: %s\n", m.Mode) - fmt.Printf("Agent: %s\n", m.Agent) + fmt.Printf("Agent: %s\n", agentLineWithSource(m.Agent.Type, resolver)) + fmt.Printf("Launch session mode: %s (%s)\n", + resolver.SessionMode().Value, infoSourceLabel(resolver.SessionMode())) // v0.5.1: display timestamps in local time. task.yaml stores UTC; // info converts for friendliness so the shown time matches the user's // wall clock. @@ -85,6 +93,48 @@ func runInfo(cmd *cobra.Command, args []string) error { return nil } +// agentLineWithSource builds the value portion of info's Agent line +// with v0.6 source attribution. When task.yaml has a non-empty agent +// field, that value is workspace state and surfaces as +// " (workspace)". When the field is empty (legacy or +// hand-crafted task.yaml), the line falls through to the user-level +// default chain and is labelled " (default)" plus the source +// when the default did NOT come from the built-in. +// +// The fallback path is informational only: ctask new always writes +// the resolved default into task.yaml, so the legacy branch never +// fires for workspaces created by recent versions. +func agentLineWithSource(workspaceAgent string, r *config.Resolver) string { + if workspaceAgent != "" { + return workspaceAgent + " (workspace)" + } + s := r.DefaultAgent() + if s.Source == config.Builtin { + return s.Value + " (default)" + } + return fmt.Sprintf("%s (default — %s)", s.Value, infoSourceLabel(s)) +} + +// infoSourceLabel renders a setting's source for info output. Mirrors +// formatSettingSource in doctor.go but without the override-chain +// suffix; info lines are single-row and cannot fit the extra "(...)" +// payload cleanly. Env-var sources surface their CTASK_X env-var name +// (more specific than the bare "env var" label) so the user can spot +// which shell variable to inspect. +func infoSourceLabel(s config.ResolvedSetting) string { + switch s.Source { + case config.EnvVar: + if s.EnvName != "" { + return s.EnvName + " env var" + } + return "env var" + case config.PlatformOverride: + return "platform override" + default: + return s.Source.String() + } +} + // printSessionBlock renders the v0.5.4 Session block for `ctask info`. // // Layout (values align at column 14 across the block): diff --git a/cmd/info_attribution_test.go b/cmd/info_attribution_test.go new file mode 100644 index 0000000..dd10af9 --- /dev/null +++ b/cmd/info_attribution_test.go @@ -0,0 +1,187 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/warrenronsiek/ctask/internal/config" + "github.com/warrenronsiek/ctask/internal/workspace" +) + +// makeTestWorkspace plants a minimal workspace at root with the +// given meta and returns the directory path. +func makeTestWorkspace(t *testing.T, root string, m *workspace.TaskMeta) string { + t.Helper() + wsDir := filepath.Join(root, m.Category, "2026-05-14_"+m.Slug) + if err := os.MkdirAll(wsDir, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), m); err != nil { + t.Fatalf("WriteMeta: %v", err) + } + return wsDir +} + +// TestInfoAgentSourceWorkspace — when task.yaml has an agent field, info +// labels the Agent line with the (workspace) source. +func TestInfoAgentSourceWorkspace(t *testing.T) { + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + clearResolverEnv(t) + + root := t.TempDir() + now := time.Now().UTC().Truncate(time.Second) + makeTestWorkspace(t, root, &workspace.TaskMeta{ + ID: "t", Slug: "agentwsdemo", Title: "agentwsdemo", + CreatedAt: now, UpdatedAt: now, + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: "opencode"}, + }) + + out, err := runInfoCapture(t, root, "agentwsdemo") + if err != nil { + t.Fatalf("runInfo: %v", err) + } + if !strings.Contains(out, "Agent:") { + t.Fatalf("output missing Agent line:\n%s", out) + } + if !strings.Contains(out, "opencode") { + t.Errorf("expected agent value 'opencode' in output:\n%s", out) + } + if !strings.Contains(out, "(workspace)") { + t.Errorf("expected (workspace) source label, got:\n%s", out) + } +} + +// TestInfoAgentSourceDefaultForLegacy — task.yaml with empty agent (a +// legacy or hand-crafted workspace) falls through to the resolver and +// is labelled with the default source. +func TestInfoAgentSourceDefaultForLegacy(t *testing.T) { + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + clearResolverEnv(t) + + root := t.TempDir() + now := time.Now().UTC().Truncate(time.Second) + makeTestWorkspace(t, root, &workspace.TaskMeta{ + ID: "t", Slug: "legacyagent", Title: "legacy agent", + CreatedAt: now, UpdatedAt: now, + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: ""}, // legacy: no agent recorded + }) + + out, err := runInfoCapture(t, root, "legacyagent") + if err != nil { + t.Fatalf("runInfo: %v", err) + } + if !strings.Contains(out, "Agent:") { + t.Fatalf("output missing Agent line:\n%s", out) + } + if !strings.Contains(out, "claude") { + t.Errorf("expected fallback agent 'claude' in output:\n%s", out) + } + if !strings.Contains(out, "(default)") { + t.Errorf("expected (default) source label, got:\n%s", out) + } +} + +// TestInfoLaunchSessionModeFromConfig — the new "Launch session mode" +// line surfaces between Agent and Created, with its source label. +func TestInfoLaunchSessionModeFromConfig(t *testing.T) { + clearResolverEnv(t) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(cfgPath, []byte("session_mode: persistent\n"), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + config.SetConfigPathForTest(t, cfgPath) + + root := t.TempDir() + now := time.Now().UTC().Truncate(time.Second) + makeTestWorkspace(t, root, &workspace.TaskMeta{ + ID: "t", Slug: "modesrc", Title: "mode src", + CreatedAt: now, UpdatedAt: now, + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, + }) + + out, err := runInfoCapture(t, root, "modesrc") + if err != nil { + t.Fatalf("runInfo: %v", err) + } + if !strings.Contains(out, "Launch session mode:") { + t.Errorf("expected 'Launch session mode:' line, got:\n%s", out) + } + if !strings.Contains(out, "persistent") { + t.Errorf("expected mode value 'persistent', got:\n%s", out) + } + if !strings.Contains(out, "config file") { + t.Errorf("expected 'config file' source label, got:\n%s", out) + } +} + +// TestInfoLaunchSessionModeBuiltinDefault — without env/config, the +// line shows the built-in default with the right source label. +func TestInfoLaunchSessionModeBuiltinDefault(t *testing.T) { + clearResolverEnv(t) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + root := t.TempDir() + now := time.Now().UTC().Truncate(time.Second) + makeTestWorkspace(t, root, &workspace.TaskMeta{ + ID: "t", Slug: "modedefault", Title: "mode default", + CreatedAt: now, UpdatedAt: now, + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, + }) + + out, err := runInfoCapture(t, root, "modedefault") + if err != nil { + t.Fatalf("runInfo: %v", err) + } + if !strings.Contains(out, "Launch session mode: direct") { + t.Errorf("expected 'Launch session mode: direct', got:\n%s", out) + } + if !strings.Contains(out, "built-in default") { + t.Errorf("expected 'built-in default' source label, got:\n%s", out) + } +} + +// TestInfoLaunchSessionModeAfterAgentBeforeCreated — placement check: +// the line lives between Agent and Created, outside the v0.5.4 Session +// block. +func TestInfoLaunchSessionModeAfterAgentBeforeCreated(t *testing.T) { + clearResolverEnv(t) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + root := t.TempDir() + now := time.Now().UTC().Truncate(time.Second) + makeTestWorkspace(t, root, &workspace.TaskMeta{ + ID: "t", Slug: "placement", Title: "placement", + CreatedAt: now, UpdatedAt: now, + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, + }) + + out, err := runInfoCapture(t, root, "placement") + if err != nil { + t.Fatalf("runInfo: %v", err) + } + agentIdx := strings.Index(out, "Agent:") + modeIdx := strings.Index(out, "Launch session mode:") + createdIdx := strings.Index(out, "Created:") + if agentIdx < 0 || modeIdx < 0 || createdIdx < 0 { + t.Fatalf("missing one of Agent/Launch session mode/Created:\n%s", out) + } + if !(agentIdx < modeIdx && modeIdx < createdIdx) { + t.Errorf("expected Agent < Launch session mode < Created order; got Agent@%d Mode@%d Created@%d:\n%s", + agentIdx, modeIdx, createdIdx, out) + } +} diff --git a/cmd/info_launch_test.go b/cmd/info_launch_test.go index 118585e..c8ce3b9 100644 --- a/cmd/info_launch_test.go +++ b/cmd/info_launch_test.go @@ -46,7 +46,7 @@ func TestInfoShowsLaunchFieldsWhenLaunchDirSet(t *testing.T) { ID: "t", Slug: "demo", Title: "demo", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "projects", Type: "project", - Mode: "local", Agent: "claude", LaunchDir: "demo", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, LaunchDir: "demo", } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) @@ -73,7 +73,7 @@ func TestInfoOmitsLaunchFieldsForTask(t *testing.T) { ID: "t", Slug: "regular", Title: "regular", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) @@ -104,8 +104,8 @@ func TestInfoFormatsTimestampsInLocalZone(t *testing.T) { ID: "t", Slug: "tz-test", Title: "tz-test", CreatedAt: fixedUTC, UpdatedAt: fixedUTC, ArchivedAt: &archived, - Status: "archived", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Status: "archived", Category: "general", Type: "task", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) @@ -153,7 +153,7 @@ func TestInfoFindsArchivedWorkspaceWithoutFlag(t *testing.T) { Category: "general", Type: "task", Mode: "local", - Agent: "claude", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) @@ -179,7 +179,7 @@ func TestInfoShowsDirExistsNoWhenLaunchDirMissing(t *testing.T) { ID: "t", Slug: "renamed", Title: "renamed", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "projects", Type: "project", - Mode: "local", Agent: "claude", LaunchDir: "renamed", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, LaunchDir: "renamed", } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) diff --git a/cmd/info_session_test.go b/cmd/info_session_test.go index a3c38b9..114baf7 100644 --- a/cmd/info_session_test.go +++ b/cmd/info_session_test.go @@ -2,6 +2,7 @@ package cmd import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -29,7 +30,7 @@ func makeInfoSessionWorkspace(t *testing.T, root, slug string) string { ID: "t", Slug: slug, Title: slug, CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) @@ -61,8 +62,12 @@ func TestInfoShowsActivePersistentSessionWithAttachHint(t *testing.T) { wsDir := makeInfoSessionWorkspace(t, root, "active-persist") now := time.Now().UTC().Truncate(time.Second) + // An active session is owned by a live process: lease freshness is + // PID-aware (v0.6 Phase 3), so a local-hostname lease must point at a + // live PID to read as active. Use the test process itself. + livePID := os.Getpid() writeLeaseAtForCmdTest(t, wsDir, &session.Lease{ - PID: 12345, + PID: livePID, Hostname: session.CurrentHostname(), Mode: "persistent", StartedAt: now, @@ -76,7 +81,7 @@ func TestInfoShowsActivePersistentSessionWithAttachHint(t *testing.T) { for _, want := range []string{ "Session: active", "Mode: persistent", - "Owner: pid 12345", // local host -> hostname omitted + fmt.Sprintf("Owner: pid %d", livePID), // local host -> hostname omitted "Attach: ctask attach active-persist", } { if !strings.Contains(out, want) { @@ -91,8 +96,10 @@ func TestInfoShowsActiveDirectSessionWithoutAttachHint(t *testing.T) { wsDir := makeInfoSessionWorkspace(t, root, "active-direct") now := time.Now().UTC().Truncate(time.Second) + // Live PID required for an active local-hostname lease — see + // TestInfoShowsActivePersistentSessionWithAttachHint. writeLeaseAtForCmdTest(t, wsDir, &session.Lease{ - PID: 8888, + PID: os.Getpid(), Hostname: session.CurrentHostname(), Mode: "direct", StartedAt: now, diff --git a/cmd/list_session_test.go b/cmd/list_session_test.go index 1ef27a3..b7691a9 100644 --- a/cmd/list_session_test.go +++ b/cmd/list_session_test.go @@ -26,7 +26,7 @@ func makeListSessionWorkspace(t *testing.T, root, category, dirName, slug, statu ID: "t", Slug: slug, Title: slug, CreatedAt: now, UpdatedAt: now, Status: status, Category: category, Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if status == "archived" { meta.ArchivedAt = &now @@ -47,17 +47,25 @@ func TestListSessionColumnShowsModeAndStaleAndDash(t *testing.T) { staleWS := makeListSessionWorkspace(t, root, "general", "2026-05-12_stale-ws", "stale-ws", "active") makeListSessionWorkspace(t, root, "general", "2026-05-11_idle-ws", "idle-ws", "active") + // persist-ws and direct-ws must read as active sessions: a fresh + // heartbeat alone is not enough now that lease freshness is PID-aware + // (v0.6 Phase 3). Use the live test process PID so the local-hostname + // lease passes the PID-liveness check, mirroring + // session.TestInspectLeaseFreshLocal. + livePID := os.Getpid() writeLeaseAtForCmdTest(t, persistWS, &session.Lease{ - PID: 1, Hostname: host, Mode: "persistent", + PID: livePID, Hostname: host, Mode: "persistent", StartedAt: now, LastHeartbeatAt: now, }) writeLeaseAtForCmdTest(t, directWS, &session.Lease{ - PID: 2, Hostname: host, Mode: "direct", + PID: livePID, Hostname: host, Mode: "direct", StartedAt: now, LastHeartbeatAt: now, }) + // stale-ws is wall-clock stale (heartbeat 10m ago); its PID is + // irrelevant — wall-clock staleness wins unconditionally. heartbeat := now.Add(-10 * time.Minute) writeLeaseAtForCmdTest(t, staleWS, &session.Lease{ - PID: 3, Hostname: host, Mode: "direct", + PID: livePID, Hostname: host, Mode: "direct", StartedAt: heartbeat, LastHeartbeatAt: heartbeat, }) diff --git a/cmd/list_test.go b/cmd/list_test.go index afd5abc..f22c7ee 100644 --- a/cmd/list_test.go +++ b/cmd/list_test.go @@ -24,16 +24,16 @@ func listTestEnv(t *testing.T) string { now := time.Now().UTC().Truncate(time.Second) slug := dirName[11:] meta := &workspace.TaskMeta{ - ID: "test", - Slug: slug, - Title: slug, - CreatedAt: now, - UpdatedAt: now, - Status: status, - Category: category, - Type: taskType, - Mode: "local", - Agent: "claude", + ID: "test", + Slug: slug, + Title: slug, + CreatedAt: now, + UpdatedAt: now, + Status: status, + Category: category, + Type: taskType, + Mode: "local", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(dir, "task.yaml"), meta) } diff --git a/cmd/new.go b/cmd/new.go index e75b53a..1769a56 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -5,6 +5,7 @@ import ( "os" "github.com/spf13/cobra" + "github.com/warrenronsiek/ctask/internal/agent" "github.com/warrenronsiek/ctask/internal/config" "github.com/warrenronsiek/ctask/internal/shell" "github.com/warrenronsiek/ctask/internal/workspace" @@ -34,7 +35,7 @@ func init() { newCmd.Flags().BoolVar(&newProject, "project", false, "Create a project workspace (longer-lived; uses CTASK_PROJECT_ROOT if set, runs git init)") newCmd.Flags().BoolVar(&newContainer, "container", false, "Launch in container sandbox (deferred)") newCmd.Flags().BoolVar(&newShell, "shell", false, "Open interactive shell instead of agent") - newCmd.Flags().StringVarP(&newAgent, "agent", "a", "", "Command to exec as the agent") + newCmd.Flags().StringVarP(&newAgent, "agent", "a", "", "Agent type for the workspace (claude, opencode, custom)") newCmd.Flags().BoolVar(&newNoLaunch, "no-launch", false, "Create workspace only, do not launch") newCmd.Flags().BoolVar(&newDirect, "direct", false, "Bypass persistent session mode for this command") rootCmd.AddCommand(newCmd) @@ -53,9 +54,22 @@ func runNew(cmd *cobra.Command, args []string) error { return err } - agent := newAgent - if agent == "" { - agent = config.ResolveAgent() + // --agent on `new` is a type selector (v0.6 spec §5): the resolved + // type is recorded in task.yaml and drives the launch. SetCLIFlagAgent + // layers the flag above CTASK_AGENT / config / builtin so doctor and + // info can render the precedence chain. Resolution and validation run + // BEFORE workspace.Create so an invalid agent (unknown type, or + // `--agent custom` with no command) refuses without leaving a + // half-created workspace on disk. + resolver := config.LoadResolver() + if cmd.Flags().Changed("agent") { + resolver.SetCLIFlagAgent(newAgent) + } + agentType := resolver.DefaultAgent().Value + spec := workspace.AgentSpec{Type: agentType} + resolved, err := agent.Resolve(spec, agentType) + if err != nil { + return err } title := "" @@ -92,7 +106,7 @@ func runNew(cmd *cobra.Command, args []string) error { Title: title, Category: category, Mode: "local", - Agent: agent, + AgentSpec: spec, IsProject: newProject, SeedDir: config.ResolveSeedDir(), SkipCategoryDir: skipCategoryDir, @@ -132,15 +146,15 @@ func runNew(cmd *cobra.Command, args []string) error { // differ from ws-derived defaults when --project + CTASK_PROJECT_ROOT // are in play). return runWorkspaceEntry(WorkspaceEntryOptions{ - WsPath: ws.Path, - WsRoot: root, - WsMeta: ws.Meta, - Agent: agent, - Shell: newShell, - Direct: newDirect, - CommandName: "new", - TmuxPath: tmuxPath, - NewlyCreated: true, + WsPath: ws.Path, + WsRoot: root, + WsMeta: ws.Meta, + ResolvedAgent: resolved, + Shell: newShell, + Direct: newDirect, + CommandName: "new", + TmuxPath: tmuxPath, + NewlyCreated: true, }) } diff --git a/cmd/new_agent_flag_test.go b/cmd/new_agent_flag_test.go new file mode 100644 index 0000000..5d54b7d --- /dev/null +++ b/cmd/new_agent_flag_test.go @@ -0,0 +1,62 @@ +package cmd + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/config" + "github.com/warrenronsiek/ctask/internal/workspace" +) + +func TestNewAgentFlagWritesTypeToTaskYaml(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + t.Setenv("CTASK_AGENT", "opencode") // env says opencode + + // Replace runWorkspaceEntry so the test does not try to launch. + prev := runWorkspaceEntry + runWorkspaceEntry = func(WorkspaceEntryOptions) error { return nil } + t.Cleanup(func() { runWorkspaceEntry = prev }) + captureRootCmd(t) // restores rootCmd SetArgs/SetOut on cleanup + + rootCmd.SetArgs([]string{"new", "agent-flag-test", "--agent", "claude"}) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + + matches, _ := filepath.Glob(filepath.Join(tmpRoot, "general", "*_agent-flag-test")) + if len(matches) != 1 { + t.Fatalf("workspace dir not found, got %v", matches) + } + meta, err := workspace.ReadMeta(filepath.Join(matches[0], "task.yaml")) + if err != nil { + t.Fatalf("ReadMeta: %v", err) + } + if meta.Agent.Type != "claude" { + t.Errorf("agent.type = %q, want claude (CLI flag wins over env)", meta.Agent.Type) + } +} + +func TestNewAgentCustomWithoutCommandRefused(t *testing.T) { + tmpRoot := t.TempDir() + t.Setenv("CTASK_ROOT", tmpRoot) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + + prev := runWorkspaceEntry + runWorkspaceEntry = func(WorkspaceEntryOptions) error { return nil } + t.Cleanup(func() { runWorkspaceEntry = prev }) + captureRootCmd(t) // restores rootCmd SetArgs/SetOut on cleanup + + rootCmd.SetArgs([]string{"new", "custom-no-cmd", "--agent", "custom"}) + err := rootCmd.Execute() + if err == nil || !strings.Contains(err.Error(), "requires command") { + t.Fatalf("err = %v, want error mentioning requires command", err) + } + // Workspace should NOT have been created. + matches, _ := filepath.Glob(filepath.Join(tmpRoot, "general", "*_custom-no-cmd")) + if len(matches) != 0 { + t.Errorf("workspace dir created despite refusal: %v", matches) + } +} diff --git a/cmd/new_persistent_test.go b/cmd/new_persistent_test.go index 015acae..3c337f5 100644 --- a/cmd/new_persistent_test.go +++ b/cmd/new_persistent_test.go @@ -4,6 +4,8 @@ import ( "os" "strings" "testing" + + "github.com/warrenronsiek/ctask/internal/config" ) // Tests in this file mutate package globals (isTTYCheck, runWorkspaceEntry). @@ -18,6 +20,12 @@ func TestNewDirectModeSkipsPreflight(t *testing.T) { } func TestNewDirectFlagUnderPersistentEmitsWarningAndProceeds(t *testing.T) { + // Disable the v0.6 platform override: this test verifies the + // --direct bypass warning that fires when persistent mode is in + // effect. On native Windows the resolver coerces persistent → direct + // before this function sees it, which is correct production + // behavior but bypasses the codepath under test. + config.SetIsNativeWindowsForTest(t, func() bool { return false }) os.Setenv("CTASK_SESSION_MODE", "persistent") t.Cleanup(func() { os.Unsetenv("CTASK_SESSION_MODE") }) diff --git a/cmd/notes_test.go b/cmd/notes_test.go index 6adcb53..11fa31a 100644 --- a/cmd/notes_test.go +++ b/cmd/notes_test.go @@ -57,7 +57,7 @@ func makeNotesWs(t *testing.T, root, category, dirName, status, notesBody string ID: "t", Slug: slug, Title: slug, CreatedAt: now, UpdatedAt: now, Status: status, Category: category, Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if status == "archived" { meta.ArchivedAt = &now diff --git a/cmd/open.go b/cmd/open.go index 3993137..f385dff 100644 --- a/cmd/open.go +++ b/cmd/open.go @@ -47,14 +47,19 @@ func runOpen(cmd *cobra.Command, args []string) error { return fmt.Errorf("updating metadata: %w", err) } + resolved, err := resolveEntryAgent(ws.Meta.Agent, "") + if err != nil { + return err + } + return runWorkspaceEntry(WorkspaceEntryOptions{ - WsPath: ws.Path, - WsRoot: ws.Root, - WsMeta: ws.Meta, - Agent: ws.Meta.Agent, - Shell: true, // open always launches a shell - Force: openForce, - Direct: openDirect, - CommandName: "open", + WsPath: ws.Path, + WsRoot: ws.Root, + WsMeta: ws.Meta, + ResolvedAgent: resolved, + Shell: true, // open always launches a shell + Force: openForce, + Direct: openDirect, + CommandName: "open", }) } diff --git a/cmd/open_persistent_test.go b/cmd/open_persistent_test.go index 13796d9..6537fb8 100644 --- a/cmd/open_persistent_test.go +++ b/cmd/open_persistent_test.go @@ -35,7 +35,7 @@ func TestOpenForwardsToEntryHelperWithShellTrue(t *testing.T) { ID: "t", Slug: "open-fwd-demo", Title: "open-fwd-demo", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) diff --git a/cmd/path_test.go b/cmd/path_test.go index 08c90a1..b44f1be 100644 --- a/cmd/path_test.go +++ b/cmd/path_test.go @@ -47,7 +47,7 @@ func TestPathPrintsAbsolutePath(t *testing.T) { ID: "t", Slug: "path-active", Title: "path-active", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) @@ -83,7 +83,7 @@ func TestPathWorksOnArchivedWorkspace(t *testing.T) { Category: "general", Type: "task", Mode: "local", - Agent: "claude", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) diff --git a/cmd/persistent.go b/cmd/persistent.go index 3d46f03..b94a324 100644 --- a/cmd/persistent.go +++ b/cmd/persistent.go @@ -7,6 +7,7 @@ import ( "runtime" "time" + "github.com/warrenronsiek/ctask/internal/config" "github.com/warrenronsiek/ctask/internal/session" "github.com/warrenronsiek/ctask/internal/shell" ) @@ -17,6 +18,38 @@ import ( // defaultIsTTYCheck. var isTTYCheck = defaultIsTTYCheck +// platformOverrideWarning is the exact stderr line emitted when the v0.6 +// session_mode platform override (persistent → direct on native Windows) +// kicks in. Kept as a package-level string so tests can reference it +// without re-typing the user-facing copy. +const platformOverrideWarning = "[ctask] warning: persistent session mode is not supported on native Windows; using direct mode. Use WSL for persistent sessions." + +// emitPlatformOverrideWarningIfNeeded prints the platform-override +// warning once per invocation when: +// 1. the workspace entry is NOT marked AlwaysPersistent (attach has no +// direct-mode fallback and refuses via preflight rather than +// downgrading), AND +// 2. the resolver's session_mode value resolved through the +// PlatformOverride source — i.e. the user asked for persistent mode +// via config or env but the host is native Windows. +// +// Doctor and info must NOT call this helper. They render source +// attribution explicitly in their own output; emitting an additional +// stderr line from a diagnostic command would be noise. +// +// "Once per invocation" is provided implicitly by the call site in +// defaultRunWorkspaceEntry, which runs at most once per ctask command. +// No warn-once subsystem is needed in v0.6. +func emitPlatformOverrideWarningIfNeeded(alwaysPersistent bool) { + if alwaysPersistent { + return + } + s := config.LoadResolver().SessionMode() + if s.Source == config.PlatformOverride && s.Value == "direct" { + fmt.Fprintln(os.Stderr, platformOverrideWarning) + } +} + func defaultIsTTYCheck() bool { return shell.IsTTY(os.Stdin) && shell.IsTTY(os.Stdout) } diff --git a/cmd/platform_warning_test.go b/cmd/platform_warning_test.go new file mode 100644 index 0000000..0d07703 --- /dev/null +++ b/cmd/platform_warning_test.go @@ -0,0 +1,128 @@ +package cmd + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/config" +) + +// captureStderr swaps os.Stderr for a pipe, runs fn, and returns +// what was written. Mirrors captureStdout in doctor_test.go. +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + prev := os.Stderr + os.Stderr = w + defer func() { os.Stderr = prev }() + fn() + w.Close() + data, _ := io.ReadAll(r) + return string(data) +} + +// writeSessionModeConfig plants a config.yaml with the given +// session_mode value and points the resolver at it. +func writeSessionModeConfig(t *testing.T, mode string) { + t.Helper() + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + body := "session_mode: " + mode + "\n" + if err := os.WriteFile(cfgPath, []byte(body), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + config.SetConfigPathForTest(t, cfgPath) +} + +// TestPlatformOverrideWarningEmittedOnLaunch — a launch/entry path +// (AlwaysPersistent=false) running on simulated native Windows with +// a configured persistent session_mode receives the one-line stderr +// warning describing the downgrade. +func TestPlatformOverrideWarningEmittedOnLaunch(t *testing.T) { + clearResolverEnv(t) + writeSessionModeConfig(t, "persistent") + config.SetIsNativeWindowsForTest(t, func() bool { return true }) + + out := captureStderr(t, func() { + emitPlatformOverrideWarningIfNeeded(false) + }) + if !strings.Contains(out, "persistent session mode is not supported on native Windows") { + t.Errorf("expected platform-override warning, got %q", out) + } + if !strings.Contains(out, "using direct mode") { + t.Errorf("expected 'using direct mode' wording, got %q", out) + } + if !strings.Contains(out, "Use WSL") { + t.Errorf("expected 'Use WSL' recommendation, got %q", out) + } +} + +// TestPlatformOverrideWarningNotEmittedWhenDirect — direct mode (no +// override) emits no warning even on simulated native Windows. +func TestPlatformOverrideWarningNotEmittedWhenDirect(t *testing.T) { + clearResolverEnv(t) + writeSessionModeConfig(t, "direct") + config.SetIsNativeWindowsForTest(t, func() bool { return true }) + + out := captureStderr(t, func() { + emitPlatformOverrideWarningIfNeeded(false) + }) + if out != "" { + t.Errorf("expected no warning in direct mode, got %q", out) + } +} + +// TestPlatformOverrideWarningNotEmittedWithoutConfig — builtin +// default (direct) on any platform emits no warning. +func TestPlatformOverrideWarningNotEmittedWithoutConfig(t *testing.T) { + clearResolverEnv(t) + config.SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + config.SetIsNativeWindowsForTest(t, func() bool { return true }) + + out := captureStderr(t, func() { + emitPlatformOverrideWarningIfNeeded(false) + }) + if out != "" { + t.Errorf("expected no warning when session_mode is builtin direct, got %q", out) + } +} + +// TestPlatformOverrideWarningNotEmittedOnNonWindows — even with a +// persistent config, when isNativeWindows reports false the override +// never fires and no warning surfaces. +func TestPlatformOverrideWarningNotEmittedOnNonWindows(t *testing.T) { + clearResolverEnv(t) + writeSessionModeConfig(t, "persistent") + config.SetIsNativeWindowsForTest(t, func() bool { return false }) + + out := captureStderr(t, func() { + emitPlatformOverrideWarningIfNeeded(false) + }) + if out != "" { + t.Errorf("expected no warning when not native Windows, got %q", out) + } +} + +// TestPlatformOverrideWarningSkippedForAlwaysPersistent — `ctask +// attach` sets AlwaysPersistent=true and has no direct-mode +// fallback; the warning must not fire for that path. attach's +// actual refusal on native Windows continues through +// preflightPersistentEntry (TestPreflightRefusesNativeWindows). +func TestPlatformOverrideWarningSkippedForAlwaysPersistent(t *testing.T) { + clearResolverEnv(t) + writeSessionModeConfig(t, "persistent") + config.SetIsNativeWindowsForTest(t, func() bool { return true }) + + out := captureStderr(t, func() { + emitPlatformOverrideWarningIfNeeded(true) + }) + if out != "" { + t.Errorf("attach path (AlwaysPersistent) must not emit downgrade warning; got %q", out) + } +} diff --git a/cmd/restore_test.go b/cmd/restore_test.go index 9251920..6edb2be 100644 --- a/cmd/restore_test.go +++ b/cmd/restore_test.go @@ -34,7 +34,7 @@ func makeArchivedWs(t *testing.T, root, category, dirName string) string { Category: category, Type: "task", Mode: "local", - Agent: "claude", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(dir, "task.yaml"), meta) return dir @@ -149,7 +149,7 @@ func TestRestoreRefusesAlreadyActive(t *testing.T) { ID: "t", Slug: "already", Title: "already", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) diff --git a/cmd/resume.go b/cmd/resume.go index c1943b3..bb757b3 100644 --- a/cmd/resume.go +++ b/cmd/resume.go @@ -84,20 +84,20 @@ func doResume(query string, container, useShell, force bool, agentOverride strin return fmt.Errorf("updating metadata: %w", err) } - agent := agentOverride - if agent == "" { - agent = ws.Meta.Agent + resolved, err := resolveEntryAgent(ws.Meta.Agent, agentOverride) + if err != nil { + return err } return runWorkspaceEntry(WorkspaceEntryOptions{ - WsPath: ws.Path, - WsRoot: ws.Root, - WsMeta: ws.Meta, - Agent: agent, - Shell: useShell, - Force: force, - Direct: directFlag, - CommandName: "resume", + WsPath: ws.Path, + WsRoot: ws.Root, + WsMeta: ws.Meta, + ResolvedAgent: resolved, + Shell: useShell, + Force: force, + Direct: directFlag, + CommandName: "resume", }) } diff --git a/cmd/resume_archived_polish_test.go b/cmd/resume_archived_polish_test.go index 253bfb3..06fea13 100644 --- a/cmd/resume_archived_polish_test.go +++ b/cmd/resume_archived_polish_test.go @@ -43,7 +43,7 @@ func TestResumeArchivedHintNoDuplicateError(t *testing.T) { Category: "general", Type: "task", Mode: "local", - Agent: "claude", + Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) diff --git a/cmd/resume_persistent_test.go b/cmd/resume_persistent_test.go index c7afd97..ae57e84 100644 --- a/cmd/resume_persistent_test.go +++ b/cmd/resume_persistent_test.go @@ -28,7 +28,7 @@ func TestRunResumeForwardsToEntryHelperWithCommandNameResume(t *testing.T) { ID: "t", Slug: "resume-fwd-demo", Title: "resume-fwd-demo", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) diff --git a/cmd/resume_test.go b/cmd/resume_test.go index ecae37f..b619d57 100644 --- a/cmd/resume_test.go +++ b/cmd/resume_test.go @@ -60,7 +60,7 @@ func TestResumeArchivedWorkspaceShowsRestoreHint(t *testing.T) { Category: "general", Type: "task", Mode: "local", - Agent: "claude", + Agent: workspace.AgentSpec{Type: "claude"}, } workspace.WriteMeta(filepath.Join(wsDir, "task.yaml"), meta) diff --git a/cmd/root.go b/cmd/root.go index 6fec2a1..6b5c71b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,7 +6,7 @@ import ( "github.com/spf13/cobra" ) -var version = "0.5.4" +var version = "0.6.0" var rootCmd = &cobra.Command{ Use: "ctask", diff --git a/internal/agent/agent.go b/internal/agent/agent.go new file mode 100644 index 0000000..dc9e402 --- /dev/null +++ b/internal/agent/agent.go @@ -0,0 +1,81 @@ +package agent + +import ( + "fmt" + + "github.com/warrenronsiek/ctask/internal/workspace" +) + +// Profile describes a built-in agent. The Default command is what ctask +// invokes when AgentSpec.Command is empty; Type is the canonical name. +type Profile struct { + Type string + Default string +} + +// BuiltinProfiles enumerates the v0.6 built-in agents. "custom" is NOT +// in this map — it is the escape hatch with no defaults. Keep this in +// sync with workspace.knownAgentTypes AND workspace.IsBuiltinAgentType +// (update all three together when a new built-in lands). +var BuiltinProfiles = map[string]Profile{ + "claude": {Type: "claude", Default: "claude"}, + "opencode": {Type: "opencode", Default: "opencode"}, +} + +// IsKnownType reports whether t is "claude", "opencode", or "custom". +func IsKnownType(t string) bool { + if t == "custom" { + return true + } + _, ok := BuiltinProfiles[t] + return ok +} + +// Resolved is the launch-ready agent value: a single executable name or +// path, optional arguments, and an env-var map merged AFTER ctask's own +// exported vars at launch time. +type Resolved struct { + Type string + Command string + Args []string + Env map[string]string +} + +// Resolve combines a workspace's AgentSpec with the user-level +// default_agent into a launch-ready Resolved. Resolution rules (per +// v0.6 spec §5): +// +// 1. If spec.Type is empty, fall through to defaultAgent. +// 2. If the resolved type is "custom", spec.Command is required. +// 3. Otherwise, the resolved command is spec.Command if set, +// else BuiltinProfiles[type].Default. +// 4. Args and Env are carried verbatim from the spec. +// +// Resolve does NOT call exec.LookPath. PATH validation is the launch +// path's job (shell.ExecAgent fails fast with a diagnostic) and +// `ctask agents check`'s job. Keeping Resolve I/O-free makes it +// trivially testable and reusable. +func Resolve(spec workspace.AgentSpec, defaultAgent string) (*Resolved, error) { + typ := spec.Type + if typ == "" { + typ = defaultAgent + } + if !IsKnownType(typ) { + return nil, fmt.Errorf("agent: unknown type %q (must be claude, opencode, or custom)", typ) + } + + cmd := spec.Command + if cmd == "" { + if typ == "custom" { + return nil, fmt.Errorf("agent: type \"custom\" requires command field") + } + cmd = BuiltinProfiles[typ].Default + } + + return &Resolved{ + Type: typ, + Command: cmd, + Args: spec.Args, + Env: spec.Env, + }, nil +} diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go new file mode 100644 index 0000000..7f44dce --- /dev/null +++ b/internal/agent/agent_test.go @@ -0,0 +1,108 @@ +package agent + +import ( + "strings" + "testing" + + "github.com/warrenronsiek/ctask/internal/workspace" +) + +func TestResolveBuiltinClaude(t *testing.T) { + spec := workspace.AgentSpec{Type: "claude"} + got, err := Resolve(spec, "claude") + if err != nil { + t.Fatalf("err: %v", err) + } + if got.Type != "claude" || got.Command != "claude" { + t.Errorf("got %+v, want type=claude command=claude", got) + } +} + +func TestResolveBuiltinOpencode(t *testing.T) { + spec := workspace.AgentSpec{Type: "opencode"} + got, err := Resolve(spec, "claude") // default doesn't matter; spec wins + if err != nil { + t.Fatalf("err: %v", err) + } + if got.Command != "opencode" { + t.Errorf("Command = %q, want opencode", got.Command) + } +} + +func TestResolveCustomRequiresCommand(t *testing.T) { + spec := workspace.AgentSpec{Type: "custom"} + _, err := Resolve(spec, "claude") + if err == nil || !strings.Contains(err.Error(), "requires command") { + t.Fatalf("got %v, want error mentioning 'requires command'", err) + } +} + +func TestResolveCustomWithCommand(t *testing.T) { + spec := workspace.AgentSpec{Type: "custom", Command: "my-agent"} + got, err := Resolve(spec, "claude") + if err != nil { + t.Fatalf("err: %v", err) + } + if got.Command != "my-agent" { + t.Errorf("Command = %q, want my-agent", got.Command) + } +} + +func TestResolveFallbackToDefaultWhenTypeMissing(t *testing.T) { + spec := workspace.AgentSpec{} // legacy / new-without-type + got, err := Resolve(spec, "opencode") + if err != nil { + t.Fatalf("err: %v", err) + } + if got.Type != "opencode" || got.Command != "opencode" { + t.Errorf("got %+v, want type=opencode command=opencode", got) + } +} + +func TestResolveCommandOverride(t *testing.T) { + spec := workspace.AgentSpec{Type: "claude", Command: "/opt/claude"} + got, err := Resolve(spec, "claude") + if err != nil { + t.Fatalf("err: %v", err) + } + if got.Command != "/opt/claude" { + t.Errorf("Command = %q, want /opt/claude", got.Command) + } +} + +func TestResolveArgsAndEnvCarriedThrough(t *testing.T) { + spec := workspace.AgentSpec{ + Type: "opencode", + Args: []string{"--model", "x"}, + Env: map[string]string{"K": "v"}, + } + got, err := Resolve(spec, "claude") + if err != nil { + t.Fatalf("err: %v", err) + } + if len(got.Args) != 2 || got.Args[0] != "--model" || got.Args[1] != "x" { + t.Errorf("Args = %v, want [--model x]", got.Args) + } + if got.Env["K"] != "v" { + t.Errorf("Env[K] = %q, want v", got.Env["K"]) + } +} + +func TestResolveUnknownDefaultRejected(t *testing.T) { + spec := workspace.AgentSpec{} + _, err := Resolve(spec, "gemini") + if err == nil || !strings.Contains(err.Error(), "unknown") { + t.Fatalf("got %v, want unknown-type error", err) + } +} + +func TestIsKnownType(t *testing.T) { + for _, name := range []string{"claude", "opencode", "custom"} { + if !IsKnownType(name) { + t.Errorf("IsKnownType(%q) = false, want true", name) + } + } + if IsKnownType("gemini") { + t.Error("IsKnownType(gemini) = true, want false") + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 94b12af..6364c8a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,39 +9,34 @@ import ( ) // ResolveRoot returns the absolute workspace root path. -// Reads CTASK_ROOT env var, falls back to ~/ai-workspaces (or %USERPROFILE%\ai-workspaces on Windows). +// v0.6: resolution order is CLI flag > CTASK_ROOT env var > config file +// (ctask_root key) > built-in default (~/ai-workspaces or +// %USERPROFILE%\ai-workspaces). No CLI flag participates in Phase 1. func ResolveRoot() string { - root := os.Getenv("CTASK_ROOT") - if root == "" { - home, _ := os.UserHomeDir() - return filepath.Join(home, "ai-workspaces") - } - return expandPath(root) + return LoadResolver().CtaskRoot().Value } // ResolveAgent returns the agent command. -// Reads CTASK_AGENT env var, falls back to "claude". +// v0.6: resolution order is CLI flag > CTASK_AGENT env var > config file +// (default_agent key) > built-in default ("claude"). No CLI flag +// participates in Phase 1. func ResolveAgent() string { - agent := os.Getenv("CTASK_AGENT") - if agent == "" { - return "claude" - } - return agent + return LoadResolver().DefaultAgent().Value } // ResolveSeedDir returns the user general seed directory. -// Reads CTASK_SEED_DIR; falls back to %APPDATA%\ctask\seed on Windows or -// ~/.config/ctask/seed on Unix. +// v0.6: resolution order is CTASK_SEED_DIR env var > config file +// (seed_dir key) > built-in default (%APPDATA%\ctask\seed on Windows, +// ~/.config/ctask/seed on Unix). func ResolveSeedDir() string { - if v := os.Getenv("CTASK_SEED_DIR"); v != "" { - return expandPath(v) - } - return defaultSeedDir("seed") + return LoadResolver().SeedDir().Value } // ResolveProjectSeedDir returns the user project seed directory. // Reads CTASK_SEED_PROJECT_DIR; falls back to %APPDATA%\ctask\seed-project on Windows // or ~/.config/ctask/seed-project on Unix. +// v0.6 Phase 1 does not give this setting a config-file equivalent +// (spec lists only seed_dir, singular). func ResolveProjectSeedDir() string { if v := os.Getenv("CTASK_SEED_PROJECT_DIR"); v != "" { return expandPath(v) @@ -50,14 +45,20 @@ func ResolveProjectSeedDir() string { } // ResolveProjectRoot returns the project workspace root override. -// Returns empty string if CTASK_PROJECT_ROOT is not set; callers should fall back -// to ResolveRoot() in that case. +// Returns empty string when no user-supplied value is set (config file +// or env var). The "" sentinel is load-bearing for SearchRoots() and +// doctor's checkProjectRoot, both of which apply their own fallback +// (the v0.5 $CTASK_ROOT/projects/ default). +// +// v0.6: a project_root value supplied via the config file or +// CTASK_PROJECT_ROOT env var is returned verbatim; a missing/empty +// value still returns "". func ResolveProjectRoot() string { - v := os.Getenv("CTASK_PROJECT_ROOT") - if v == "" { + s := LoadResolver().ProjectRoot() + if s.Source == Builtin { return "" } - return expandPath(v) + return s.Value } // SearchRoots returns the deduplicated list of workspace roots that all query @@ -149,23 +150,32 @@ func defaultSeedDir(leaf string) string { return filepath.Join(home, ".config", "ctask", leaf) } -// ResolveSessionMode returns "direct" or "persistent" based on CTASK_SESSION_MODE. -// Default (unset/empty) is "direct". Any other value falls back to "direct" -// after printing a stderr warning. Used by entry commands (new, resume, last, -// open) to dispatch between the standard session.Run path and the tmux-backed -// persistent path. +// ResolveSessionMode returns "direct" or "persistent" — the effective +// launch session mode after resolving CLI flag > env var > config file +// > built-in default ("direct"), with the platform-override rule +// applied (persistent → direct on native Windows). Unknown values from +// the env var or config file are coerced to "direct" with a one-line +// stderr warning, preserving the v0.5.3 behavior. +// +// Used by entry commands (new, resume, last, open) to dispatch between +// the standard session.Run path and the tmux-backed persistent path. func ResolveSessionMode() string { - v := os.Getenv("CTASK_SESSION_MODE") - switch v { - case "", "direct": - return "direct" - case "persistent": - return "persistent" - default: - fmt.Fprintf(os.Stderr, - "[ctask] warning: CTASK_SESSION_MODE=%q is not recognized; using direct mode\n", v) - return "direct" + r := LoadResolver() + s := r.SessionMode() + if s.Value == "direct" || s.Value == "persistent" { + return s.Value } + // Unknown value path — preserve the v0.5.3 stderr warning. Source + // of the bad value is reported so the user can find where to fix it. + switch s.Source { + case EnvVar: + fmt.Fprintf(os.Stderr, + "[ctask] warning: CTASK_SESSION_MODE=%q is not recognized; using direct mode\n", s.Value) + case ConfigFileSrc: + fmt.Fprintf(os.Stderr, + "[ctask] warning: config session_mode=%q is not recognized; using direct mode\n", s.Value) + } + return "direct" } // expandPath expands a leading ~/ and resolves to an absolute path when possible. diff --git a/internal/config/config_roots_test.go b/internal/config/config_roots_test.go index a4f3faf..71cd53a 100644 --- a/internal/config/config_roots_test.go +++ b/internal/config/config_roots_test.go @@ -9,6 +9,7 @@ import ( func TestSearchRootsUnsetProjectRoot(t *testing.T) { // v0.5: when CTASK_PROJECT_ROOT is unset, SearchRoots also appends // $CTASK_ROOT/projects/ so default-location projects are findable. + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_PROJECT_ROOT") os.Setenv("CTASK_ROOT", t.TempDir()) defer os.Unsetenv("CTASK_ROOT") @@ -55,6 +56,7 @@ func TestSearchRootsSameRootDedupes(t *testing.T) { } func TestSearchRootsAppendsProjectsDirWhenUnset(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) taskRoot := t.TempDir() os.Setenv("CTASK_ROOT", taskRoot) os.Unsetenv("CTASK_PROJECT_ROOT") diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c88e66f..46cfd90 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -10,6 +10,7 @@ import ( ) func TestDefaultRoot(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_ROOT") root := ResolveRoot() home, _ := os.UserHomeDir() @@ -39,6 +40,7 @@ func TestRootResolvesRelative(t *testing.T) { } func TestDefaultAgent(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_AGENT") agent := ResolveAgent() if agent != "claude" { @@ -71,6 +73,7 @@ func TestRootResolvesTilde(t *testing.T) { } func TestResolveSeedDirDefault(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_SEED_DIR") got := ResolveSeedDir() home, _ := os.UserHomeDir() @@ -129,6 +132,7 @@ func TestResolveProjectSeedDirOverride(t *testing.T) { } func TestResolveProjectRootEmptyWhenUnset(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_PROJECT_ROOT") got := ResolveProjectRoot() if got != "" { @@ -195,6 +199,7 @@ func TestEnvVarsLaunchDirEmpty(t *testing.T) { } func TestResolveSessionModeDefault(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Unsetenv("CTASK_SESSION_MODE") if got := ResolveSessionMode(); got != "direct" { t.Errorf("default: got %q, want %q", got, "direct") @@ -202,6 +207,7 @@ func TestResolveSessionModeDefault(t *testing.T) { } func TestResolveSessionModeEmpty(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) os.Setenv("CTASK_SESSION_MODE", "") defer os.Unsetenv("CTASK_SESSION_MODE") if got := ResolveSessionMode(); got != "direct" { @@ -218,6 +224,11 @@ func TestResolveSessionModeDirect(t *testing.T) { } func TestResolveSessionModePersistent(t *testing.T) { + // Disable the v0.6 platform override so this test can verify the + // pure layering behavior (env var → persistent). The platform + // override is covered separately by + // TestResolverSessionModePlatformOverride. + SetIsNativeWindowsForTest(t, func() bool { return false }) os.Setenv("CTASK_SESSION_MODE", "persistent") defer os.Unsetenv("CTASK_SESSION_MODE") if got := ResolveSessionMode(); got != "persistent" { diff --git a/internal/config/configfile.go b/internal/config/configfile.go new file mode 100644 index 0000000..3fa7949 --- /dev/null +++ b/internal/config/configfile.go @@ -0,0 +1,119 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + + "gopkg.in/yaml.v3" +) + +// CurrentConfigSchemaVersion is the highest config-file schema version +// this binary understands. A future schema version bump must add the +// matching parser support alongside this constant. +const CurrentConfigSchemaVersion = 1 + +// ConfigFile is the strict YAML schema for the global ctask config file. +// All fields are optional; missing fields fall back to built-in defaults +// via the Resolver. Unknown keys invalidate the entire file (no partial +// apply) per the v0.6 spec. +type ConfigFile struct { + SchemaVersion int `yaml:"schema_version"` + CtaskRoot string `yaml:"ctask_root"` + ProjectRoot string `yaml:"project_root"` + SeedDir string `yaml:"seed_dir"` + DefaultAgent string `yaml:"default_agent"` + DefaultCategory string `yaml:"default_category"` + Editor string `yaml:"editor"` + SessionMode string `yaml:"session_mode"` +} + +// configFileKnownKeys returns the set of YAML keys recognized by the +// ConfigFile struct. Kept in lockstep with the yaml tags above. +func configFileKnownKeys() map[string]struct{} { + return map[string]struct{}{ + "schema_version": {}, + "ctask_root": {}, + "project_root": {}, + "seed_dir": {}, + "default_agent": {}, + "default_category": {}, + "editor": {}, + "session_mode": {}, + } +} + +// LoadConfigFile reads and parses the config file at path. Returns: +// - (nil, nil) the file does not exist (caller falls back to defaults) +// - (nil, err) any I/O, parse, unknown-key, or schema-version error +// - (cfg, nil) on success +// +// Unknown top-level keys invalidate the entire file with a descriptive +// error; ctask never partially applies a file with typos. Future schema +// versions are rejected with an upgrade message. +func LoadConfigFile(path string) (*ConfigFile, error) { + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + // First pass: parse into a generic map of YAML nodes so we can + // inspect raw key names without trusting the typed-struct unmarshal + // (yaml.v3 silently drops unknown keys by default). + var raw map[string]yaml.Node + if err := yaml.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("parsing config file: %w", err) + } + + known := configFileKnownKeys() + for key := range raw { + if _, ok := known[key]; !ok { + return nil, fmt.Errorf("unknown key: %q", key) + } + } + + // Second pass: typed unmarshal. + var cfg ConfigFile + if err := yaml.Unmarshal(data, &cfg); err != nil { + return nil, fmt.Errorf("parsing config file: %w", err) + } + + if cfg.SchemaVersion > CurrentConfigSchemaVersion { + return nil, fmt.Errorf( + "config file requires schema version %d, but this binary supports up to version %d. Please upgrade ctask.", + cfg.SchemaVersion, CurrentConfigSchemaVersion) + } + + return &cfg, nil +} + +// ConfigFilePath returns the platform-appropriate global config path. +// +// Linux / macOS / WSL: +// - $XDG_CONFIG_HOME/ctask/config.yaml when XDG_CONFIG_HOME is set +// - ~/.config/ctask/config.yaml otherwise +// +// Native Windows: +// - %APPDATA%\ctask\config.yaml (fallback: %USERPROFILE%\AppData\Roaming\ctask\config.yaml) +// +// The path is returned regardless of whether the file exists; callers +// stat the path themselves. +func ConfigFilePath() string { + if runtime.GOOS == "windows" { + appData := os.Getenv("APPDATA") + if appData == "" { + home, _ := os.UserHomeDir() + appData = filepath.Join(home, "AppData", "Roaming") + } + return filepath.Join(appData, "ctask", "config.yaml") + } + if xdg := os.Getenv("XDG_CONFIG_HOME"); xdg != "" { + return filepath.Join(xdg, "ctask", "config.yaml") + } + home, _ := os.UserHomeDir() + return filepath.Join(home, ".config", "ctask", "config.yaml") +} diff --git a/internal/config/configfile_test.go b/internal/config/configfile_test.go new file mode 100644 index 0000000..0692003 --- /dev/null +++ b/internal/config/configfile_test.go @@ -0,0 +1,193 @@ +package config + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// TestLoadConfigFileMissingReturnsNil — a missing config file is not an +// error; callers fall back to env vars / built-in defaults. The +// resolver tracks "no file" as INFO state. +func TestLoadConfigFileMissingReturnsNil(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "nonexistent.yaml") + + cfg, err := LoadConfigFile(path) + if err != nil { + t.Fatalf("missing file should not error, got %v", err) + } + if cfg != nil { + t.Errorf("missing file should return nil cfg, got %+v", cfg) + } +} + +// TestLoadConfigFileBasic — all known keys parse into the typed struct. +func TestLoadConfigFileBasic(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + body := []byte(`schema_version: 1 +ctask_root: ~/ai-workspaces +project_root: ~/ai-workspaces/projects +seed_dir: ~/.config/ctask/seed +default_agent: opencode +default_category: tasks +editor: code +session_mode: persistent +`) + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + cfg, err := LoadConfigFile(path) + if err != nil { + t.Fatalf("LoadConfigFile: %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil cfg") + } + if cfg.SchemaVersion != 1 { + t.Errorf("SchemaVersion: got %d, want 1", cfg.SchemaVersion) + } + if cfg.DefaultAgent != "opencode" { + t.Errorf("DefaultAgent: got %q, want opencode", cfg.DefaultAgent) + } + if cfg.SessionMode != "persistent" { + t.Errorf("SessionMode: got %q, want persistent", cfg.SessionMode) + } +} + +// TestLoadConfigFilePartial — only some keys set; rest remain zero-valued. +func TestLoadConfigFilePartial(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + body := []byte("default_agent: opencode\n") + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + cfg, err := LoadConfigFile(path) + if err != nil { + t.Fatalf("LoadConfigFile: %v", err) + } + if cfg.DefaultAgent != "opencode" { + t.Errorf("DefaultAgent: got %q, want opencode", cfg.DefaultAgent) + } + if cfg.CtaskRoot != "" { + t.Errorf("CtaskRoot: expected empty (unset), got %q", cfg.CtaskRoot) + } + if cfg.SessionMode != "" { + t.Errorf("SessionMode: expected empty (unset), got %q", cfg.SessionMode) + } +} + +// TestLoadConfigFileUnknownKey — strict-key rejection per spec. +// Entire file is invalidated; the error names the offending key. +func TestLoadConfigFileUnknownKey(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + body := []byte("default_agnet: foo\n") // typo + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + cfg, err := LoadConfigFile(path) + if err == nil { + t.Fatalf("expected error for unknown key, got cfg=%+v", cfg) + } + if cfg != nil { + t.Errorf("expected nil cfg on error, got %+v", cfg) + } + if !strings.Contains(err.Error(), "default_agnet") { + t.Errorf("error should name unknown key, got: %v", err) + } +} + +// TestLoadConfigFileSchemaVersionFuture — a config requesting a higher +// schema version than this binary supports is rejected with an upgrade +// message. No partial parsing. +func TestLoadConfigFileSchemaVersionFuture(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + body := []byte("schema_version: 2\ndefault_agent: claude\n") + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + cfg, err := LoadConfigFile(path) + if err == nil { + t.Fatalf("expected error for future schema version, got cfg=%+v", cfg) + } + if cfg != nil { + t.Errorf("expected nil cfg on schema-version error, got %+v", cfg) + } + if !strings.Contains(err.Error(), "schema version") { + t.Errorf("error should mention schema version, got: %v", err) + } + if !strings.Contains(err.Error(), "upgrade") { + t.Errorf("error should mention upgrade, got: %v", err) + } +} + +// TestLoadConfigFileMalformedYAML — invalid YAML produces an error. +func TestLoadConfigFileMalformedYAML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + body := []byte("default_agent: [not closed\n") + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + cfg, err := LoadConfigFile(path) + if err == nil { + t.Errorf("expected error for malformed YAML, got cfg=%+v", cfg) + } + if cfg != nil { + t.Errorf("expected nil cfg on YAML error, got %+v", cfg) + } +} + +// TestConfigFilePathLinux — Unix paths honor XDG_CONFIG_HOME. +func TestConfigFilePathLinux(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix-path semantics") + } + t.Setenv("XDG_CONFIG_HOME", "/tmp/xdgtest") + got := ConfigFilePath() + want := filepath.Join("/tmp/xdgtest", "ctask", "config.yaml") + if got != want { + t.Errorf("ConfigFilePath with XDG: got %q, want %q", got, want) + } +} + +// TestConfigFilePathLinuxDefault — fallback to ~/.config when XDG unset. +func TestConfigFilePathLinuxDefault(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix-path semantics") + } + t.Setenv("XDG_CONFIG_HOME", "") + got := ConfigFilePath() + home, _ := os.UserHomeDir() + want := filepath.Join(home, ".config", "ctask", "config.yaml") + if got != want { + t.Errorf("ConfigFilePath default: got %q, want %q", got, want) + } +} + +// TestConfigFilePathWindows — %APPDATA% path on native Windows. +func TestConfigFilePathWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Windows-only") + } + appData := os.Getenv("APPDATA") + if appData == "" { + t.Skip("APPDATA unset on this host") + } + got := ConfigFilePath() + want := filepath.Join(appData, "ctask", "config.yaml") + if got != want { + t.Errorf("ConfigFilePath on Windows: got %q, want %q", got, want) + } +} diff --git a/internal/config/resolver.go b/internal/config/resolver.go new file mode 100644 index 0000000..cea726b --- /dev/null +++ b/internal/config/resolver.go @@ -0,0 +1,320 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "runtime" +) + +// SettingSource records where a resolved setting's value came from. +// Defined high-to-low priority order is implicit in the resolver: +// CLIFlag > EnvVar > ConfigFileSrc > Builtin. PlatformOverride is a +// special-case post-resolution adjustment (currently used only for +// session_mode on native Windows). +type SettingSource int + +const ( + // Builtin: ctask's compiled-in default. + Builtin SettingSource = iota + // ConfigFileSrc: value came from the global config.yaml. + // Named with the "Src" suffix to avoid colliding with the + // ConfigFile type in this package. + ConfigFileSrc + // EnvVar: value came from an environment variable. + EnvVar + // CLIFlag: value came from a CLI flag. Reserved for v0.6 Phase 2 + // (--agent) and later flags; no Phase 1 setting reaches this source. + CLIFlag + // PlatformOverride: ctask itself overrode the user-configured value + // because the requested setting is unsupported on the current + // platform (currently: persistent session_mode on native Windows). + PlatformOverride +) + +// String returns the human-readable label used in doctor/info output. +func (s SettingSource) String() string { + switch s { + case Builtin: + return "built-in default" + case ConfigFileSrc: + return "config file" + case EnvVar: + return "env var" + case CLIFlag: + return "CLI flag" + case PlatformOverride: + return "platform override" + default: + return fmt.Sprintf("unknown source (%d)", int(s)) + } +} + +// ResolvedSetting carries a single setting's effective value plus +// provenance. Override (if non-nil) chains to the next-lower-priority +// source that contributed a value but was overridden — letting +// doctor/info render lines like "source: CTASK_AGENT env var +// (overrides config file: claude)". +type ResolvedSetting struct { + Key string + Value string + Source SettingSource + EnvName string // for EnvVar source, the env-var name + Override *ResolvedSetting +} + +// Resolver carries the loaded config file plus a snapshot of relevant +// env vars so callers can compute each setting's effective value and +// its source. Build one with LoadResolver() per command invocation; +// reuse across calls within the same invocation to keep doctor/info +// output internally consistent. +type Resolver struct { + ConfigPath string // path consulted (regardless of whether file exists) + ConfigErr error // non-nil when config file failed to load + cfg *ConfigFile + + envCtaskRoot string + envProjectRoot string + envSeedDir string + envAgent string + envSessionMode string + envEditor string + + // cliFlagAgent, when non-empty, is the value of the --agent CLI flag. + // Set by SetCLIFlagAgent after Cobra parses flags; consulted by + // DefaultAgent as the highest-priority layer (SettingSource.CLIFlag). + cliFlagAgent string +} + +// SetCLIFlagAgent records the value of the --agent CLI flag for +// resolution. Cmd-layer code calls this AFTER Cobra parses flags, so the +// flag value can participate in source attribution. Empty string means +// the flag was not passed; the env-var layer is consulted instead. +// +// Currently used only by `ctask new` (where --agent is a type selector, +// per v0.6 spec §5 + Open Q 1). Other commands' --agent flags act as +// one-shot agent.command overrides on the AgentSpec, NOT as resolver +// inputs — they do not call this setter. +func (r *Resolver) SetCLIFlagAgent(value string) { + r.cliFlagAgent = value +} + +// configPathForTest, when non-empty, overrides ConfigFilePath() for +// the duration of a test. The legacy production code path consults +// ConfigFilePath() directly. +var configPathForTest string + +// SetConfigPathForTest points the resolver at path for the duration +// of t. Use this in any test that depends on Builtin-source resolution +// (so a real ~/.config/ctask/config.yaml on the developer host cannot +// pollute the assertion). Pass a path under t.TempDir() to guarantee +// the file does not exist. +func SetConfigPathForTest(t interface{ Cleanup(func()) }, path string) { + prev := configPathForTest + configPathForTest = path + t.Cleanup(func() { configPathForTest = prev }) +} + +// isNativeWindowsForTest lets tests simulate the native-Windows-no-WSL +// platform regardless of the actual GOOS. Production callers use the +// real check via isNativeWindows(). +var isNativeWindowsForTest func() bool + +// SetIsNativeWindowsForTest replaces the native-Windows detector for +// the duration of t. Use it from tests outside the config package +// that need to exercise the session_mode platform-override path +// (or, conversely, disable the override on a Windows host so a test +// can assert pre-override layering). Restored via t.Cleanup. +func SetIsNativeWindowsForTest(t interface{ Cleanup(func()) }, f func() bool) { + prev := isNativeWindowsForTest + isNativeWindowsForTest = f + t.Cleanup(func() { isNativeWindowsForTest = prev }) +} + +// isNativeWindows reports whether ctask is running on Windows outside +// of a WSL distro. WSL exports WSL_DISTRO_NAME for any process started +// from a WSL shell. +func isNativeWindows() bool { + if isNativeWindowsForTest != nil { + return isNativeWindowsForTest() + } + return runtime.GOOS == "windows" && os.Getenv("WSL_DISTRO_NAME") == "" +} + +// resolverPath returns the config-file path the resolver should read, +// honoring the test override when present. +func resolverPath() string { + if configPathForTest != "" { + return configPathForTest + } + return ConfigFilePath() +} + +// LoadResolver reads the config file (if present) and snapshots the +// environment variables the resolver needs. Always returns a non-nil +// Resolver — when the config file is missing or invalid, the resolver +// still serves builtin and env-derived values. +func LoadResolver() *Resolver { + r := &Resolver{ConfigPath: resolverPath()} + cfg, err := LoadConfigFile(r.ConfigPath) + if err != nil { + r.ConfigErr = err + } else if cfg != nil { + r.cfg = cfg + } + r.envCtaskRoot = os.Getenv("CTASK_ROOT") + r.envProjectRoot = os.Getenv("CTASK_PROJECT_ROOT") + r.envSeedDir = os.Getenv("CTASK_SEED_DIR") + r.envAgent = os.Getenv("CTASK_AGENT") + r.envSessionMode = os.Getenv("CTASK_SESSION_MODE") + r.envEditor = os.Getenv("EDITOR") + return r +} + +// stringSetting layers Builtin → ConfigFileSrc → EnvVar for a single +// string-valued key. Empty values at any layer are treated as "not +// supplied" and skipped. transform, when non-nil, is applied to every +// non-empty layer's value (used for path expansion). +func (r *Resolver) stringSetting(key, envName, builtin, cfgVal, envVal string, transform func(string) string) ResolvedSetting { + apply := func(v string) string { + if transform == nil { + return v + } + return transform(v) + } + + s := ResolvedSetting{Key: key, Value: apply(builtin), Source: Builtin} + if cfgVal != "" { + prev := s + s = ResolvedSetting{ + Key: key, + Value: apply(cfgVal), + Source: ConfigFileSrc, + Override: &prev, + } + } + if envVal != "" { + prev := s + s = ResolvedSetting{ + Key: key, + Value: apply(envVal), + Source: EnvVar, + EnvName: envName, + Override: &prev, + } + } + return s +} + +// CtaskRoot resolves the workspace root path with source attribution. +func (r *Resolver) CtaskRoot() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.CtaskRoot + } + return r.stringSetting("ctask_root", "CTASK_ROOT", + defaultCtaskRoot(), cfgVal, r.envCtaskRoot, expandPath) +} + +// ProjectRoot resolves the project-workspace root. Its built-in +// default is derived from the resolved ctask_root, mirroring the +// SearchRoots() v0.5 fallback (/projects). The Builtin +// chain therefore contains the derived path, not an empty string. +func (r *Resolver) ProjectRoot() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.ProjectRoot + } + builtin := filepath.Join(r.CtaskRoot().Value, "projects") + return r.stringSetting("project_root", "CTASK_PROJECT_ROOT", + builtin, cfgVal, r.envProjectRoot, expandPath) +} + +// SeedDir resolves the user general seed directory. Built-in default +// is the platform-default seed location. +func (r *Resolver) SeedDir() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.SeedDir + } + return r.stringSetting("seed_dir", "CTASK_SEED_DIR", + defaultSeedDir("seed"), cfgVal, r.envSeedDir, expandPath) +} + +// DefaultAgent resolves the default agent command. Layering is +// Builtin → ConfigFile → EnvVar → CLIFlag: when SetCLIFlagAgent has +// recorded a non-empty --agent value, it wins and the previously +// resolved setting is chained as Override so doctor/info can render the +// full precedence path. +func (r *Resolver) DefaultAgent() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.DefaultAgent + } + base := r.stringSetting("default_agent", "CTASK_AGENT", + "claude", cfgVal, r.envAgent, nil) + if r.cliFlagAgent != "" { + prev := base + return ResolvedSetting{ + Key: "default_agent", + Value: r.cliFlagAgent, + Source: CLIFlag, + Override: &prev, + } + } + return base +} + +// DefaultCategory resolves the default category for new workspaces. +// No env-var equivalent in v0.6 Phase 1. +func (r *Resolver) DefaultCategory() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.DefaultCategory + } + return r.stringSetting("default_category", "", + "tasks", cfgVal, "", nil) +} + +// Editor resolves the user's text editor (used by future ctask edit). +// Env var is the conventional EDITOR. +func (r *Resolver) Editor() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.Editor + } + return r.stringSetting("editor", "EDITOR", + "", cfgVal, r.envEditor, nil) +} + +// SessionMode resolves the launch session mode. Layers Builtin (direct) +// → ConfigFile → EnvVar, then applies the native-Windows platform +// override: when the resolved value is "persistent" but the host is +// native Windows, the value is forced to "direct" with PlatformOverride +// as the source and the previously resolved setting chained as +// Override (so doctor can render "configured: persistent"). +func (r *Resolver) SessionMode() ResolvedSetting { + cfgVal := "" + if r.cfg != nil { + cfgVal = r.cfg.SessionMode + } + s := r.stringSetting("session_mode", "CTASK_SESSION_MODE", + "direct", cfgVal, r.envSessionMode, nil) + + if s.Value == "persistent" && isNativeWindows() { + prev := s + s = ResolvedSetting{ + Key: "session_mode", + Value: "direct", + Source: PlatformOverride, + Override: &prev, + } + } + return s +} + +// defaultCtaskRoot returns the built-in default for the workspace root. +func defaultCtaskRoot() string { + home, _ := os.UserHomeDir() + return filepath.Join(home, "ai-workspaces") +} diff --git a/internal/config/resolver_test.go b/internal/config/resolver_test.go new file mode 100644 index 0000000..fb04640 --- /dev/null +++ b/internal/config/resolver_test.go @@ -0,0 +1,329 @@ +package config + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// writeConfig is a test helper that writes body to a temp directory's +// config.yaml, points ctaskConfigPathOverrideForTest at it, and returns +// the path. Test cleanup restores the override. +func writeConfig(t *testing.T, body string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(path, []byte(body), 0644); err != nil { + t.Fatalf("writeConfig: %v", err) + } + prev := configPathForTest + configPathForTest = path + t.Cleanup(func() { configPathForTest = prev }) + return path +} + +// noConfig points the resolver at a nonexistent path for the duration of +// a single test. Used to assert "no config file" behavior even when the +// host happens to have a real ~/.config/ctask/config.yaml. +func noConfig(t *testing.T) { + t.Helper() + dir := t.TempDir() + prev := configPathForTest + configPathForTest = filepath.Join(dir, "nonexistent.yaml") + t.Cleanup(func() { configPathForTest = prev }) +} + +// clearConfigEnv unsets every env var the resolver reads. Use this in +// every resolver test to insulate from inherited shell state. t.Setenv +// is preferred when a single value is needed (it auto-restores). +func clearConfigEnv(t *testing.T) { + t.Helper() + for _, name := range []string{ + "CTASK_ROOT", "CTASK_PROJECT_ROOT", "CTASK_SEED_DIR", + "CTASK_AGENT", "CTASK_SESSION_MODE", "EDITOR", + } { + t.Setenv(name, "") + os.Unsetenv(name) + } +} + +// TestResolverNoConfigUsesBuiltins — when no config file exists and no +// env vars are set, every setting resolves to Builtin source. +func TestResolverNoConfigUsesBuiltins(t *testing.T) { + clearConfigEnv(t) + noConfig(t) + r := LoadResolver() + if r.ConfigErr != nil { + t.Errorf("missing file should not be an error, got %v", r.ConfigErr) + } + + if s := r.CtaskRoot(); s.Source != Builtin { + t.Errorf("CtaskRoot source: got %v, want Builtin", s.Source) + } + if s := r.DefaultAgent(); s.Source != Builtin || s.Value != "claude" { + t.Errorf("DefaultAgent: got %+v, want value=claude source=Builtin", s) + } + if s := r.SessionMode(); s.Source != Builtin || s.Value != "direct" { + t.Errorf("SessionMode: got %+v, want value=direct source=Builtin", s) + } + if s := r.DefaultCategory(); s.Source != Builtin || s.Value != "tasks" { + t.Errorf("DefaultCategory: got %+v, want value=tasks source=Builtin", s) + } +} + +// TestResolverConfigFilePartial — only default_agent set in config; that +// setting resolves to ConfigFileSrc, others stay Builtin. +func TestResolverConfigFilePartial(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "default_agent: opencode\n") + + r := LoadResolver() + if r.ConfigErr != nil { + t.Fatalf("ConfigErr: %v", r.ConfigErr) + } + + s := r.DefaultAgent() + if s.Source != ConfigFileSrc { + t.Errorf("DefaultAgent source: got %v, want ConfigFileSrc", s.Source) + } + if s.Value != "opencode" { + t.Errorf("DefaultAgent value: got %q, want opencode", s.Value) + } + if s.Override == nil || s.Override.Source != Builtin { + t.Errorf("DefaultAgent Override should chain to Builtin, got %+v", s.Override) + } + + if s := r.CtaskRoot(); s.Source != Builtin { + t.Errorf("CtaskRoot should remain Builtin, got %v", s.Source) + } +} + +// TestResolverEnvOverridesConfig — env var beats config file; the +// override chain points back at the config-file setting. +func TestResolverEnvOverridesConfig(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "default_agent: opencode\n") + t.Setenv("CTASK_AGENT", "aider") + + r := LoadResolver() + s := r.DefaultAgent() + if s.Source != EnvVar { + t.Errorf("source: got %v, want EnvVar", s.Source) + } + if s.Value != "aider" { + t.Errorf("value: got %q, want aider", s.Value) + } + if s.EnvName != "CTASK_AGENT" { + t.Errorf("EnvName: got %q, want CTASK_AGENT", s.EnvName) + } + if s.Override == nil || s.Override.Source != ConfigFileSrc { + t.Fatalf("override should chain to ConfigFileSrc, got %+v", s.Override) + } + if s.Override.Value != "opencode" { + t.Errorf("override value: got %q, want opencode", s.Override.Value) + } +} + +// TestResolverConfigFileUnknownKeyInvalidatesFile — strict-key error +// recorded on ConfigErr; settings fall back to env/builtin only. +func TestResolverConfigFileUnknownKeyInvalidatesFile(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "default_agnet: foo\n") + + r := LoadResolver() + if r.ConfigErr == nil { + t.Fatalf("expected ConfigErr from unknown key, got nil") + } + if !strings.Contains(r.ConfigErr.Error(), "default_agnet") { + t.Errorf("ConfigErr should name unknown key, got %v", r.ConfigErr) + } + + if s := r.DefaultAgent(); s.Source != Builtin { + t.Errorf("DefaultAgent should fall back to Builtin when config invalid, got %v", s.Source) + } +} + +// TestResolverConfigSchemaVersionFutureRejected — same as configfile +// test, but viewed through the resolver. +func TestResolverConfigSchemaVersionFutureRejected(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "schema_version: 2\ndefault_agent: opencode\n") + + r := LoadResolver() + if r.ConfigErr == nil { + t.Fatalf("expected ConfigErr from future schema version") + } + if !strings.Contains(r.ConfigErr.Error(), "schema version") { + t.Errorf("ConfigErr should mention schema version, got %v", r.ConfigErr) + } + + if s := r.DefaultAgent(); s.Source != Builtin { + t.Errorf("DefaultAgent should fall back to Builtin, got %v", s.Source) + } +} + +// TestResolverPathExpansion — ~/ in config values is expanded. +func TestResolverPathExpansion(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("tilde expansion not typical on Windows") + } + clearConfigEnv(t) + writeConfig(t, "ctask_root: ~/ai-workspaces\n") + + r := LoadResolver() + s := r.CtaskRoot() + if !filepath.IsAbs(s.Value) { + t.Errorf("CtaskRoot should be absolute after expansion, got %q", s.Value) + } + if strings.HasPrefix(s.Value, "~") { + t.Errorf("CtaskRoot tilde not expanded: %q", s.Value) + } +} + +// TestResolverSessionModePlatformOverride — config requests persistent +// mode, but the simulated host is native Windows. Resolver should +// return Value="direct" with Source=PlatformOverride and Override +// chained back to the ConfigFile setting. +func TestResolverSessionModePlatformOverride(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "session_mode: persistent\n") + + prev := isNativeWindowsForTest + isNativeWindowsForTest = func() bool { return true } + t.Cleanup(func() { isNativeWindowsForTest = prev }) + + r := LoadResolver() + s := r.SessionMode() + if s.Value != "direct" { + t.Errorf("value: got %q, want direct (platform override)", s.Value) + } + if s.Source != PlatformOverride { + t.Errorf("source: got %v, want PlatformOverride", s.Source) + } + if s.Override == nil { + t.Fatalf("override should chain to configured value, got nil") + } + if s.Override.Value != "persistent" { + t.Errorf("override value: got %q, want persistent", s.Override.Value) + } + if s.Override.Source != ConfigFileSrc { + t.Errorf("override source: got %v, want ConfigFileSrc", s.Override.Source) + } +} + +// TestResolverSessionModeNoOverrideWhenNotPersistent — even on native +// Windows, direct mode doesn't trip the override. +func TestResolverSessionModeNoOverrideWhenNotPersistent(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "session_mode: direct\n") + + prev := isNativeWindowsForTest + isNativeWindowsForTest = func() bool { return true } + t.Cleanup(func() { isNativeWindowsForTest = prev }) + + r := LoadResolver() + s := r.SessionMode() + if s.Value != "direct" { + t.Errorf("value: got %q, want direct", s.Value) + } + if s.Source != ConfigFileSrc { + t.Errorf("source: got %v, want ConfigFileSrc (no override needed)", s.Source) + } +} + +// TestSettingSourceString — human-readable strings for doctor/info. +func TestSettingSourceString(t *testing.T) { + cases := []struct { + s SettingSource + want string + }{ + {Builtin, "built-in default"}, + {ConfigFileSrc, "config file"}, + {EnvVar, "env var"}, + {CLIFlag, "CLI flag"}, + {PlatformOverride, "platform override"}, + } + for _, c := range cases { + if got := c.s.String(); got != c.want { + t.Errorf("%v.String(): got %q, want %q", c.s, got, c.want) + } + } +} + +// TestResolverProjectRootDefaultsToCtaskRootProjects — built-in default +// for project_root is /projects, mirroring the existing +// SearchRoots() fallback behavior introduced in v0.5. +func TestResolverProjectRootDefaultsToCtaskRootProjects(t *testing.T) { + clearConfigEnv(t) + noConfig(t) + tmp := t.TempDir() + t.Setenv("CTASK_ROOT", tmp) + + r := LoadResolver() + want := filepath.Join(tmp, "projects") + s := r.ProjectRoot() + if filepath.Clean(s.Value) != filepath.Clean(want) { + t.Errorf("ProjectRoot value: got %q, want %q", s.Value, want) + } + if s.Source != Builtin { + t.Errorf("ProjectRoot source: got %v, want Builtin (derived from CtaskRoot)", s.Source) + } +} + +// TestResolverEditorFromConfigAndEnv — Editor honors EDITOR env var when +// set, then config, then empty default. +func TestResolverEditorFromConfigAndEnv(t *testing.T) { + clearConfigEnv(t) + writeConfig(t, "editor: vim\n") + t.Setenv("EDITOR", "nano") + + r := LoadResolver() + s := r.Editor() + if s.Value != "nano" || s.Source != EnvVar { + t.Errorf("Editor with both env and config: got %+v, want value=nano source=EnvVar", s) + } + if s.Override == nil || s.Override.Value != "vim" { + t.Errorf("Editor override should chain to config (vim), got %+v", s.Override) + } +} + +func TestCLIFlagOverridesEnvVar(t *testing.T) { + // Phase 1 deferred this — Phase 2 activates SettingSource.CLIFlag for + // --agent. The test sets CTASK_AGENT in the env, then applies a CLI + // flag override and asserts the resolved value, source, and override + // chain. + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + t.Setenv("CTASK_AGENT", "opencode") + + r := LoadResolver() + r.SetCLIFlagAgent("claude") + + s := r.DefaultAgent() + if s.Value != "claude" { + t.Errorf("Value = %q, want claude", s.Value) + } + if s.Source != CLIFlag { + t.Errorf("Source = %v, want CLIFlag", s.Source) + } + if s.Override == nil { + t.Fatal("Override = nil, want non-nil pointing at the EnvVar layer") + } + if s.Override.Source != EnvVar || s.Override.Value != "opencode" { + t.Errorf("Override = {%v %q}, want {EnvVar opencode}", s.Override.Source, s.Override.Value) + } +} + +func TestCLIFlagDoesNotApplyWhenUnset(t *testing.T) { + SetConfigPathForTest(t, filepath.Join(t.TempDir(), "no-config.yaml")) + t.Setenv("CTASK_AGENT", "opencode") + + r := LoadResolver() + // No SetCLIFlagAgent call. + + s := r.DefaultAgent() + if s.Source != EnvVar || s.Value != "opencode" { + t.Errorf("got {%v %q}, want {EnvVar opencode}", s.Source, s.Value) + } +} diff --git a/internal/seed/agents.go b/internal/seed/agents.go new file mode 100644 index 0000000..a85ca41 --- /dev/null +++ b/internal/seed/agents.go @@ -0,0 +1,130 @@ +package seed + +// AgentsMD returns the canonical AGENTS.md content for a new workspace. +// This is the single source of truth for agent-facing instructions in +// v0.6+. Agent-specific files (CLAUDE.md, etc.) become thin shims +// pointing back here. Per v0.6 spec §6. +// +// isProject controls a small set of project-specific additions +// (workspace-structure section, git conventions). +func AgentsMD(isProject bool) string { + base := `# Workspace Instructions + +This workspace uses ctask. AGENTS.md is the canonical instruction file +for agents working here. Agent-specific files (CLAUDE.md, etc.) are +thin shims that defer to this document. + +## Session Workflow + +- Read handoff.md first when starting a session. It carries the + current-state briefing: last completed work, immediate next step, + blockers, and files to inspect first. +- Update notes.md with decisions, design rationale, and observations + during the session. +- Update handoff.md before ending the session so the next agent (or + the next instance of you) can pick up cleanly. Keep it short and + current -- handoff.md is not a history file. + +## Notes Archival + +When notes.md becomes too large to scan comfortably (roughly 300-500 +lines or 10-20 KB), archive older completed-phase sections: + +1. Create a new file: context/notes-archive/YYYY-MM-DD-topic.md +2. Move the older sections into it, preserving them exactly. +3. Add a pointer at the top of notes.md: + "Older notes archived in context/notes-archive/YYYY-MM-DD-topic.md" +4. Do not delete or summarize-away historical notes as the only + preservation mechanism. +5. Keep handoff.md short and current. It is not a history file. + +## Cross-Workspace Context + +Related work may exist in other ctask workspaces. Use: + + ctask list --all discover all workspaces, including archived + ctask info view metadata and status + ctask notes read another workspace's notes.md + ctask path get the filesystem path + +Treat other workspaces as read-only unless the user explicitly asks +otherwise. + +## Do Not Touch + +- Do not modify .ctask/ -- those are ctask internals (lease, manifests, + session summary, write lock). +- Do not edit task.yaml's metadata fields by hand. ctask owns them. +` + if !isProject { + return base + ` +## File Placement (task workspace) + +- Source code and scripts -> workspace root or src/ +- Documentation, summaries, reports -> docs/ +- Deliverables and exports -> output/ +- Reference material and imported files -> context/ +- Do not place non-code outputs in the workspace root. +` + } + return base + ` +## Workspace Structure (project workspace) + +This workspace uses ctask's project layout. The workspace root contains +ctask management files; your project code lives in the project +subdirectory (named after the workspace slug). When working on project +code, operate inside the project subdirectory. + +- Workspace root: ctask metadata, session logs, notes, reference material +- Project subdirectory: source code, project-specific configuration +- context/: reference material and imported specs (workspace level) +- output/: deliverables and exports (workspace level) +- logs/: session logs (managed by ctask) + +## Git Conventions + +This workspace uses a single git repository initialized at the workspace +root. The project subdirectory and all its contents are tracked by this +root repo. Do not initialize additional git repositories inside the +project subdirectory or elsewhere -- the repo root is the workspace root. +` +} + +// ClaudeShimMD returns the thin CLAUDE.md generated for new workspaces +// whose agent.type is "claude". It exists purely to point Claude Code at +// AGENTS.md; canonical instructions live there. Per v0.6 spec §6. +func ClaudeShimMD() string { + return ` + +# Claude Code Instructions + +This workspace uses AGENTS.md as the canonical agent guidance file. +Read AGENTS.md first. + +## Claude-specific notes + +- Use claude CLI conventions for file operations. +- Respect the workspace structure described in AGENTS.md. +` +} + +// HandoffMD returns the minimal handoff.md template seeded into new +// workspaces. The agent fills it in at session end; the next session +// reads it first. Per v0.6 spec §8. +func HandoffMD() string { + return `# Handoff + +## Current state + +New workspace. No work completed yet. + +## Next step + +[Describe the task or project goal here.] + +## Files to read first + +- AGENTS.md - workspace instructions and conventions +` +} diff --git a/internal/seed/agents_test.go b/internal/seed/agents_test.go new file mode 100644 index 0000000..ffffe5f --- /dev/null +++ b/internal/seed/agents_test.go @@ -0,0 +1,72 @@ +package seed + +import ( + "strings" + "testing" +) + +func TestAgentsMDContainsHandoffWorkflow(t *testing.T) { + body := AgentsMD(false) + for _, want := range []string{ + "handoff.md", // mentions the handoff file + "Read handoff.md first", // session-start convention + "notes.md", // mentions notes + "context/notes-archive/", // archive convention + "300", // size hint + "ctask list", // cross-workspace discovery + } { + if !strings.Contains(body, want) { + t.Errorf("AGENTS.md missing %q", want) + } + } +} + +func TestAgentsMDProjectVariantHasGitConventions(t *testing.T) { + body := AgentsMD(true) + for _, want := range []string{ + "git", // git mentioned for projects + "Workspace Structure", // project-specific section + } { + if !strings.Contains(body, want) { + t.Errorf("AGENTS.md (project) missing %q", want) + } + } +} + +func TestClaudeShimMDPointsAtAgentsMD(t *testing.T) { + body := ClaudeShimMD() + for _, want := range []string{ + "Generated by ctask", + "AGENTS.md", + "Read AGENTS.md first", + } { + if !strings.Contains(body, want) { + t.Errorf("CLAUDE.md shim missing %q", want) + } + } +} + +func TestHandoffMDIsMinimalTemplate(t *testing.T) { + body := HandoffMD() + for _, want := range []string{ + "# Handoff", + "Current state", + "Next step", + "AGENTS.md", + } { + if !strings.Contains(body, want) { + t.Errorf("handoff.md missing %q", want) + } + } +} + +func TestAgentsMDIsASCII(t *testing.T) { + for _, body := range []string{AgentsMD(false), AgentsMD(true), ClaudeShimMD(), HandoffMD()} { + for i, b := range []byte(body) { + if b > 127 { + t.Errorf("non-ASCII byte 0x%02x at position %d", b, i) + break + } + } + } +} diff --git a/internal/seed/templates.go b/internal/seed/templates.go index 195792c..116ef38 100644 --- a/internal/seed/templates.go +++ b/internal/seed/templates.go @@ -2,131 +2,6 @@ package seed import "fmt" -// ClaudeMD returns the built-in default CLAUDE.md content for a task workspace. -// Parameters are accepted for API compatibility but are not interpolated in v0.3. -func ClaudeMD(slug, category, workspacePath string) string { - _ = slug - _ = category - _ = workspacePath - return `# Workspace Guidelines - -This is a ctask workspace. Prefer operating inside this directory unless explicitly instructed otherwise. - -## File Placement - -- Source code and scripts -> workspace root or ` + "`src/`" + ` -- Documentation, summaries, reports -> ` + "`docs/`" + ` -- Deliverables and exports -> ` + "`output/`" + ` -- Reference material and imported files -> ` + "`context/`" + ` -- Do not place non-code outputs (docs, summaries, exports) in the workspace root - -## Conventions - -- Do not install global packages or modify system files unless asked -- Record important assumptions and actions in notes.md -- Keep the workspace root clean -- use subdirectories for organization - -## Cross-Workspace Context - -Related work may exist in other ctask workspaces. For project continuation, -migration, debugging, or building on prior work, inspect related workspaces -before making changes. - -Available commands: - - ctask list --all discover all workspaces, including archived - ctask info view metadata and status of any workspace - ctask notes read another workspace's notes.md - ctask path get the filesystem path to inspect files directly - -Treat other workspaces as read-only unless the user explicitly asks you to -modify them. - -## Session Handoff - -Before ending a session, append a brief summary to notes.md with: - -- What was accomplished -- Key decisions made -- Open follow-ups or unfinished work -- How to continue from here - -Keep it concise -- a few bullet points is enough. -` -} - -// ClaudeMDProject returns the built-in default CLAUDE.md content for a project workspace. -func ClaudeMDProject() string { - return `# Project Workspace Guidelines - -This is a ctask project workspace -- a long-lived working environment, not a disposable task. - -## Workspace Structure - -This is a ctask project workspace. The workspace root contains ctask management -files. Your project code lives in the project subdirectory. - -- Workspace root: ctask metadata, session logs, notes, reference material -- Project subdirectory: source code, project CLAUDE.md, project configuration -- ` + "`context/`" + `: reference material and imported specs (workspace level) -- ` + "`output/`" + `: deliverables and exports (workspace level) -- ` + "`logs/`" + `: session logs (managed by ctask) - -When working on project code, operate inside the project subdirectory. -Place project-specific CLAUDE.md, documentation, and configuration there. - -## File Placement - -- Source code -> ` + "`src/`" + ` (inside the project subdirectory) -- Documentation -> ` + "`docs/`" + ` (workspace level for general notes, project subdir for project docs) -- Deliverables and exports -> ` + "`output/`" + ` (workspace level) -- Reference material -> ` + "`context/`" + ` (workspace level) -- Tests -> ` + "`tests/`" + ` (inside the project subdirectory) -- Configuration files -> inside the project subdirectory -- Do not place non-code outputs in the workspace root - -## Conventions - -- This project uses git. Commit meaningful changes with clear messages. -- Do not install global packages or modify system files unless asked. -- Record important assumptions and actions in notes.md (workspace level). -- Keep the workspace root clean. - -## Git - -This workspace uses a single git repository initialized at the workspace root. -The project subdirectory and all its contents are tracked by this root repo. -Do not initialize additional git repositories inside the project subdirectory -or any other subdirectory. If you need to check git status or make commits, -the repo root is the workspace root. - -## Cross-Workspace Context - -Related work may exist in other ctask workspaces. For project continuation, -migration, debugging, or building on prior work, inspect related workspaces -before making changes. - -Available commands: - - ctask list --all discover all workspaces, including archived - ctask info view metadata and status of any workspace - ctask notes read another workspace's notes.md - ctask path get the filesystem path to inspect files directly - -Treat other workspaces as read-only unless the user explicitly asks you to -modify them. - -## Session Handoff - -Before ending a session, append a brief summary to notes.md with: - -- What was accomplished -- Key decisions made -- Open follow-ups or unfinished work -- How to continue from here -` -} - // NotesMD returns the skeleton notes.md content. func NotesMD(title string) string { return fmt.Sprintf(`# %s diff --git a/internal/seed/templates_test.go b/internal/seed/templates_test.go index 9f3bcf2..0a011ef 100644 --- a/internal/seed/templates_test.go +++ b/internal/seed/templates_test.go @@ -1,128 +1,9 @@ package seed import ( - "strings" "testing" ) -func TestClaudeMDIsASCII(t *testing.T) { - content := ClaudeMD("test-slug", "general", "/tmp/test") - for i, b := range []byte(content) { - if b > 127 { - t.Errorf("non-ASCII byte 0x%02x at position %d in CLAUDE.md template", b, i) - break - } - } -} - -func TestClaudeMDContainsV03Sections(t *testing.T) { - content := ClaudeMD("ignored", "ignored", "ignored") - for _, want := range []string{ - "# Workspace Guidelines", - "## File Placement", - "Source code and scripts", - "Documentation, summaries, reports", - "Deliverables and exports", - "Reference material and imported files", - "## Conventions", - "## Session Handoff", - "What was accomplished", - } { - if !strings.Contains(content, want) { - t.Errorf("CLAUDE.md missing required section: %q", want) - } - } -} - -func TestClaudeMDProjectIsASCII(t *testing.T) { - content := ClaudeMDProject() - for i, b := range []byte(content) { - if b > 127 { - t.Errorf("non-ASCII byte 0x%02x at position %d in project CLAUDE.md template", b, i) - break - } - } -} - -func TestClaudeMDProjectContainsRequiredSections(t *testing.T) { - content := ClaudeMDProject() - for _, want := range []string{ - "# Project Workspace Guidelines", - "long-lived working environment", - "## File Placement", - "Source code -> ", - "Tests -> ", - "## Conventions", - "This project uses git", - "## Session Handoff", - } { - if !strings.Contains(content, want) { - t.Errorf("project CLAUDE.md missing required section: %q", want) - } - } -} - -func TestClaudeMDProjectContainsNestedGitRule(t *testing.T) { - body := ClaudeMDProject() - for _, must := range []string{ - "single git repository", - "Do not initialize additional git repositories", - } { - if !strings.Contains(body, must) { - t.Errorf("ClaudeMDProject missing guidance %q", must) - } - } -} - -func TestClaudeMDProjectDescribesWorkspaceStructure(t *testing.T) { - body := ClaudeMDProject() - for _, must := range []string{ - "## Workspace Structure", - "Workspace root: ctask metadata", - "Project subdirectory", - "project CLAUDE.md", - "operate inside the project subdirectory", - } { - if !strings.Contains(body, must) { - t.Errorf("project CLAUDE.md missing marker %q", must) - } - } -} - -func TestClaudeMDContainsCrossWorkspaceSection(t *testing.T) { - // v0.5.2: both templates teach agents to use ctask's read-only context - // commands before starting work. - content := ClaudeMD("ignored", "ignored", "ignored") - for _, must := range []string{ - "## Cross-Workspace Context", - "ctask list --all", - "ctask info ", - "ctask notes ", - "ctask path ", - "Treat other workspaces as read-only", - } { - if !strings.Contains(content, must) { - t.Errorf("task CLAUDE.md missing cross-workspace marker %q", must) - } - } -} - -func TestClaudeMDProjectContainsCrossWorkspaceSection(t *testing.T) { - content := ClaudeMDProject() - for _, must := range []string{ - "## Cross-Workspace Context", - "ctask list --all", - "ctask info ", - "ctask notes ", - "ctask path ", - "Treat other workspaces as read-only", - } { - if !strings.Contains(content, must) { - t.Errorf("project CLAUDE.md missing cross-workspace marker %q", must) - } - } -} - func TestNotesMDIsASCII(t *testing.T) { content := NotesMD("test title") for i, b := range []byte(content) { diff --git a/internal/session/adopt.go b/internal/session/adopt.go index 3e35f3e..7030d0d 100644 --- a/internal/session/adopt.go +++ b/internal/session/adopt.go @@ -68,7 +68,7 @@ func AdoptExistingPersistentSession(tmuxPath, sessionName, wsDir string, opts La fmt.Fprintf(os.Stderr, "[ctask] warning: failed to remove orphaned lease: %v\n", rmErr) } - lease := NewLease(startTime, opts.Agent, opts.Mode) + lease := NewLease(startTime, leaseAgentCommand(opts), opts.Mode) if err := WriteLease(leasePath, lease); err != nil { return fmt.Errorf("writing lease: %w", err) } @@ -152,12 +152,12 @@ func finalizeAdopted(opts LaunchOpts, wsDir string, startManifest *Manifest, sta } diff := DiffManifests(startManifest, endManifest) - agent := opts.Agent + agentCmd := leaseAgentCommand(opts) if opts.Shell { - agent = "shell" + agentCmd = "shell" } info := &SessionInfo{ - Agent: agent, + Agent: agentCmd, Mode: opts.Mode, StartTime: startTime, EndTime: endTime, @@ -170,7 +170,7 @@ func finalizeAdopted(opts LaunchOpts, wsDir string, startManifest *Manifest, sta } summary := SummarizeFromDiff( - sessionID, currentHostname(), agent, opts.Mode, + sessionID, currentHostname(), agentCmd, opts.Mode, startTime, endTime, diff, endManifest, ) summary.EndReason = "tmux_session_ended" diff --git a/internal/session/adopt_pidcheck_test.go b/internal/session/adopt_pidcheck_test.go new file mode 100644 index 0000000..73f94f9 --- /dev/null +++ b/internal/session/adopt_pidcheck_test.go @@ -0,0 +1,45 @@ +package session + +import ( + "testing" + "time" +) + +// A fresh-by-wall-clock local lease whose owner PID is dead must be +// classified LeaseStateStale by InspectLease, so the persistent-mode +// dispatcher routes to adoption immediately rather than after the 60s +// wall-clock wait. (dispatchPersistent itself lives in cmd/ and is +// covered there; this test pins the InspectLease half of the contract.) +func TestInspectLeaseDeadLocalPIDIsStale(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessDead }) + ws := t.TempDir() + now := time.Now().UTC() + writeLeaseAt(t, ws, &Lease{ + SessionID: "test", + PID: 4242, + Hostname: currentHostname(), + LastHeartbeatAt: now, // fresh by wall-clock + StartedAt: now, + }) + if got := InspectLease(ws); got != LeaseStateStale { + t.Errorf("dead-PID local lease: InspectLease = %v, want LeaseStateStale", got) + } +} + +// The control case: a fresh local lease with a live PID stays FreshLocal, +// so passive reattach (not adoption) is chosen. +func TestInspectLeaseLiveLocalPIDIsFreshLocal(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessAlive }) + ws := t.TempDir() + now := time.Now().UTC() + writeLeaseAt(t, ws, &Lease{ + SessionID: "test", + PID: 4242, + Hostname: currentHostname(), + LastHeartbeatAt: now, + StartedAt: now, + }) + if got := InspectLease(ws); got != LeaseStateFreshLocal { + t.Errorf("live-PID local lease: InspectLease = %v, want LeaseStateFreshLocal", got) + } +} diff --git a/internal/session/adopt_test.go b/internal/session/adopt_test.go index f8e67af..69bb459 100644 --- a/internal/session/adopt_test.go +++ b/internal/session/adopt_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/warrenronsiek/ctask/internal/agent" "github.com/warrenronsiek/ctask/internal/workspace" ) @@ -31,7 +32,7 @@ func newAdoptionFixture(t *testing.T) *adoptionFixture { ID: "demo-id", Slug: "demo", Title: "demo", CreatedAt: initial, UpdatedAt: initial, Status: "active", Category: "projects", Type: "project", - Mode: "local", Agent: "claude", + Mode: "local", Agent: workspace.AgentSpec{Type: "claude"}, } if err := workspace.WriteMeta(filepath.Join(ws, "task.yaml"), meta); err != nil { t.Fatalf("WriteMeta: %v", err) @@ -76,14 +77,14 @@ func (fx *adoptionFixture) stubSeams(t *testing.T) { func defaultAdoptionOpts(wsDir string) LaunchOpts { return LaunchOpts{ - WsDir: wsDir, - Agent: "claude", - Mode: "local", - Slug: "demo", - Category: "projects", - SessionMode: "persistent", - SessionName: "ctask-projects-demo-abc123", - TmuxPath: "/usr/bin/tmux", + WsDir: wsDir, + ResolvedAgent: &agent.Resolved{Command: "claude"}, + Mode: "local", + Slug: "demo", + Category: "projects", + SessionMode: "persistent", + SessionName: "ctask-projects-demo-abc123", + TmuxPath: "/usr/bin/tmux", } } diff --git a/internal/session/isstale_test.go b/internal/session/isstale_test.go new file mode 100644 index 0000000..4325097 --- /dev/null +++ b/internal/session/isstale_test.go @@ -0,0 +1,101 @@ +package session + +import ( + "testing" + "time" +) + +// withCheckProcess swaps the package-level checkProcess seam for the +// duration of a test and restores it on cleanup. Tests using it must not +// run with t.Parallel(). +func withCheckProcess(t *testing.T, fn func(pid int) ProcessState) { + t.Helper() + orig := checkProcess + checkProcess = fn + t.Cleanup(func() { checkProcess = orig }) +} + +func freshLocalLease(pid int) *Lease { + now := time.Now().UTC() + return &Lease{ + PID: pid, + Hostname: currentHostname(), + StartedAt: now, + LastHeartbeatAt: now, + } +} + +// Wall-clock staleness wins unconditionally — existing behavior preserved. +func TestIsStaleByWallClock(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessAlive }) + now := time.Now().UTC() + l := &Lease{PID: 999, Hostname: currentHostname(), LastHeartbeatAt: now.Add(-90 * time.Second)} + if !IsStale(l, now, StaleLeaseAfter) { + t.Error("a lease 90s past heartbeat must be stale even if PID is alive") + } +} + +// Fresh wall-clock + local host + dead owner PID => stale immediately. +func TestIsStaleByDeadPID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessDead }) + l := freshLocalLease(4242) + if !IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with a dead PID must be stale") + } +} + +// Fresh wall-clock + local host + live owner PID => not stale. +func TestIsStaleFreshWithLivePID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessAlive }) + l := freshLocalLease(4242) + if IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with a live PID must not be stale") + } +} + +// Remote lease => PID check skipped entirely; wall-clock only. +func TestIsStaleRemoteIgnoresPID(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { + t.Error("checkProcess must not be called for a remote lease") + return ProcessDead + }) + now := time.Now().UTC() + l := &Lease{PID: 4242, Hostname: "some-other-host", LastHeartbeatAt: now} + if l.Hostname == currentHostname() { + t.Skip("test hostname collision; cannot construct a remote lease") + } + if IsStale(l, now, StaleLeaseAfter) { + t.Error("fresh remote lease must not be stale (PID check skipped)") + } +} + +// Inconclusive PID check => conservative; fall back to wall-clock fresh. +func TestIsStaleUnknownPIDConservative(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { return ProcessUnknown }) + l := freshLocalLease(4242) + if IsStale(l, time.Now().UTC(), StaleLeaseAfter) { + t.Error("fresh local lease with inconclusive PID check must not be stale") + } +} + +// pid <= 0 => skip PID liveness entirely; wall-clock only. +func TestIsStaleInvalidPIDSkipsCheck(t *testing.T) { + withCheckProcess(t, func(int) ProcessState { + t.Error("checkProcess must not be called when pid <= 0") + return ProcessDead + }) + now := time.Now().UTC() + for _, pid := range []int{0, -1} { + l := &Lease{PID: pid, Hostname: currentHostname(), LastHeartbeatAt: now} + if IsStale(l, now, StaleLeaseAfter) { + t.Errorf("fresh local lease with pid=%d must not be stale (wall-clock only)", pid) + } + } +} + +// Defensive: a nil lease is stale. +func TestIsStaleNilLease(t *testing.T) { + if !IsStale(nil, time.Now().UTC(), StaleLeaseAfter) { + t.Error("nil lease must be reported stale") + } +} diff --git a/internal/session/lease.go b/internal/session/lease.go index b12efa2..24e556f 100644 --- a/internal/session/lease.go +++ b/internal/session/lease.go @@ -131,6 +131,41 @@ func IsFresh(l *Lease, now time.Time, threshold time.Duration) bool { return now.Sub(l.LastHeartbeatAt) <= threshold } +// IsStale reports whether the lease should be treated as stale. It +// supplements the wall-clock heartbeat threshold with PID liveness: +// +// - A heartbeat older than threshold is stale (existing behavior). +// - Otherwise, for a lease whose hostname matches the current host and +// whose PID is valid (> 0), a dead owner PID makes the lease stale +// immediately. This is the v0.6 lazy-cleanup signal: a Ctrl-C'd or +// terminal-closed session is recognized without the 60s wall-clock +// wait. +// - Remote leases, leases with pid <= 0, and inconclusive PID checks +// (ProcessUnknown) fall back to wall-clock freshness only. +// +// PID liveness can only flip a wall-clock-fresh lease to stale; it never +// revives a wall-clock-stale lease. IsStale is the freshness predicate +// for adoption and Layer-1 decisions — prefer it over a bare IsFresh +// call at any site that decides whether a session is still owned. +func IsStale(l *Lease, now time.Time, threshold time.Duration) bool { + if l == nil { + return true + } + if !IsFresh(l, now, threshold) { + return true // wall-clock stale — existing behavior + } + if l.PID <= 0 { + return false // no usable PID — wall-clock only + } + if l.Hostname != currentHostname() { + return false // remote lease — wall-clock only + } + if checkProcess(l.PID) == ProcessDead { + return true // local owner confirmed dead + } + return false // alive or inconclusive — conservative +} + // CleanupStaleLease inspects the lease at path: // - missing file: no-op, returns (nil, nil) // - corrupt / unparseable: removes the file, returns (nil, nil) @@ -154,7 +189,7 @@ func CleanupStaleLease(path string, staleAfter time.Duration) (*Lease, error) { } return nil, nil } - if IsFresh(&l, time.Now(), staleAfter) { + if !IsStale(&l, time.Now(), staleAfter) { return nil, nil } if rmErr := os.Remove(path); rmErr != nil && !errors.Is(rmErr, os.ErrNotExist) { diff --git a/internal/session/lease_inspect.go b/internal/session/lease_inspect.go index 71beaca..ee915ba 100644 --- a/internal/session/lease_inspect.go +++ b/internal/session/lease_inspect.go @@ -56,7 +56,7 @@ func InspectLease(wsDir string) LeaseState { if l == nil { return LeaseStateNone } - if !IsFresh(l, time.Now(), StaleLeaseAfter) { + if IsStale(l, time.Now(), StaleLeaseAfter) { return LeaseStateStale } if l.Hostname != currentHostname() { diff --git a/internal/session/pidcheck.go b/internal/session/pidcheck.go new file mode 100644 index 0000000..6e968ec --- /dev/null +++ b/internal/session/pidcheck.go @@ -0,0 +1,22 @@ +package session + +// ProcessState is the tri-state result of a PID liveness check. The call +// site decides how to treat ProcessUnknown; IsStale treats it +// conservatively by falling back to wall-clock freshness. +type ProcessState int + +const ( + // ProcessAlive: the OS confirms a process with this PID exists. + ProcessAlive ProcessState = iota + // ProcessDead: the OS confirms no process with this PID exists. + ProcessDead + // ProcessUnknown: the liveness check was inconclusive (permission + // error, unexpected OS error). Callers must NOT treat this as dead. + ProcessUnknown +) + +// checkProcess is the test seam for PID liveness. Production code uses the +// platform-specific defaultCheckProcess. Tests override this package-level +// variable; such tests must restore it via t.Cleanup and must NOT run with +// t.Parallel(). +var checkProcess = defaultCheckProcess diff --git a/internal/session/pidcheck_test.go b/internal/session/pidcheck_test.go new file mode 100644 index 0000000..71a5261 --- /dev/null +++ b/internal/session/pidcheck_test.go @@ -0,0 +1,38 @@ +package session + +import ( + "os" + "os/exec" + "runtime" + "testing" +) + +// The compiled-in defaultCheckProcess (platform-specific) must report the +// running test process as alive. +func TestCheckProcessSelfAlive(t *testing.T) { + if got := defaultCheckProcess(os.Getpid()); got != ProcessAlive { + t.Errorf("defaultCheckProcess(self pid=%d) = %v, want ProcessAlive", os.Getpid(), got) + } +} + +// A child process that has exited and been reaped must report as dead. +// Best-effort: a PID-reuse race is theoretically possible but negligible +// within a test run. +func TestCheckProcessDeadAfterExit(t *testing.T) { + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + cmd = exec.Command("cmd", "/c", "exit", "0") + } else { + cmd = exec.Command("true") + } + if err := cmd.Start(); err != nil { + t.Fatalf("start child: %v", err) + } + pid := cmd.Process.Pid + if err := cmd.Wait(); err != nil { + t.Fatalf("wait child: %v", err) + } + if got := defaultCheckProcess(pid); got != ProcessDead { + t.Errorf("defaultCheckProcess(exited child pid=%d) = %v, want ProcessDead", pid, got) + } +} diff --git a/internal/session/pidcheck_unix.go b/internal/session/pidcheck_unix.go new file mode 100644 index 0000000..0326ecc --- /dev/null +++ b/internal/session/pidcheck_unix.go @@ -0,0 +1,27 @@ +//go:build !windows + +package session + +import ( + "errors" + "syscall" +) + +// defaultCheckProcess reports whether process pid exists, using signal 0 +// (the standard POSIX existence probe). pid is assumed > 0; IsStale guards +// the pid <= 0 case before calling. +func defaultCheckProcess(pid int) ProcessState { + err := syscall.Kill(pid, 0) + switch { + case err == nil: + return ProcessAlive + case errors.Is(err, syscall.ESRCH): + return ProcessDead + case errors.Is(err, syscall.EPERM): + // Process exists but is owned by another user. It exists — that + // is all IsStale needs. Conservative: treat as alive. + return ProcessAlive + default: + return ProcessUnknown + } +} diff --git a/internal/session/pidcheck_windows.go b/internal/session/pidcheck_windows.go new file mode 100644 index 0000000..dd0ac73 --- /dev/null +++ b/internal/session/pidcheck_windows.go @@ -0,0 +1,38 @@ +//go:build windows + +package session + +import ( + "errors" + "syscall" +) + +// processQueryLimitedInformation is the minimal access right needed to +// probe a process's existence. Defined locally to avoid a dependency on +// golang.org/x/sys/windows. +const processQueryLimitedInformation = 0x1000 + +// errorInvalidParameter is the Win32 error OpenProcess returns when no +// process with the given PID exists. +const errorInvalidParameter = syscall.Errno(87) + +// defaultCheckProcess reports whether process pid exists, by trying to +// open a query handle to it. pid is assumed > 0; IsStale guards the +// pid <= 0 case before calling. +// +// Note: a handle that opens successfully is treated as ProcessAlive even +// in the rare zombie-handle case — this is conservative (it preserves +// wall-clock fallback rather than declaring a live owner dead). +func defaultCheckProcess(pid int) ProcessState { + h, err := syscall.OpenProcess(processQueryLimitedInformation, false, uint32(pid)) + if err != nil { + if errors.Is(err, errorInvalidParameter) { + return ProcessDead + } + // Access-denied or any other error: the process may exist; do + // not claim it is dead. + return ProcessUnknown + } + syscall.CloseHandle(h) + return ProcessAlive +} diff --git a/internal/session/run.go b/internal/session/run.go index 9e1e7eb..c10012a 100644 --- a/internal/session/run.go +++ b/internal/session/run.go @@ -7,19 +7,57 @@ import ( "path/filepath" "time" + "github.com/warrenronsiek/ctask/internal/agent" "github.com/warrenronsiek/ctask/internal/lockfile" "github.com/warrenronsiek/ctask/internal/shell" "github.com/warrenronsiek/ctask/internal/workspace" ) +// execAgent is the test seam for the direct-mode child launch. Tests +// replace this to capture the (command, args, wsDir, env) tuple Run +// would have passed to shell.ExecAgent. Restored via t.Cleanup. +var execAgent = shell.ExecAgent + +// mergeAgentEnv overlays agentEnv on top of base, returning a new map. +// Per v0.6 spec §5 ("merged into the agent's environment, after ctask's +// own exported vars"), agentEnv keys WIN on collision — the spec is +// explicit that user-supplied env vars take precedence. Callers that +// want a warning for shadowed CTASK_* keys must emit it themselves +// (ctask agents check surfaces this). +func mergeAgentEnv(base, agentEnv map[string]string) map[string]string { + out := make(map[string]string, len(base)+len(agentEnv)) + for k, v := range base { + out[k] = v + } + for k, v := range agentEnv { + out[k] = v + } + return out +} + +// leaseAgentCommand returns the command string recorded in leases and +// summaries for diagnostics. A nil ResolvedAgent yields "" (only happens +// in tests that do not exercise a launch). +func leaseAgentCommand(opts LaunchOpts) string { + if opts.ResolvedAgent != nil { + return opts.ResolvedAgent.Command + } + return "" +} + // LaunchOpts configures a session launch. type LaunchOpts struct { WsDir string EnvVars map[string]string - Agent string - Mode string - Slug string - Shell bool // true = interactive shell, false = agent + + // ResolvedAgent is the launch-ready agent (command + args + env), + // produced by the cmd layer via agent.Resolve. Required in agent mode; + // in shell mode it is still populated for lease/summary diagnostics. + ResolvedAgent *agent.Resolved + + Mode string + Slug string + Shell bool // true = interactive shell, false = agent // LaunchDir is the workspace-relative launch directory (v0.5). Empty for // tasks and pre-v0.5 projects. When set, Run resolves the absolute path @@ -127,7 +165,7 @@ func Run(opts LaunchOpts) error { leasePath := LeasePath(opts.WsDir) ownLease := !preflight.ActiveLeaseFound if ownLease { - lease := NewLease(startTime, opts.Agent, opts.Mode) + lease := NewLease(startTime, leaseAgentCommand(opts), opts.Mode) skipped, lockErr := lockfile.WithLock( ctaskWriteLockPath(opts.WsDir), sessionWriteLockTimeout, sessionWriteLockStaleAfter, @@ -187,6 +225,15 @@ func Run(opts LaunchOpts) error { fmt.Fprintln(os.Stderr, launchWarn) } + // Agent-launch branches dereference ResolvedAgent; a nil here is an + // internal wiring bug (the cmd layer must always Resolve before Run). + if !opts.Shell && opts.ResolvedAgent == nil { + if hb != nil { + hb.Stop() + } + return fmt.Errorf("internal error: LaunchOpts.ResolvedAgent is nil in agent mode") + } + // ---- Run the child ---- var childErr error switch { @@ -213,7 +260,9 @@ func Run(opts LaunchOpts) error { if opts.Shell { childErr = shell.ExecTmuxShell(opts.TmuxPath, opts.SessionName, launchAbs, opts.EnvVars) } else { - childErr = shell.ExecTmuxAgent(opts.TmuxPath, opts.SessionName, launchAbs, opts.EnvVars, opts.Agent) + childErr = shell.ExecTmuxAgent(opts.TmuxPath, opts.SessionName, launchAbs, + mergeAgentEnv(opts.EnvVars, opts.ResolvedAgent.Env), + opts.ResolvedAgent.Command, opts.ResolvedAgent.Args) } case opts.Shell: childErr = shell.ExecShell(launchAbs, opts.EnvVars, opts.Slug, opts.Mode) @@ -221,7 +270,8 @@ func Run(opts LaunchOpts) error { for _, line := range shell.BannerLines(opts.Mode, opts.Slug, opts.WsDir, opts.LaunchDir) { fmt.Println(line) } - childErr = shell.ExecAgent(opts.Agent, launchAbs, opts.EnvVars) + childErr = execAgent(opts.ResolvedAgent.Command, opts.ResolvedAgent.Args, launchAbs, + mergeAgentEnv(opts.EnvVars, opts.ResolvedAgent.Env)) } if hb != nil { @@ -284,12 +334,12 @@ func finalize(opts LaunchOpts, startManifest *Manifest, startTime, endTime time. } diff := DiffManifests(startManifest, endManifest) - agent := opts.Agent + agentCmd := leaseAgentCommand(opts) if opts.Shell { - agent = "shell" + agentCmd = "shell" } info := &SessionInfo{ - Agent: agent, + Agent: agentCmd, Mode: opts.Mode, StartTime: startTime, EndTime: endTime, @@ -307,7 +357,7 @@ func finalize(opts LaunchOpts, startManifest *Manifest, startTime, endTime time. } summary := SummarizeFromDiff( - sessionID, currentHostname(), agent, opts.Mode, + sessionID, currentHostname(), agentCmd, opts.Mode, startTime, endTime, diff, endManifest, ) if opts.SessionMode == "persistent" { diff --git a/internal/session/run_env_test.go b/internal/session/run_env_test.go new file mode 100644 index 0000000..552d24f --- /dev/null +++ b/internal/session/run_env_test.go @@ -0,0 +1,72 @@ +package session + +import ( + "os" + "path/filepath" + "testing" + + "github.com/warrenronsiek/ctask/internal/agent" +) + +func TestMergeAgentEnvOverlays(t *testing.T) { + base := map[string]string{"CTASK_WORKSPACE": "/ws", "PATH": "/bin"} + agentEnv := map[string]string{"OPENAI_API_KEY": "x", "CTASK_WORKSPACE": "shadowed"} + got := mergeAgentEnv(base, agentEnv) + + if got["OPENAI_API_KEY"] != "x" { + t.Errorf("OPENAI_API_KEY = %q, want x", got["OPENAI_API_KEY"]) + } + if got["PATH"] != "/bin" { + t.Errorf("PATH = %q, want /bin (untouched base)", got["PATH"]) + } + if got["CTASK_WORKSPACE"] != "shadowed" { + t.Errorf("CTASK_WORKSPACE = %q, want shadowed (agent.env wins per spec §5)", + got["CTASK_WORKSPACE"]) + } +} + +func TestRunMergesAgentEnvIntoChildEnvironment(t *testing.T) { + wsDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(wsDir, ".ctask"), 0755); err != nil { + t.Fatalf("mkdir .ctask: %v", err) + } + + var capturedEnv map[string]string + prev := execAgent + execAgent = func(command string, args []string, dir string, env map[string]string) error { + capturedEnv = env + return nil + } + t.Cleanup(func() { execAgent = prev }) + + err := Run(LaunchOpts{ + WsDir: wsDir, + EnvVars: map[string]string{ + "CTASK_WORKSPACE": "/ws-base", + "PATH": "/bin", + }, + Mode: "local", + Slug: "env-merge", + ResolvedAgent: &agent.Resolved{ + Command: "go", + Args: nil, + Env: map[string]string{ + "OPENAI_API_KEY": "x", + "CTASK_WORKSPACE": "shadowed-by-agent-env", + }, + }, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if capturedEnv["OPENAI_API_KEY"] != "x" { + t.Errorf("OPENAI_API_KEY = %q, want x", capturedEnv["OPENAI_API_KEY"]) + } + if capturedEnv["PATH"] != "/bin" { + t.Errorf("PATH = %q, want /bin", capturedEnv["PATH"]) + } + if capturedEnv["CTASK_WORKSPACE"] != "shadowed-by-agent-env" { + t.Errorf("CTASK_WORKSPACE = %q, want shadowed-by-agent-env (spec §5: agent.env wins)", + capturedEnv["CTASK_WORKSPACE"]) + } +} diff --git a/internal/session/run_integration_test.go b/internal/session/run_integration_test.go index 80db557..6f2124c 100644 --- a/internal/session/run_integration_test.go +++ b/internal/session/run_integration_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "testing" "time" + + "github.com/warrenronsiek/ctask/internal/agent" ) func TestFinalizeWritesSummaryAndClearsLease(t *testing.T) { @@ -31,10 +33,10 @@ func TestFinalizeWritesSummaryAndClearsLease(t *testing.T) { end := time.Now().UTC().Truncate(time.Second) opts := LaunchOpts{ - WsDir: wsDir, - Agent: "claude", - Mode: "local", - Slug: "test", + WsDir: wsDir, + ResolvedAgent: &agent.Resolved{Command: "claude"}, + Mode: "local", + Slug: "test", } if err := finalize(opts, startManifest, start, end, true); err != nil { diff --git a/internal/session/run_preflight.go b/internal/session/run_preflight.go index 3fcfa1a..38540d7 100644 --- a/internal/session/run_preflight.go +++ b/internal/session/run_preflight.go @@ -101,7 +101,7 @@ func runActiveLeaseCheck(opts PreflightOpts) (bool, bool, error) { existing, err := ReadLease(leasePath) switch { case err == nil && existing != nil: - if IsFresh(existing, time.Now(), StaleLeaseAfter) { + if !IsStale(existing, time.Now(), StaleLeaseAfter) { if opts.Force { return true, true, nil } diff --git a/internal/session/status.go b/internal/session/status.go index 689bdc3..f8d3ac9 100644 --- a/internal/session/status.go +++ b/internal/session/status.go @@ -74,7 +74,7 @@ func statusAt(wsDir string, now time.Time) Status { } state := SessionStateStale - if IsFresh(&l, now, StaleLeaseAfter) { + if !IsStale(&l, now, StaleLeaseAfter) { state = SessionStateActive } diff --git a/internal/shell/launch.go b/internal/shell/launch.go index c19d8bd..89683c3 100644 --- a/internal/shell/launch.go +++ b/internal/shell/launch.go @@ -54,14 +54,29 @@ func ContainerNotice() string { return "[ctask] container mode is not available in v0.1. Use local mode or see docs for manual container setup." } -// ExecAgent launches the agent command in the workspace directory. -func ExecAgent(agent string, wsDir string, envVars map[string]string) error { +// execAgentArgs is the pure helper: returns the (path, argv) pair that +// ExecAgent will hand to exec.Command. Extracted so unit tests can +// assert argv shape without spawning a real child. +func execAgentArgs(agent string, args []string) (string, []string, error) { path, err := exec.LookPath(agent) if err != nil { - return fmt.Errorf("agent command not found: %s", agent) + return "", nil, fmt.Errorf("agent command not found: %s", agent) + } + return path, args, nil +} + +// ExecAgent launches the agent command in the workspace directory. +// args is appended after the executable; envVars is merged into the +// process environment via BuildEnvList (which keeps os.Environ() as the +// base — agent.env entries layered into envVars by the caller take +// precedence on collision, per v0.6 spec §5). +func ExecAgent(agent string, args []string, wsDir string, envVars map[string]string) error { + path, argv, err := execAgentArgs(agent, args) + if err != nil { + return err } - cmd := exec.Command(path) + cmd := exec.Command(path, argv...) cmd.Dir = wsDir cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/internal/shell/launch_test.go b/internal/shell/launch_test.go index 421a18d..fd74683 100644 --- a/internal/shell/launch_test.go +++ b/internal/shell/launch_test.go @@ -6,6 +6,28 @@ import ( "testing" ) +func TestExecAgentArgsAppendedAfterPath(t *testing.T) { + // Use any executable guaranteed to exist on PATH. "go" works on dev + // machines; the test suite already assumes Go is present. + path, argv, err := execAgentArgs("go", []string{"version"}) + if err != nil { + t.Skipf("go not on PATH: %v", err) + } + if !strings.HasSuffix(path, "go") && !strings.HasSuffix(path, "go.exe") { + t.Errorf("path = %q, want suffix 'go' or 'go.exe'", path) + } + if len(argv) != 1 || argv[0] != "version" { + t.Errorf("argv = %v, want [version]", argv) + } +} + +func TestExecAgentArgsCommandNotFound(t *testing.T) { + _, _, err := execAgentArgs("definitely-not-on-path-zzz", nil) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("err = %v, want command-not-found error", err) + } +} + func TestDefaultShell(t *testing.T) { cmd := DefaultShell() if runtime.GOOS == "windows" { diff --git a/internal/shell/tmux.go b/internal/shell/tmux.go index 4271e69..c281ff8 100644 --- a/internal/shell/tmux.go +++ b/internal/shell/tmux.go @@ -183,13 +183,15 @@ func pollSessionEndWith(tmuxPath, name string, interval time.Duration, hs func(s } // ExecTmuxAgent orchestrates the three-call pattern for agent mode: -// NewSession -> AttachSession -> PollSessionEnd. +// NewSession -> AttachSession -> PollSessionEnd. args (when non-empty) +// are appended after the agent command on the tmux session's child +// invocation. // // AttachSession failures abort early — the polling loop is meaningful only // after a successful attach (otherwise we'd block waiting for a session // the user never connected to). -func ExecTmuxAgent(tmuxPath, sessionName, launchAbs string, env map[string]string, agent string) error { - if err := NewSession(tmuxPath, sessionName, launchAbs, env, agent); err != nil { +func ExecTmuxAgent(tmuxPath, sessionName, launchAbs string, env map[string]string, agent string, args []string) error { + if err := NewSession(tmuxPath, sessionName, launchAbs, env, agent, args...); err != nil { return err } if err := AttachSession(tmuxPath, sessionName); err != nil { diff --git a/internal/shell/tmux_test.go b/internal/shell/tmux_test.go index 5b6405d..1d1efbe 100644 --- a/internal/shell/tmux_test.go +++ b/internal/shell/tmux_test.go @@ -11,6 +11,21 @@ import ( "time" ) +func TestTmuxArgsIncludesCommandArgs(t *testing.T) { + got := tmuxArgs("session-x", "/tmp/ws", map[string]string{"K": "v"}, "claude", "--debug", "--mode=foo") + // Tail of args must be: -- claude --debug --mode=foo + if len(got) < 4 { + t.Fatalf("argv too short: %v", got) + } + tail := got[len(got)-4:] + want := []string{"--", "claude", "--debug", "--mode=foo"} + for i, w := range want { + if tail[i] != w { + t.Fatalf("tail = %v, want %v", tail, want) + } + } +} + func TestParseTmuxVersionMajor(t *testing.T) { cases := []struct { raw string diff --git a/internal/workspace/agent_spec_test.go b/internal/workspace/agent_spec_test.go new file mode 100644 index 0000000..cc3e972 --- /dev/null +++ b/internal/workspace/agent_spec_test.go @@ -0,0 +1,127 @@ +package workspace + +import ( + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestAgentSpecRoundTripMappingForm(t *testing.T) { + src := []byte(` +agent: + type: opencode + command: /usr/local/bin/opencode + args: + - --model + - deepseek-coder + env: + OPENAI_API_KEY: ollama +`) + var doc struct { + Agent AgentSpec `yaml:"agent"` + } + if err := yaml.Unmarshal(src, &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if doc.Agent.Type != "opencode" { + t.Errorf("Type = %q, want opencode", doc.Agent.Type) + } + if doc.Agent.Command != "/usr/local/bin/opencode" { + t.Errorf("Command = %q, want /usr/local/bin/opencode", doc.Agent.Command) + } + if got := doc.Agent.Args; len(got) != 2 || got[0] != "--model" || got[1] != "deepseek-coder" { + t.Errorf("Args = %v, want [--model deepseek-coder]", got) + } + if doc.Agent.Env["OPENAI_API_KEY"] != "ollama" { + t.Errorf("Env[OPENAI_API_KEY] = %q, want ollama", doc.Agent.Env["OPENAI_API_KEY"]) + } +} + +func TestAgentSpecLegacyScalarForm(t *testing.T) { + cases := []struct { + name string + src string + wantType string + wantCommand string + }{ + {"builtin claude", "agent: claude", "claude", ""}, + {"builtin opencode", "agent: opencode", "opencode", ""}, + {"arbitrary command", "agent: aider", "custom", "aider"}, + {"absolute path", "agent: /opt/agent", "custom", "/opt/agent"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var doc struct { + Agent AgentSpec `yaml:"agent"` + } + if err := yaml.Unmarshal([]byte(tc.src), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if doc.Agent.Type != tc.wantType { + t.Errorf("Type = %q, want %q", doc.Agent.Type, tc.wantType) + } + if doc.Agent.Command != tc.wantCommand { + t.Errorf("Command = %q, want %q", doc.Agent.Command, tc.wantCommand) + } + }) + } +} + +func TestAgentSpecMissingFieldStaysEmpty(t *testing.T) { + // No `agent:` key at all — Type stays empty so resolution falls + // through to the user-level default_agent. + var doc struct { + Agent AgentSpec `yaml:"agent"` + } + if err := yaml.Unmarshal([]byte("title: x\n"), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if doc.Agent.Type != "" || doc.Agent.Command != "" { + t.Errorf("got %+v, want zero AgentSpec", doc.Agent) + } +} + +func TestIsBuiltinAgentType(t *testing.T) { + for _, name := range []string{"claude", "opencode"} { + if !IsBuiltinAgentType(name) { + t.Errorf("IsBuiltinAgentType(%q) = false, want true", name) + } + } + for _, name := range []string{"custom", "gemini", "", "aider"} { + if IsBuiltinAgentType(name) { + t.Errorf("IsBuiltinAgentType(%q) = true, want false", name) + } + } +} + +func TestValidateAgentSpec(t *testing.T) { + cases := []struct { + name string + spec AgentSpec + wantErr string // substring; "" means no error + }{ + {"empty ok (resolved later)", AgentSpec{}, ""}, + {"claude ok", AgentSpec{Type: "claude"}, ""}, + {"opencode ok", AgentSpec{Type: "opencode"}, ""}, + {"custom needs command", AgentSpec{Type: "custom"}, "type \"custom\" requires command"}, + {"custom with command ok", AgentSpec{Type: "custom", Command: "my-agent"}, ""}, + {"unknown type rejected", AgentSpec{Type: "gemini"}, "unknown agent type"}, + {"command with space rejected", AgentSpec{Type: "claude", Command: "claude --debug"}, "must be an executable name"}, + {"command with shell metachar rejected", AgentSpec{Type: "claude", Command: "claude|grep"}, "must be an executable name"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateAgentSpec("ws", &TaskMeta{Agent: tc.spec}) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("got %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("got %v, want substring %q", err, tc.wantErr) + } + }) + } +} diff --git a/internal/workspace/create.go b/internal/workspace/create.go index b450b2e..5672788 100644 --- a/internal/workspace/create.go +++ b/internal/workspace/create.go @@ -12,11 +12,11 @@ import ( // CreateOpts holds parameters for workspace creation. type CreateOpts struct { - Root string - Title string - Category string - Mode string - Agent string + Root string + Title string + Category string + Mode string + AgentSpec AgentSpec // IsProject switches the built-in defaults to project-oriented templates, // records type=project in task.yaml, and triggers project seed overlay. @@ -88,12 +88,16 @@ func Create(opts CreateOpts) (*CreateResult, error) { return nil, fmt.Errorf("creating workspace dir: %w", err) } - // Standard subdirectories - for _, sub := range []string{"context", "output", "logs"} { + // Standard subdirectories. v0.6: context/notes-archive/ is created + // (and pinned by .gitkeep) so the notes-archival convention in + // AGENTS.md has a ready destination directory. + for _, sub := range []string{"context", "context/notes-archive", "output", "logs"} { if err := os.MkdirAll(filepath.Join(wsDir, sub), 0755); err != nil { return nil, fmt.Errorf("creating %s dir: %w", sub, err) } } + gitkeepPath := filepath.Join(wsDir, "context", "notes-archive", ".gitkeep") + _ = os.WriteFile(gitkeepPath, nil, 0644) // Adjust title if slug was suffixed due to collision actualTitle := title @@ -108,7 +112,7 @@ func Create(opts CreateOpts) (*CreateResult, error) { } // Layer 1: built-in defaults (workspace is brand new, so this always writes them). - writeBuiltinDefaults(wsDir, actualTitle, opts.IsProject) + writeBuiltinDefaults(wsDir, actualTitle, opts.IsProject, opts.AgentSpec.Type) // Layer 2: general user seed (overwrites built-in defaults; skips task.yaml/.ctask). if opts.SeedDir != "" { @@ -125,6 +129,13 @@ func Create(opts CreateOpts) (*CreateResult, error) { } meta := &TaskMeta{ + // v0.6: every new workspace is stamped with the current schema + // version and the "native" workspace mode. These keys are written + // ONLY here (and by a future explicit migration command); legacy + // task.yaml files are never retroactively backfilled. The omitempty + // tags on the struct enforce that invariant from the write side. + SchemaVersion: CurrentMetaSchemaVersion, + Workspace: WorkspaceSection{Mode: "native"}, ID: id, Slug: actualSlug, Title: actualTitle, @@ -134,7 +145,7 @@ func Create(opts CreateOpts) (*CreateResult, error) { Category: opts.Category, Type: taskType, Mode: opts.Mode, - Agent: opts.Agent, + Agent: opts.AgentSpec, ArchivedAt: nil, } if opts.IsProject { @@ -161,19 +172,26 @@ func Create(opts CreateOpts) (*CreateResult, error) { return &CreateResult{Path: wsDir, Meta: meta}, nil } -// writeBuiltinDefaults writes the built-in CLAUDE.md and notes.md for a new workspace. -// These files are unconditionally replaceable by the seed overlay layers. -func writeBuiltinDefaults(wsDir, title string, isProject bool) { - claudePath := filepath.Join(wsDir, "CLAUDE.md") - var claudeBody string - if isProject { - claudeBody = seed.ClaudeMDProject() - } else { - // The slug/category/path args are kept for API compatibility but are - // not interpolated by the v0.3 template. - claudeBody = seed.ClaudeMD("", "", wsDir) +// writeBuiltinDefaults writes the v0.6 built-in workspace files for a new +// workspace: AGENTS.md (always), CLAUDE.md shim (claude type only), +// handoff.md (always), and notes.md (always). All four are unconditionally +// replaceable by the seed overlay layers — a user seed dir's AGENTS.md or +// CLAUDE.md wins. +// +// The CLAUDE.md shim is generated only for agentType == "claude" per v0.6 +// spec §6 (the opencode shim is deferred until its instruction-file +// convention is verified). +func writeBuiltinDefaults(wsDir, title string, isProject bool, agentType string) { + agentsPath := filepath.Join(wsDir, "AGENTS.md") + _ = os.WriteFile(agentsPath, []byte(seed.AgentsMD(isProject)), 0644) + + if agentType == "claude" { + claudePath := filepath.Join(wsDir, "CLAUDE.md") + _ = os.WriteFile(claudePath, []byte(seed.ClaudeShimMD()), 0644) } - _ = os.WriteFile(claudePath, []byte(claudeBody), 0644) + + handoffPath := filepath.Join(wsDir, "handoff.md") + _ = os.WriteFile(handoffPath, []byte(seed.HandoffMD()), 0644) notesPath := filepath.Join(wsDir, "notes.md") _ = os.WriteFile(notesPath, []byte(seed.NotesMD(title)), 0644) diff --git a/internal/workspace/create_test.go b/internal/workspace/create_test.go index fb8ae9d..e4130ce 100644 --- a/internal/workspace/create_test.go +++ b/internal/workspace/create_test.go @@ -12,11 +12,11 @@ func TestCreateWorkspace(t *testing.T) { root := t.TempDir() opts := CreateOpts{ - Root: root, - Title: "test task", - Category: "general", - Mode: "local", - Agent: "claude", + Root: root, + Title: "test task", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, } ws, err := Create(opts) @@ -68,11 +68,11 @@ func TestCreateWorkspaceNoTitle(t *testing.T) { root := t.TempDir() opts := CreateOpts{ - Root: root, - Title: "", - Category: "general", - Mode: "local", - Agent: "claude", + Root: root, + Title: "", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, } ws, err := Create(opts) @@ -89,55 +89,140 @@ func TestCreateWorkspaceNoTitle(t *testing.T) { } } -func TestCreateProjectWritesProjectClaudeMD(t *testing.T) { +func TestCreateProjectWritesAgentsMDAndShim(t *testing.T) { root := t.TempDir() opts := CreateOpts{ Root: root, Title: "billing", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, } ws, err := Create(opts) if err != nil { t.Fatalf("Create: %v", err) } - body, err := os.ReadFile(filepath.Join(ws.Path, "CLAUDE.md")) + agentsBody, err := os.ReadFile(filepath.Join(ws.Path, "AGENTS.md")) + if err != nil { + t.Fatalf("read AGENTS.md: %v", err) + } + if !strings.Contains(string(agentsBody), "Workspace Structure (project workspace)") { + t.Errorf("project AGENTS.md missing project-structure section, got:\n%s", agentsBody) + } + claudeBody, err := os.ReadFile(filepath.Join(ws.Path, "CLAUDE.md")) if err != nil { t.Fatalf("read CLAUDE.md: %v", err) } - content := string(body) - if !strings.Contains(content, "# Project Workspace Guidelines") { - t.Errorf("expected project CLAUDE.md, got:\n%s", content) + if !strings.Contains(string(claudeBody), "Read AGENTS.md first") { + t.Errorf("CLAUDE.md is not the AGENTS.md shim, got:\n%s", claudeBody) } if ws.Meta.Type != "project" { t.Errorf("Type: got %q, want \"project\"", ws.Meta.Type) } } -func TestCreateTaskWritesTaskClaudeMD(t *testing.T) { +func TestCreateTaskWritesAgentsMDAndShim(t *testing.T) { root := t.TempDir() opts := CreateOpts{ - Root: root, - Title: "fix bug", - Category: "general", - Mode: "local", - Agent: "claude", + Root: root, + Title: "fix bug", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, } ws, err := Create(opts) if err != nil { t.Fatalf("Create: %v", err) } - body, _ := os.ReadFile(filepath.Join(ws.Path, "CLAUDE.md")) - if !strings.Contains(string(body), "# Workspace Guidelines") { - t.Errorf("expected task CLAUDE.md, got:\n%s", string(body)) + agentsBody, _ := os.ReadFile(filepath.Join(ws.Path, "AGENTS.md")) + if !strings.Contains(string(agentsBody), "File Placement (task workspace)") { + t.Errorf("task AGENTS.md missing task file-placement section, got:\n%s", agentsBody) + } + claudeBody, _ := os.ReadFile(filepath.Join(ws.Path, "CLAUDE.md")) + if !strings.Contains(string(claudeBody), "Read AGENTS.md first") { + t.Errorf("CLAUDE.md is not the AGENTS.md shim, got:\n%s", claudeBody) } if ws.Meta.Type != "task" { t.Errorf("Type: got %q, want \"task\"", ws.Meta.Type) } } +func TestCreateWritesAgentsMDAlways(t *testing.T) { + tmp := t.TempDir() + res, err := Create(CreateOpts{ + Root: tmp, + Title: "agents-md-test", + Category: "general", + AgentSpec: AgentSpec{Type: "opencode"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if _, err := os.Stat(filepath.Join(res.Path, "AGENTS.md")); err != nil { + t.Errorf("AGENTS.md missing: %v", err) + } + // No CLAUDE.md for opencode in v0.6. + if _, err := os.Stat(filepath.Join(res.Path, "CLAUDE.md")); !os.IsNotExist(err) { + t.Errorf("CLAUDE.md should NOT exist for opencode (got %v)", err) + } +} + +func TestCreateWritesHandoffAndContextArchive(t *testing.T) { + tmp := t.TempDir() + res, err := Create(CreateOpts{ + Root: tmp, + Title: "scaffold-test", + Category: "general", + AgentSpec: AgentSpec{Type: "claude"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if _, err := os.Stat(filepath.Join(res.Path, "handoff.md")); err != nil { + t.Errorf("handoff.md missing: %v", err) + } + archiveDir := filepath.Join(res.Path, "context", "notes-archive") + if info, err := os.Stat(archiveDir); err != nil || !info.IsDir() { + t.Errorf("context/notes-archive/ missing or not a dir: %v", err) + } + if _, err := os.Stat(filepath.Join(archiveDir, ".gitkeep")); err != nil { + t.Errorf(".gitkeep missing: %v", err) + } +} + +func TestCreateDoesNotModifyExistingWorkspace(t *testing.T) { + // Pin the no-retroactive-modification invariant: a pre-v0.6 workspace + // that already exists on disk must not acquire AGENTS.md, handoff.md, + // or context/notes-archive/ from any read-side code path. + tmp := t.TempDir() + wsDir := filepath.Join(tmp, "general", "2026-01-01_legacy-ws") + if err := os.MkdirAll(wsDir, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + yaml := []byte("id: 20260101-000000\nslug: legacy-ws\ntitle: legacy\n" + + "created_at: 2026-01-01T00:00:00Z\nupdated_at: 2026-01-01T00:00:00Z\n" + + "status: active\ncategory: general\ntype: task\nmode: local\nagent: claude\n") + if err := os.WriteFile(filepath.Join(wsDir, "task.yaml"), yaml, 0644); err != nil { + t.Fatalf("write task.yaml: %v", err) + } + + // Read it back through ReadMeta — the path every other ctask command + // uses. A read must not trigger any writes to the workspace. + if _, err := ReadMeta(filepath.Join(wsDir, "task.yaml")); err != nil { + t.Fatalf("ReadMeta: %v", err) + } + + for _, file := range []string{"AGENTS.md", "handoff.md"} { + if _, err := os.Stat(filepath.Join(wsDir, file)); !os.IsNotExist(err) { + t.Errorf("legacy workspace acquired %s after ReadMeta (got %v)", file, err) + } + } + if _, err := os.Stat(filepath.Join(wsDir, "context", "notes-archive")); !os.IsNotExist(err) { + t.Errorf("legacy workspace acquired context/notes-archive/ after ReadMeta (got %v)", err) + } +} + func TestCreateAppliesGeneralSeed(t *testing.T) { root := t.TempDir() seedDir := t.TempDir() @@ -150,12 +235,12 @@ func TestCreateAppliesGeneralSeed(t *testing.T) { } opts := CreateOpts{ - Root: root, - Title: "seeded", - Category: "general", - Mode: "local", - Agent: "claude", - SeedDir: seedDir, + Root: root, + Title: "seeded", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, + SeedDir: seedDir, } ws, err := Create(opts) if err != nil { @@ -193,7 +278,7 @@ func TestCreateProjectAppliesGeneralThenProjectSeed(t *testing.T) { Title: "billing", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, SeedDir: general, ProjectSeedDir: project, @@ -222,12 +307,12 @@ func TestCreateSeedDoesNotReplaceTaskYAML(t *testing.T) { t.Fatalf("write seed task.yaml: %v", err) } opts := CreateOpts{ - Root: root, - Title: "no clobber", - Category: "general", - Mode: "local", - Agent: "claude", - SeedDir: seedDir, + Root: root, + Title: "no clobber", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, + SeedDir: seedDir, } ws, err := Create(opts) if err != nil { @@ -249,7 +334,7 @@ func TestCreateSkipCategoryDirPlacesUnderRoot(t *testing.T) { Title: "billing service", Category: "projects", // recorded in metadata Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, SkipCategoryDir: true, } @@ -273,7 +358,7 @@ func TestCreateExplicitCategoryAppendsSubdir(t *testing.T) { Title: "billing service", Category: "backend", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, // SkipCategoryDir: false (default) } @@ -291,11 +376,11 @@ func TestCreateCollisionSuffix(t *testing.T) { root := t.TempDir() opts := CreateOpts{ - Root: root, - Title: "same name", - Category: "general", - Mode: "local", - Agent: "claude", + Root: root, + Title: "same name", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, } ws1, _ := Create(opts) @@ -318,7 +403,7 @@ func TestCreateProjectScaffoldsSubdirAndSetsLaunchDir(t *testing.T) { Title: "litlink-v2", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, }) if err != nil { @@ -345,7 +430,7 @@ func TestCreateProjectSubdirMatchesSuffixedSlug(t *testing.T) { root := t.TempDir() first, err := Create(CreateOpts{ Root: root, Title: "dup", Category: "projects", - Mode: "local", Agent: "claude", IsProject: true, + Mode: "local", AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, }) if err != nil { t.Fatalf("first Create: %v", err) @@ -356,7 +441,7 @@ func TestCreateProjectSubdirMatchesSuffixedSlug(t *testing.T) { second, err := Create(CreateOpts{ Root: root, Title: "dup", Category: "projects", - Mode: "local", Agent: "claude", IsProject: true, + Mode: "local", AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, }) if err != nil { t.Fatalf("second Create: %v", err) @@ -380,7 +465,7 @@ func TestCreateDirectoryPrefixUsesLocalDate(t *testing.T) { expected := time.Now().Format("2006-01-02") // LOCAL — no .UTC() res, err := Create(CreateOpts{ Root: root, Title: "tz-check", Category: "general", - Mode: "local", Agent: "claude", + Mode: "local", AgentSpec: AgentSpec{Type: "claude"}, }) if err != nil { t.Fatalf("Create: %v", err) @@ -397,7 +482,7 @@ func TestCreateStoresTimestampsInUTC(t *testing.T) { root := t.TempDir() res, err := Create(CreateOpts{ Root: root, Title: "tz-check-stored", Category: "general", - Mode: "local", Agent: "claude", + Mode: "local", AgentSpec: AgentSpec{Type: "claude"}, }) if err != nil { t.Fatalf("Create: %v", err) @@ -414,7 +499,7 @@ func TestCreateTaskDoesNotScaffoldSubdir(t *testing.T) { root := t.TempDir() res, err := Create(CreateOpts{ Root: root, Title: "hello", Category: "general", - Mode: "local", Agent: "claude", IsProject: false, + Mode: "local", AgentSpec: AgentSpec{Type: "claude"}, IsProject: false, }) if err != nil { t.Fatalf("Create: %v", err) diff --git a/internal/workspace/git_test.go b/internal/workspace/git_test.go index 41730a3..05531bf 100644 --- a/internal/workspace/git_test.go +++ b/internal/workspace/git_test.go @@ -59,7 +59,7 @@ func TestCreateProjectPreservesGeneralSeedGitignore(t *testing.T) { Title: "gen gi proj", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, SeedDir: general, }) @@ -91,7 +91,7 @@ func TestCreateProjectPreservesProjectSeedGitignore(t *testing.T) { Title: "proj gi proj", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, ProjectSeedDir: project, }) @@ -116,7 +116,7 @@ func TestCreateProjectCreatesMinimalGitignoreWhenNoSeed(t *testing.T) { Title: "no seed gi", Category: "projects", Mode: "local", - Agent: "claude", + AgentSpec: AgentSpec{Type: "claude"}, IsProject: true, }) if err != nil { diff --git a/internal/workspace/metadata.go b/internal/workspace/metadata.go index 6da8d6c..d9d207c 100644 --- a/internal/workspace/metadata.go +++ b/internal/workspace/metadata.go @@ -4,30 +4,205 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/warrenronsiek/ctask/internal/lockfile" "gopkg.in/yaml.v3" ) +// CurrentMetaSchemaVersion is the highest task.yaml schema version +// this binary writes and reads. Reading a higher version is an error +// (see ValidateSchemaVersion). New workspaces always get this value; +// legacy workspaces with a missing schema_version field load as zero +// and are treated as v1 via EffectiveSchemaVersion. +const CurrentMetaSchemaVersion = 1 + +// WorkspaceSection is the nested `workspace:` block in task.yaml, +// introduced in v0.6 alongside schema_version. The only valid Mode +// in v0.6 is "native"; v0.7 will add "adopted". Empty Mode loads +// silently for legacy workspaces (treated as native). +type WorkspaceSection struct { + Mode string `yaml:"mode,omitempty"` +} + // TaskMeta represents the task.yaml schema. // The Type field is v0.3+; older workspaces without this field are treated as "task" // (see EffectiveType). LaunchDir is v0.5+; empty for tasks and pre-v0.5 projects, -// defaults to the project slug for new projects. Field order in this struct is -// the YAML output order. +// defaults to the project slug for new projects. SchemaVersion and Workspace +// are v0.6+; both are omitempty so legacy task.yaml files round-trip without +// acquiring these keys (the "no opportunistic schema writes" invariant — they +// are written ONLY by workspace.Create or a future explicit migration command). +// Field order in this struct is the YAML output order. type TaskMeta struct { - ID string `yaml:"id"` - Slug string `yaml:"slug"` - Title string `yaml:"title"` - CreatedAt time.Time `yaml:"created_at"` - UpdatedAt time.Time `yaml:"updated_at"` - Status string `yaml:"status"` - Category string `yaml:"category"` - Type string `yaml:"type"` - Mode string `yaml:"mode"` - Agent string `yaml:"agent"` - ArchivedAt *time.Time `yaml:"archived_at"` - LaunchDir string `yaml:"launch_dir,omitempty"` + SchemaVersion int `yaml:"schema_version,omitempty"` + Workspace WorkspaceSection `yaml:"workspace,omitempty"` + ID string `yaml:"id"` + Slug string `yaml:"slug"` + Title string `yaml:"title"` + CreatedAt time.Time `yaml:"created_at"` + UpdatedAt time.Time `yaml:"updated_at"` + Status string `yaml:"status"` + Category string `yaml:"category"` + Type string `yaml:"type"` + Mode string `yaml:"mode"` + Agent AgentSpec `yaml:"agent,omitempty"` + ArchivedAt *time.Time `yaml:"archived_at"` + LaunchDir string `yaml:"launch_dir,omitempty"` +} + +// AgentSpec is the v0.6 agent profile carried in task.yaml. All fields are +// optional; a missing Type falls through to the user-level default_agent at +// resolution time (see internal/agent.Resolve). +type AgentSpec struct { + Type string `yaml:"type,omitempty"` + Command string `yaml:"command,omitempty"` + Args []string `yaml:"args,omitempty"` + Env map[string]string `yaml:"env,omitempty"` +} + +// IsBuiltinAgentType reports whether name is a built-in agent type with a +// known default command (claude, opencode). "custom" is NOT a built-in — +// it is the escape hatch. internal/agent.BuiltinProfiles must mirror this +// exact set; when a new built-in lands, update both together. +func IsBuiltinAgentType(name string) bool { + switch name { + case "claude", "opencode": + return true + default: + return false + } +} + +// UnmarshalYAML accepts either: +// - a mapping: the v0.6+ structured form, decoded field-by-field. +// - a scalar string: the legacy v0.1-v0.5 form. The scalar is PRESERVED, +// not dropped. A scalar matching a built-in name (claude, opencode) +// maps to AgentSpec{Type: }. Any other scalar — a path or an +// arbitrary command such as `aider` or `/opt/agent` — maps to +// AgentSpec{Type: "custom", Command: }, so a legacy workspace +// keeps launching the agent it was created with. +// - a null/empty node: Type stays empty so resolution falls through to +// the user-level default_agent (the agent field is effectively +// missing). +func (a *AgentSpec) UnmarshalYAML(node *yaml.Node) error { + switch node.Kind { + case yaml.ScalarNode: + var s string + if err := node.Decode(&s); err != nil { + return fmt.Errorf("agent: %w", err) + } + switch { + case s == "": + // Empty/null — leave Type empty; resolver fills in the default. + case IsBuiltinAgentType(s): + *a = AgentSpec{Type: s} + default: + *a = AgentSpec{Type: "custom", Command: s} + } + return nil + case yaml.MappingNode: + type rawAgentSpec AgentSpec // alias to avoid recursion + var v rawAgentSpec + if err := node.Decode(&v); err != nil { + return fmt.Errorf("agent: %w", err) + } + *a = AgentSpec(v) + return nil + default: + return fmt.Errorf("agent: expected string or mapping, got node kind %d", node.Kind) + } +} + +// knownAgentTypes is the set of agent types accepted in task.yaml. "custom" +// is the escape hatch — any executable can be used, but agent.command is +// required for it. Built-in profiles (claude, opencode) live in +// internal/agent.BuiltinProfiles; this set must stay in sync with that map +// and with IsBuiltinAgentType. +var knownAgentTypes = map[string]struct{}{ + "claude": {}, + "opencode": {}, + "custom": {}, +} + +// agentCommandForbidden is the set of characters disallowed in agent.command. +// command must be an executable name or absolute/relative path — arguments +// belong in agent.args. Whitespace and shell metacharacters indicate the +// user is trying to embed args; we reject them with a hint. +const agentCommandForbidden = " \t|&;<>()$`" + +// ValidateAgentSpec enforces the v0.6 invariants on the agent block. An +// empty Type is always allowed (resolution fills in default_agent at +// launch time). Any non-empty Type must be in knownAgentTypes. type:custom +// requires a non-empty Command. Command (when set) must look like a single +// executable: no whitespace, no shell metacharacters. +func ValidateAgentSpec(slug string, m *TaskMeta) error { + if m == nil { + return nil + } + a := m.Agent + if a.Type != "" { + if _, ok := knownAgentTypes[a.Type]; !ok { + return fmt.Errorf("workspace %q: unknown agent type %q (must be claude, opencode, or custom)", + slug, a.Type) + } + } + if a.Type == "custom" && a.Command == "" { + return fmt.Errorf("workspace %q: agent type \"custom\" requires command field", + slug) + } + if a.Command != "" && strings.ContainsAny(a.Command, agentCommandForbidden) { + return fmt.Errorf("workspace %q: agent.command %q must be an executable name or path with no whitespace or shell metacharacters; put arguments in agent.args", + slug, a.Command) + } + return nil +} + +// EffectiveSchemaVersion returns the schema version a meta should be +// treated as. A stored value of 0 means the field was missing in +// task.yaml (legacy pre-v0.6 workspace); per spec, these are treated +// as schema version 1. Any non-zero stored value is returned verbatim. +func EffectiveSchemaVersion(m *TaskMeta) int { + if m == nil { + return 1 + } + if m.SchemaVersion == 0 { + return 1 + } + return m.SchemaVersion +} + +// ValidateSchemaVersion returns an error if the meta's stored +// SchemaVersion is greater than CurrentMetaSchemaVersion. A stored +// value of 0 (legacy missing field) is always accepted — the meta is +// not retroactively rewritten by ctask. +func ValidateSchemaVersion(slug string, m *TaskMeta) error { + if m == nil { + return nil + } + if m.SchemaVersion > CurrentMetaSchemaVersion { + return fmt.Errorf( + "workspace %q uses schema version %d, but this binary supports up to version %d. Please upgrade ctask.", + slug, m.SchemaVersion, CurrentMetaSchemaVersion) + } + return nil +} + +// ValidateWorkspaceMode returns an error if the meta's Workspace.Mode +// is set to anything other than "" (legacy) or "native" (current). The +// "adopted" mode is reserved for v0.7 and must not be accepted here. +func ValidateWorkspaceMode(slug string, m *TaskMeta) error { + if m == nil { + return nil + } + switch m.Workspace.Mode { + case "", "native": + return nil + default: + return fmt.Errorf( + "workspace %q has unsupported workspace.mode %q (only \"native\" is valid in this binary version)", + slug, m.Workspace.Mode) + } } // EffectiveType returns "task" or "project". An empty or missing Type field @@ -54,7 +229,12 @@ func WriteMeta(path string, meta *TaskMeta) error { return os.WriteFile(path, data, 0644) } -// ReadMeta reads a TaskMeta from a YAML file. +// ReadMeta reads a TaskMeta from a YAML file. v0.6: also enforces +// the schema_version and workspace.mode contracts — task.yaml files +// declaring a higher schema version than this binary supports, or +// a workspace.mode value other than "" or "native", are rejected. +// Legacy files missing either field load silently and are treated +// via the Effective* helpers. func ReadMeta(path string) (*TaskMeta, error) { data, err := os.ReadFile(path) if err != nil { @@ -64,6 +244,19 @@ func ReadMeta(path string) (*TaskMeta, error) { if err := yaml.Unmarshal(data, &meta); err != nil { return nil, err } + slug := meta.Slug + if slug == "" { + slug = filepath.Base(filepath.Dir(path)) + } + if err := ValidateSchemaVersion(slug, &meta); err != nil { + return nil, err + } + if err := ValidateWorkspaceMode(slug, &meta); err != nil { + return nil, err + } + if err := ValidateAgentSpec(slug, &meta); err != nil { + return nil, err + } return &meta, nil } diff --git a/internal/workspace/metadata_test.go b/internal/workspace/metadata_test.go index b4d26f3..3a80687 100644 --- a/internal/workspace/metadata_test.go +++ b/internal/workspace/metadata_test.go @@ -17,7 +17,7 @@ func TestWriteMetaLockedHappyPath(t *testing.T) { ID: "1", Slug: "s", Title: "t", CreatedAt: now, UpdatedAt: now, Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Mode: "local", Agent: AgentSpec{Type: "claude"}, } if err := WriteMetaLocked(metaPath, meta); err != nil { @@ -68,16 +68,16 @@ func TestWriteAndReadMeta(t *testing.T) { now := time.Now().UTC().Truncate(time.Second) meta := &TaskMeta{ - ID: "20260405-143022", - Slug: "arch-notes", - Title: "arch notes", - CreatedAt: now, - UpdatedAt: now, - Status: "active", - Category: "general", - Mode: "local", - Agent: "claude", - ArchivedAt: nil, + ID: "20260405-143022", + Slug: "arch-notes", + Title: "arch notes", + CreatedAt: now, + UpdatedAt: now, + Status: "active", + Category: "general", + Mode: "local", + Agent: AgentSpec{Type: "claude"}, + ArchivedAt: nil, } if err := WriteMeta(path, meta); err != nil { @@ -112,15 +112,15 @@ func TestMetaYAMLFieldsPresent(t *testing.T) { now := time.Now().UTC().Truncate(time.Second) meta := &TaskMeta{ - ID: "20260405-143022", - Slug: "test", - Title: "test", - CreatedAt: now, - UpdatedAt: now, - Status: "active", - Category: "general", - Mode: "local", - Agent: "claude", + ID: "20260405-143022", + Slug: "test", + Title: "test", + CreatedAt: now, + UpdatedAt: now, + Status: "active", + Category: "general", + Mode: "local", + Agent: AgentSpec{Type: "claude"}, } WriteMeta(path, meta) @@ -141,16 +141,16 @@ func TestMetaTypeRoundTrip(t *testing.T) { now := time.Now().UTC().Truncate(time.Second) meta := &TaskMeta{ - ID: "20260410-120000", - Slug: "billing", - Title: "billing", - CreatedAt: now, - UpdatedAt: now, - Status: "active", - Category: "projects", - Type: "project", - Mode: "local", - Agent: "claude", + ID: "20260410-120000", + Slug: "billing", + Title: "billing", + CreatedAt: now, + UpdatedAt: now, + Status: "active", + Category: "projects", + Type: "project", + Mode: "local", + Agent: AgentSpec{Type: "claude"}, } if err := WriteMeta(path, meta); err != nil { t.Fatalf("WriteMeta: %v", err) @@ -221,15 +221,15 @@ func TestMetaArchive(t *testing.T) { now := time.Now().UTC().Truncate(time.Second) meta := &TaskMeta{ - ID: "20260405-143022", - Slug: "test", - Title: "test", - CreatedAt: now, - UpdatedAt: now, - Status: "active", - Category: "general", - Mode: "local", - Agent: "claude", + ID: "20260405-143022", + Slug: "test", + Title: "test", + CreatedAt: now, + UpdatedAt: now, + Status: "active", + Category: "general", + Mode: "local", + Agent: AgentSpec{Type: "claude"}, } WriteMeta(path, meta) @@ -265,7 +265,7 @@ func TestMetaLaunchDirRoundTrip(t *testing.T) { Category: "projects", Type: "project", Mode: "local", - Agent: "claude", + Agent: AgentSpec{Type: "claude"}, LaunchDir: "demo", } if err := WriteMeta(path, meta); err != nil { @@ -289,8 +289,8 @@ func TestMetaLaunchDirOmittedWhenEmpty(t *testing.T) { ID: "t", Slug: "t", Title: "t", CreatedAt: time.Now().UTC().Truncate(time.Second), UpdatedAt: time.Now().UTC().Truncate(time.Second), - Status: "active", Category: "general", Type: "task", - Mode: "local", Agent: "claude", + Status: "active", Category: "general", Type: "task", + Mode: "local", Agent: AgentSpec{Type: "claude"}, } if err := WriteMeta(path, meta); err != nil { t.Fatalf("WriteMeta: %v", err) diff --git a/internal/workspace/query_test.go b/internal/workspace/query_test.go index ddc4a24..82c661c 100644 --- a/internal/workspace/query_test.go +++ b/internal/workspace/query_test.go @@ -30,16 +30,16 @@ func createTestWorkspaceFull(t *testing.T, root, category, dirName, status, task // Extract slug from dirName (skip "YYYY-MM-DD_") slug := dirName[11:] meta := &TaskMeta{ - ID: "test", - Slug: slug, - Title: slug, - CreatedAt: updatedAt, - UpdatedAt: updatedAt, - Status: status, - Category: category, - Type: taskType, - Mode: "local", - Agent: "claude", + ID: "test", + Slug: slug, + Title: slug, + CreatedAt: updatedAt, + UpdatedAt: updatedAt, + Status: status, + Category: category, + Type: taskType, + Mode: "local", + Agent: AgentSpec{Type: "claude"}, } WriteMeta(filepath.Join(dir, "task.yaml"), meta) } @@ -175,16 +175,16 @@ func createFlatProjectWorkspaceFull(t *testing.T, root, dirName string, updatedA os.MkdirAll(dir, 0755) slug := dirName[11:] meta := &TaskMeta{ - ID: "test", - Slug: slug, - Title: slug, - CreatedAt: updatedAt, - UpdatedAt: updatedAt, - Status: "active", - Category: "projects", - Type: "project", - Mode: "local", - Agent: "claude", + ID: "test", + Slug: slug, + Title: slug, + CreatedAt: updatedAt, + UpdatedAt: updatedAt, + Status: "active", + Category: "projects", + Type: "project", + Mode: "local", + Agent: AgentSpec{Type: "claude"}, } WriteMeta(filepath.Join(dir, "task.yaml"), meta) } diff --git a/internal/workspace/schema_test.go b/internal/workspace/schema_test.go new file mode 100644 index 0000000..03eaf9c --- /dev/null +++ b/internal/workspace/schema_test.go @@ -0,0 +1,305 @@ +package workspace + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestNewMetaIncludesSchemaVersion — workspace.Create writes +// schema_version: 1 into the new task.yaml. +func TestNewMetaIncludesSchemaVersion(t *testing.T) { + root := t.TempDir() + result, err := Create(CreateOpts{ + Root: root, + Title: "Test schema", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + body, err := os.ReadFile(filepath.Join(result.Path, "task.yaml")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !strings.Contains(string(body), "schema_version: 1") { + t.Errorf("task.yaml should contain schema_version: 1, got:\n%s", body) + } +} + +// TestNewMetaIncludesWorkspaceMode — workspace.Create writes +// workspace.mode: native into the new task.yaml. +func TestNewMetaIncludesWorkspaceMode(t *testing.T) { + root := t.TempDir() + result, err := Create(CreateOpts{ + Root: root, + Title: "Test mode", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + body, err := os.ReadFile(filepath.Join(result.Path, "task.yaml")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + text := string(body) + if !strings.Contains(text, "workspace:") { + t.Errorf("task.yaml should contain workspace: block, got:\n%s", text) + } + if !strings.Contains(text, "mode: native") { + t.Errorf("task.yaml should contain mode: native, got:\n%s", text) + } +} + +// TestNewMetaRoundTripWithSchemaFields — write + read returns the +// schema fields verbatim. +func TestNewMetaRoundTripWithSchemaFields(t *testing.T) { + root := t.TempDir() + result, err := Create(CreateOpts{ + Root: root, + Title: "Round trip", + Category: "general", + Mode: "local", + AgentSpec: AgentSpec{Type: "claude"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + meta, err := ReadMeta(filepath.Join(result.Path, "task.yaml")) + if err != nil { + t.Fatalf("ReadMeta: %v", err) + } + if meta.SchemaVersion != 1 { + t.Errorf("SchemaVersion: got %d, want 1", meta.SchemaVersion) + } + if meta.Workspace.Mode != "native" { + t.Errorf("Workspace.Mode: got %q, want native", meta.Workspace.Mode) + } +} + +// TestLegacyMetaDefaultsToSchemaVersion1 — a task.yaml without the +// schema_version field still loads, and EffectiveSchemaVersion returns +// 1 for it (the spec rule: missing schema_version is treated as v1). +func TestLegacyMetaDefaultsToSchemaVersion1(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "task.yaml") + body := []byte(`id: legacy +slug: legacy-task +title: legacy task +created_at: 2026-04-01T00:00:00Z +updated_at: 2026-04-01T00:00:00Z +status: active +category: general +type: task +mode: local +agent: claude +archived_at: null +`) + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write legacy yaml: %v", err) + } + + meta, err := ReadMeta(path) + if err != nil { + t.Fatalf("ReadMeta: %v", err) + } + if meta.SchemaVersion != 0 { + t.Errorf("stored SchemaVersion: got %d, want 0 (legacy missing)", meta.SchemaVersion) + } + if eff := EffectiveSchemaVersion(meta); eff != 1 { + t.Errorf("EffectiveSchemaVersion legacy: got %d, want 1", eff) + } + if meta.Workspace.Mode != "" { + t.Errorf("Workspace.Mode legacy: got %q, want empty", meta.Workspace.Mode) + } +} + +// TestUnsupportedSchemaVersionRefused — task.yaml with schema_version +// higher than CurrentMetaSchemaVersion is rejected at read time. +func TestUnsupportedSchemaVersionRefused(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "task.yaml") + body := []byte(`id: future +slug: future-task +title: future +schema_version: 2 +created_at: 2026-04-01T00:00:00Z +updated_at: 2026-04-01T00:00:00Z +status: active +category: general +type: task +mode: local +agent: claude +`) + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + meta, err := ReadMeta(path) + if err == nil { + t.Fatalf("expected error for schema_version 2, got meta=%+v", meta) + } + if !strings.Contains(err.Error(), "schema version") { + t.Errorf("error should mention schema version, got: %v", err) + } + if !strings.Contains(err.Error(), "upgrade") { + t.Errorf("error should mention upgrade, got: %v", err) + } +} + +// TestInvalidWorkspaceModeRefused — task.yaml with workspace.mode set +// to a value other than "native" (e.g., "adopted") is rejected. v0.7 +// adds "adopted"; v0.6 only knows "native". +func TestInvalidWorkspaceModeRefused(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "task.yaml") + body := []byte(`id: adopted-test +slug: adopted-test +title: adopted test +schema_version: 1 +workspace: + mode: adopted +created_at: 2026-04-01T00:00:00Z +updated_at: 2026-04-01T00:00:00Z +status: active +category: general +type: task +mode: local +agent: claude +`) + if err := os.WriteFile(path, body, 0644); err != nil { + t.Fatalf("write: %v", err) + } + + meta, err := ReadMeta(path) + if err == nil { + t.Fatalf("expected error for workspace.mode=adopted, got meta=%+v", meta) + } + if !strings.Contains(err.Error(), "adopted") { + t.Errorf("error should mention the bad mode value, got: %v", err) + } +} + +// TestLegacyTaskYamlNotBackfilledByWrite — the no-opportunistic-writes +// invariant: when WriteMeta or WriteMetaLocked is called against a meta +// that was read from a legacy task.yaml (no schema_version, no +// workspace block), the re-written file MUST NOT acquire those keys. +// Backfilling them on an unrelated metadata update would be an implicit +// schema migration that the v0.6 spec explicitly forbids. +func TestLegacyTaskYamlNotBackfilledByWrite(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "task.yaml") + original := []byte(`id: legacy +slug: legacy-task +title: legacy task +created_at: 2026-04-01T00:00:00Z +updated_at: 2026-04-01T00:00:00Z +status: active +category: general +type: task +mode: local +agent: claude +archived_at: null +`) + if err := os.WriteFile(path, original, 0644); err != nil { + t.Fatalf("write legacy yaml: %v", err) + } + + // Simulate what resume / archive / restore do: read, mutate one + // unrelated field, write back. + meta, err := ReadMeta(path) + if err != nil { + t.Fatalf("ReadMeta: %v", err) + } + meta.UpdatedAt = time.Date(2026, 5, 14, 12, 0, 0, 0, time.UTC) + if err := WriteMeta(path, meta); err != nil { + t.Fatalf("WriteMeta: %v", err) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read back: %v", err) + } + rewritten := string(got) + if strings.Contains(rewritten, "schema_version") { + t.Errorf("legacy task.yaml MUST NOT acquire schema_version on rewrite; got:\n%s", rewritten) + } + if strings.Contains(rewritten, "workspace:") { + t.Errorf("legacy task.yaml MUST NOT acquire workspace block on rewrite; got:\n%s", rewritten) + } +} + +// TestLegacyTaskYamlNotBackfilledByLockedWrite — same invariant via +// WriteMetaLocked (the path used by resume/archive/restore in +// production). +func TestLegacyTaskYamlNotBackfilledByLockedWrite(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "task.yaml") + original := []byte(`id: legacy +slug: legacy-task +title: legacy task +created_at: 2026-04-01T00:00:00Z +updated_at: 2026-04-01T00:00:00Z +status: active +category: general +type: task +mode: local +agent: claude +archived_at: null +`) + if err := os.WriteFile(path, original, 0644); err != nil { + t.Fatalf("write legacy yaml: %v", err) + } + + meta, err := ReadMeta(path) + if err != nil { + t.Fatalf("ReadMeta: %v", err) + } + meta.UpdatedAt = time.Date(2026, 5, 14, 12, 0, 0, 0, time.UTC) + if err := WriteMetaLocked(path, meta); err != nil { + t.Fatalf("WriteMetaLocked: %v", err) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read back: %v", err) + } + rewritten := string(got) + if strings.Contains(rewritten, "schema_version") { + t.Errorf("legacy task.yaml MUST NOT acquire schema_version on locked rewrite; got:\n%s", rewritten) + } + if strings.Contains(rewritten, "workspace:") { + t.Errorf("legacy task.yaml MUST NOT acquire workspace block on locked rewrite; got:\n%s", rewritten) + } +} + +// TestValidateSchemaVersionAccepts0And1 — the helper returns nil for +// 0 (legacy missing) and 1 (current). Higher values error. +func TestValidateSchemaVersionAccepts0And1(t *testing.T) { + for _, v := range []int{0, 1} { + m := &TaskMeta{SchemaVersion: v} + if err := ValidateSchemaVersion("test", m); err != nil { + t.Errorf("ValidateSchemaVersion(%d): got err %v, want nil", v, err) + } + } +} + +// TestValidateWorkspaceModeAccepts — empty and "native" are valid. +func TestValidateWorkspaceModeAccepts(t *testing.T) { + for _, mode := range []string{"", "native"} { + m := &TaskMeta{Workspace: WorkspaceSection{Mode: mode}} + if err := ValidateWorkspaceMode("test", m); err != nil { + t.Errorf("ValidateWorkspaceMode(%q): got err %v, want nil", mode, err) + } + } +} diff --git a/notes.md b/notes.md index b262cf9..25d2297 100644 --- a/notes.md +++ b/notes.md @@ -1,14 +1,14 @@ # ctask — Session Handoff Notes -Last touched: 2026-05-14. **v0.5.4 is shipped on `main` and installed locally as `v0.5.4`. Branch `feat/v0.5.4-session-visibility-polish` merged via `--no-ff` (merge commit `10b7d5a`) and deleted. No tag pushed (no remote). Theme: surface workspace session state in everyday commands + finish the v0.5.3 invocation-name work + bring `docs/commands.md` current — pure polish, no new subsystems.** +Last touched: 2026-05-15. **v0.6 Phases 1–3 are all implemented and verified on branch `feat/v0.6-multi-agent-config` (18 commits ahead of `main`, NOT yet merged). Version is bumped to `0.6.0`. The branch is feature-complete for v0.6 and awaiting review before the merge into `main`. The installed binary at `%LOCALAPPDATA%\ctask\bin\ctask.exe` is still `v0.5.4` — the branch builds locally but has not been installed.** ## Where we are -- **`main`:** v0.5.4 (session-visibility polish). Tip at merge commit `10b7d5a`; version-bump commit `7704cd9`. Installed binary at `%LOCALAPPDATA%\ctask\bin\ctask.exe` is `v0.5.4`. v0.5.3 still load-bearing underneath — none of its surfaces changed. -- **Active branches:** none. v0.5.4 feature branch was deleted post-merge. -- **Pending action:** none. v0.6 Phase 1 is the next horizon (config file + schema_version + workspace.mode + source attribution in doctor/info). See "Next" section below. +- **`main`:** v0.5.4 (session-visibility polish). Tip at merge commit `10b7d5a`; version-bump commit `7704cd9`. Installed binary at `%LOCALAPPDATA%\ctask\bin\ctask.exe` is `v0.5.4`. Not refreshed — the v0.6 branch builds locally but has not been installed. +- **Active branches:** `feat/v0.6-multi-agent-config` — 18 commits ahead of `main`, all under the v0.6 theme (Phase 1 + Phase 2 + Phase 3). Not yet merged. +- **Pending action:** review of v0.6 Phase 3, then merge `feat/v0.6-multi-agent-config` into `main`. Do not merge before explicit approval. - Remote: none (local-only, intentional — see `CLAUDE.md`). -- `ctask doctor` reports 5 pass/fail + 2 seed-directory + 1 `CTASK_PROJECT_ROOT` check + 1 `Session mode` INFO line + 1 tmux INFO/FAIL line (when persistent mode is configured). Unchanged in v0.5.4. +- `ctask doctor` reports 5 pass/fail + 2 seed-directory + 1 `CTASK_PROJECT_ROOT` check + 1 `Session mode` INFO line + 1 tmux INFO/FAIL line (when persistent mode is configured) + the new v0.6 `── Settings ──` block with per-key source attribution. ### What v0.4 delivered (still true, unchanged) @@ -156,7 +156,7 @@ Out of scope (deferred): ### Known limitation (v0.5.3) - **Refusal/bypass hints reflect `basename(os.Args[0])` for the command-form line, but descriptive prose ("ctask persistent mode requires...") and the ssh-remote hint stay hardcoded as "ctask".** Intentional — descriptive prose refers to program identity, and the ssh-remote `ctask` runs on the remote, not the local binary. **v0.5.4 audit confirmed this split is the right line** (spec §2 codified it: command-form via `invocationName()`, product-identity literal). Closed. -- **Adoption requires waiting 60s for the previous owner's lease to go stale (`StaleLeaseAfter`).** A user who Ctrl-C's the foreground `ctask` and immediately re-runs `ctask resume` hits the v0.4 Layer-1 prompt instead of the adoption path. Acceptable but lazy-cleanup-unfriendly; deferred to v0.6 Phase 2 (see follow-ups). Not in v0.6 Phase 1 scope. +- ~~**Adoption requires waiting 60s for the previous owner's lease to go stale (`StaleLeaseAfter`).**~~ **RESOLVED in v0.6 Phase 3.** A lease whose owner PID is confirmed dead on the local host is now treated as stale immediately via the PID-aware `IsStale` predicate — the 60s wall-clock wait no longer applies to a Ctrl-C'd / terminal-closed local session. See "What v0.6 Phase 3 delivered". ### What v0.5.4 delivered @@ -184,9 +184,227 @@ Spec deviations (intentional, not bugs): - **The v0.5.4 spec referred to `.ctask/lease.json` and a `session_mode` field; the actual existing implementation uses `.ctask/session.json` and `mode`.** The implementation correctly followed the existing metadata names per the spec's "no new metadata fields, no behavioral changes" constraint — renaming would have been a v0.5.3 schema change masquerading as polish. Future specs touching this surface should use `.ctask/session.json` and `mode` unless intentionally renaming the metadata. - **The invocation-name audit found no production hardcoded command-form hints to change.** v0.5.3's `c204d87` had already done the work. v0.5.4 added two regression tests (`TestInvocationNameInActiveSessionPrompt`, `TestInvocationNameInRestoreHintNonCanonical`) that pin a non-canonical invocation name to prevent future drift. The split between command-form (uses `invocationName()`) and product-identity (literal `"ctask"`) is the codified rule per spec §2. -### Next: v0.6 Phase 1 +### What v0.6 Phase 1 delivered (branch `feat/v0.6-multi-agent-config`, NOT merged) -v0.6 is the **last specced direction** but the spec splits into two phases. **Only Phase 1 is in scope for the next branch.** Phase 2 (agent profiles, AGENTS.md / context-file templates, PID liveness, lazy-cleanup adoption changes) is explicitly out of scope until Phase 1 is implemented, tested, and reviewed. +Theme: **infrastructure foundation for v0.6** — global config file, schema versioning in task.yaml, and source attribution in doctor/info. Five commits on the feature branch; no version bump (lands at end of Phase 3). All Phase 1 spec items (`v0.6-spec.md` sections 1–4) covered, plus a follow-up commit added on reviewer request for the native-Windows session_mode launch-time warning. + +#### Commit list (oldest → newest) + +- `6f80c8b` `feat(v0.6): config file parser + resolver + source attribution` + - `internal/config/configfile.go` — strict-key YAML parser; unknown top-level keys invalidate the whole file with the offending key named; future schema versions rejected with an upgrade message. Platform-appropriate path (`$XDG_CONFIG_HOME/ctask/config.yaml` → `~/.config/...` fallback on Unix; `%APPDATA%\ctask\config.yaml` on native Windows). + - `internal/config/resolver.go` — `Resolver` / `ResolvedSetting` / `SettingSource` with five values (`Builtin`, `ConfigFileSrc`, `EnvVar`, `CLIFlag` reserved for v0.6 Phase 2, `PlatformOverride`). Each setting carries an `Override` chain so doctor can render `env var (overrides config file: X)`. Per-setting methods: `CtaskRoot`, `ProjectRoot`, `SeedDir`, `DefaultAgent`, `DefaultCategory`, `Editor`, `SessionMode`. `SessionMode` applies the native-Windows platform override at the resolver layer with the configured value chained as `Override`. Exports `SetConfigPathForTest` and `SetIsNativeWindowsForTest` so cross-package tests can isolate. + - `internal/config/config.go` — legacy `ResolveRoot` / `ResolveAgent` / `ResolveSeedDir` / `ResolveProjectRoot` / `ResolveSessionMode` wrappers migrated to `LoadResolver()` so config-file values take effect for entry commands. `ResolveProjectRoot` preserves the `""` sentinel for `Builtin` source so `SearchRoots` and doctor's `checkProjectRoot` keep their v0.5 fallback semantics. `ResolveSessionMode` keeps the unknown-value stderr warning, distinguishing env-var vs config-file sources in the message. + - Tests: 6 LoadConfigFile + 3 ConfigFilePath + 11 resolver cases including layering, override chains, path expansion, platform override, and `SettingSource.String()` rendering. +- `0b21b8d` `feat(v0.6): schema_version and workspace.mode in task.yaml` + - `CurrentMetaSchemaVersion = 1` constant + `WorkspaceSection struct{ Mode string }` nested block. + - `TaskMeta` gains `SchemaVersion int` and `Workspace WorkspaceSection` fields at the top of the struct. Both `omitempty` — this is what enforces the no-opportunistic-writes invariant: legacy task.yaml files (no `schema_version`, no `workspace:`) round-trip through `WriteMeta` / `WriteMetaLocked` without acquiring those keys. + - `EffectiveSchemaVersion(meta)` returns 1 for stored-value-0 legacy workspaces; non-zero stored values pass through verbatim. + - `ValidateSchemaVersion(slug, meta)` rejects values above `CurrentMetaSchemaVersion` with the spec-mandated upgrade message; `ValidateWorkspaceMode(slug, meta)` rejects values other than `""` and `"native"` (so a Phase 1 binary refuses an "adopted" mode set by hand or by a future v0.7). + - `ReadMeta` now runs both validators after YAML unmarshal. The error includes the workspace slug (derived from the file's `slug:` field or the directory basename when the file itself is corrupt). + - `workspace.Create` stamps every new meta with `SchemaVersion: 1` and `Workspace.Mode: "native"`. This is the ONLY write site for these fields in v0.6. + - Tests: 10 cases including the dedicated `TestLegacyTaskYamlNotBackfilledByWrite` and `TestLegacyTaskYamlNotBackfilledByLockedWrite` regressions that pin the no-opportunistic-writes invariant. +- `c918e5c` `feat(v0.6): doctor Settings section with source attribution` + - New `── Settings ──` block appended between the existing `checkTmux` line and the `N checks passed, M failed` summary. Loaded via `config.LoadResolver()` exactly once per `runDoctor` invocation and reused across every settings line (per user correction: "load the resolver once and reuse it"). + - First emits the config-file lifecycle line: `not found` (INFO), `` (valid, INFO), or `` + `[FAIL] unknown key: "..."` + advisory (invalid, increments `failed`). + - Then iterates `r.CtaskRoot() / ProjectRoot() / SeedDir() / DefaultAgent() / DefaultCategory() / SessionMode() / Editor()` and renders each via `printSettingLine` (key/value/source trio + chained-override info). `PlatformOverride` adds a `configured: ` row so doctor surfaces both the effective and the user-asked-for value. + - Helper `formatSettingSource` centralises the `EnvVar → CTASK_X env var` / `PlatformOverride → ... (persistent mode requires tmux; not available on native Windows)` / override-chain wording. + - Tests: 6 cases including the platform-override `configured: persistent` rendering. +- `937a1c8` `feat(v0.6): info source attribution on Agent and Launch session mode` + - `cmd/info.go::runInfo` calls `config.LoadResolver()` once and reuses it. + - `Agent:` line gains `(workspace)` when `m.Agent` is non-empty (the common case), or `(default)` / `(default — )` when the legacy field is empty and the value falls through to the resolver. + - New `Launch session mode:` row inserted directly between `Agent:` and `Created:`, **outside** the v0.5.4 Session block (per user decision: the Session block represents the current lease's recorded mode; the new line represents the configured launch default — two different things). + - Helper `infoSourceLabel` is the info-side counterpart to `formatSettingSource`. No override-chain suffix in info (single-row layout has no room for the extra parenthetical). + - Tests: 5 cases including a placement check that asserts `Agent < Launch session mode < Created` in the rendered output. +- `6182d89` `feat(v0.6): platform-override stderr warning on launch paths` + - Reviewer follow-up: the v0.6 spec section 1.8 also calls for a stderr warning at launch time when persistent mode is downgraded; commits 1–4 covered the doctor/info display path but the launch paths were still downgrading silently. This commit fixes that. + - New helper `cmd/persistent.go::emitPlatformOverrideWarningIfNeeded(alwaysPersistent bool)` — loads the resolver, emits the warning on stderr when `SessionMode()` reports `PlatformOverride` source AND the caller is not `AlwaysPersistent`. + - Single call site at the top of `cmd/entry.go::defaultRunWorkspaceEntry`, before any launch work. That function is the funnel for `new`, `resume`, `last`, `open` (which can downgrade) AND `attach` (which can't). attach sets `AlwaysPersistent: true`, so the helper short-circuits. + - Warning text (exact): `[ctask] warning: persistent session mode is not supported on native Windows; using direct mode. Use WSL for persistent sessions.` + - "Once per invocation" is provided implicitly by the call site running at most once per ctask command — no warn-once subsystem. + - Tests: 5 cases including the AlwaysPersistent skip gate. attach's actual native-Windows refusal contract (via `preflightPersistentEntry`) continues to be enforced by the pre-existing `TestPreflightRefusesNativeWindows`. + +#### Verification (run on `feat/v0.6-multi-agent-config` tip `6182d89`) + +- `go test ./... -count=1` — all 7 packages `ok`, 0 failures. +- `go vet ./...` — exit 0. +- `go build -o ctask.exe .` — exit 0. +- `just build-linux` — produces `dist/ctask-linux-amd64`, statically linked ELF (`file` reports `statically linked`). +- Version remains `v0.5.4`; bump deferred to the end-of-Phase-3 commit per `v0.6-spec.md` "Commit ordering". + +#### Phase 1 constraints held + +- **No config auto-creation.** `LoadConfigFile` returns `(nil, nil)` on `IsNotExist`; nothing in the codebase writes a config file. Missing file renders as INFO in doctor, never FAIL. +- **No opportunistic schema writes.** The two new task.yaml fields use YAML `omitempty`; legacy files round-trip through `WriteMetaLocked` (the path used by `resume` / `archive` / `restore`) without acquiring `schema_version` or `workspace:`. Pinned by `TestLegacyTaskYamlNotBackfilledByWrite` + `TestLegacyTaskYamlNotBackfilledByLockedWrite`. +- **Env vars preserved as overrides.** The resolver layers them above config; no deprecation. Override chain captured in `ResolvedSetting.Override` so doctor renders `CTASK_X env var (overrides config file: Y)`. +- **task.yaml remains workspace state.** Phase 1 does NOT introduce per-workspace fields like `agent.type` or fold task.yaml into the resolver chain. The Agent and Launch session mode lines in info correctly distinguish `(workspace)` from user-level-default sources. +- **No Phase 2 work started.** Verified by diff: no agent-profile fields in `TaskMeta`, no `--agent` flag on `ctask new`, no AGENTS.md / CLAUDE.md shim generation, no `ctask agents check`, no `context/notes-archive/` scaffolding. `internal/seed/templates.go` unchanged. +- **No Phase 3 work started.** Verified by diff: no PID-liveness logic, no changes to `internal/session/lease.go` or `internal/session/adopt.go`, no change to the 60s `StaleLeaseAfter` threshold or its callers. + +#### Native-Windows platform-override behavior (codified in Phase 1) + +- **Doctor + info show source attribution.** Doctor's Settings block renders `session_mode: direct` + `source: platform override (...)` + `configured: persistent` when config says persistent on native Windows. Info's `Launch session mode:` line surfaces the same effective value with its source label. +- **Launch paths warn once per invocation and downgrade to direct.** `new`, `resume`, `last`, `open` all funnel through `defaultRunWorkspaceEntry`, which calls `emitPlatformOverrideWarningIfNeeded(false)` at the top. Exactly one stderr line per process invocation. No warn-once subsystem — the call site frequency is what enforces "once". +- **`ctask attach` does NOT downgrade.** attach sets `AlwaysPersistent: true`, which (a) bypasses the warning helper and (b) routes through `preflightPersistentEntry` which refuses on native Windows with the v0.5.3 "ctask persistent mode requires tmux ... Recommended: Run ctask from WSL" message. attach has no direct-mode equivalent — refusal is the right contract. + +#### Architecture notes (worth preserving) + +- **One resolver per command.** `config.LoadResolver()` is cheap (one config file read + env snapshot) but doctor/info call it exactly once per invocation and reuse the result across many `Resolver.X()` accessors. Legacy `Resolve*` wrappers (`ResolveRoot` etc.) each construct a fresh resolver — acceptable for entry commands that call them once or twice; new code in `cmd/` should follow the doctor/info pattern. +- **Test seams are exported.** `config.SetConfigPathForTest(t, path)` and `config.SetIsNativeWindowsForTest(t, f)` are exported helpers so `cmd/`-package tests can isolate from developer-host config files and simulate platform-override scenarios without `runtime.GOOS` skips. +- **Validation lives in `ReadMeta`, not in callers.** The schema-version and workspace-mode checks happen at read time — so any path that gets a `*TaskMeta` has already passed validation. Callers do not need to re-check. +- **`omitempty` is the load-bearing primitive for "no opportunistic writes."** If a future change needs to track another optional field, follow the same pattern: zero value must round-trip without serialization, and only `workspace.Create` (or an explicit migration) writes a non-zero value. + +### What v0.6 Phase 2 delivered (branch `feat/v0.6-multi-agent-config`, NOT merged) + +Theme: **the multi-agent layer** — ctask is now agent-agnostic. task.yaml carries an agent profile (`type` / `command` / `args` / `env`), `ctask new --agent` selects the agent type, the launch path carries a resolved command + args + env end-to-end, new workspaces get an `AGENTS.md` canonical instruction file (CLAUDE.md becomes a thin shim), and `ctask agents check` validates agent configuration. Six commits on the feature branch; no version bump (lands at end of Phase 3). All Phase 2 spec items (`v0.6-spec.md` §5–§7, plus the §8 seed scaffolding that rides along with the template change) covered. + +#### Commit list (oldest → newest) + +- `8120c39` `feat(v0.6): AgentSpec field on TaskMeta with backward-compat unmarshal` + - `TaskMeta.Agent` changes from `string` to `workspace.AgentSpec{Type, Command, Args, Env}`. Custom `UnmarshalYAML` accepts both the v0.6 mapping form and the legacy scalar form. **Legacy scalar handling (corrected at plan review):** a scalar matching a built-in name (`claude`, `opencode`) maps to `AgentSpec{Type: }`; any other scalar (a path, `aider`, etc.) maps to `AgentSpec{Type: "custom", Command: }`; a missing `agent:` key leaves `Type` empty so the resolver fills in `default_agent`. The scalar value is NEVER dropped — a legacy workspace keeps launching the agent it was created with. + - `IsBuiltinAgentType(name)` helper in `internal/workspace` (true for `claude`, `opencode`) — the built-in/custom discriminator used by the unmarshaler. `internal/agent.BuiltinProfiles` mirrors the same set. + - `ValidateAgentSpec` (run inside `ReadMeta`): known type only; `type=custom` requires `command`; `command` rejects whitespace and shell metacharacters (`|&;<>()$\``) with a hint pointing at `args` — ctask does not shell-split. + - `CreateOpts.Agent string` → `CreateOpts.AgentSpec workspace.AgentSpec`. cmd-layer call sites patched to the minimum needed to compile (full wiring lands in commit 3). +- `24f2134` `feat(v0.6): internal/agent package — Resolve + BuiltinProfiles` + - New `internal/agent` package: `Profile`, `BuiltinProfiles` (`claude`, `opencode`), `IsKnownType` (those two + `custom`), `Resolved{Type, Command, Args, Env}`, and `Resolve(spec, defaultAgent)`. Resolution: empty `Type` falls through to `defaultAgent`; `custom` requires `command`; otherwise command is `spec.Command` or the built-in default. **No I/O** — PATH lookup stays in `shell.ExecAgent` and `ctask agents check`, so `Resolve` is trivially testable. +- `b75b82e` `feat(v0.6): launch path carries ResolvedAgent (command + args + env)` + - `LaunchOpts.Agent string` and `WorkspaceEntryOptions.Agent string` → `*agent.Resolved`. All five entry commands (`new`, `resume`, `last`, `open`, `attach`) construct an `AgentSpec`, apply `--agent` as a one-shot `agent.command` override (Open Q 1 — resume/last/attach `--agent` stays a command override, NOT a type selector), call `agent.Resolve`, and pass the result through. `cmd/entry.go::resolveEntryAgent` centralises the resume/last/open/attach path. + - `shell.ExecAgent` and `shell.ExecTmuxAgent` gain an `args []string` parameter. `agent.env` is merged into the child environment at the `session.Run` launch switch, AFTER ctask's own `CTASK_*` exports — `agent.env` wins on collision per spec §5. `session.mergeAgentEnv` is the centralised merge; `var execAgent = shell.ExecAgent` is a new test seam. + - Lease, manifest, write lock, heartbeat, summary, and provisional cleanup are untouched. The `Agent string` fields on `Lease` / `SessionSummary` / `SessionInfo` still record the resolved command string for diagnostics. +- `a61f900` `feat(v0.6): --agent flag on ctask new selects agent type` + - `Resolver` gains `cliFlagAgent` + `SetCLIFlagAgent`; `DefaultAgent()` layers `CLIFlag` above `EnvVar` so doctor/info render the full precedence chain. `SettingSource.CLIFlag` (reserved in Phase 1) is now reachable. + - `ctask new --agent ` writes `agent.type` into the new workspace's task.yaml. Resolution + validation run BEFORE `workspace.Create`, so `--agent custom` with no companion command refuses (`type "custom" requires command`) without leaving a half-created workspace on disk. The deferred Phase 1 test `TestCLIFlagOverridesEnvVar` landed here. +- `0c6ed0c` `feat(v0.6): AGENTS.md seed + CLAUDE.md shim + handoff + context-archive scaffold` + - New workspaces get `AGENTS.md` (canonical, always — handoff workflow, notes-archive convention with a ~300-500 line trigger, cross-workspace discovery, do-not-touch warnings; project variant adds workspace-structure + git-conventions sections), a `CLAUDE.md` shim (claude type only — opencode shim deferred), `handoff.md` (minimal current-state template), and `context/notes-archive/.gitkeep`. + - `seed.ClaudeMD` and `seed.ClaudeMDProject` deleted — no callers remain. `seed.NotesMD` retained. Existing workspaces are NOT retroactively modified (pinned by `TestCreateDoesNotModifyExistingWorkspace`). +- `0f96d20` `feat(v0.6): ctask agents check + doctor integration` + - `ctask agents check [workspace]` — pure validation, no launch: agent type known, command resolvable on PATH, launch_dir valid, AGENTS.md present, CLAUDE.md shim present (WARN, claude only). `agent.env` keys displayed informationally; a WARN line names any key shadowing a `CTASK_*` export. Non-zero exit on any FAIL. + - `ctask doctor` runs the same sweep against the most-recently-active workspace (`workspace.MostRecentActive`); skips with `Agent check: skipped (no workspace context)` when none exists. `runAgentsCheckOnWorkspace` is shared between the standalone command and doctor. + - `TestCompletionSubcommandViaExecute` was made order-independent: cobra's default `completion` command captures the root output writer once, on the first `Execute()` in the process — the new agents-check tests run an `Execute()` earlier in the suite, so the test now drops any pre-created completion command before its own `Execute()`. + +#### Verification (run on `feat/v0.6-multi-agent-config` tip `0f96d20`) + +- `go test ./... -count=1` — all 8 packages `ok` (the new `internal/agent` package included), 0 failures. +- `go vet ./...` — exit 0. +- `just build` — `ctask.exe` builds locally. +- `just build-linux` — produces `dist/ctask-linux-amd64`, statically linked ELF. +- Version remains `v0.5.4`; bump deferred to the end-of-Phase-3 commit. + +#### Phase 2 constraints held + +- **No PID liveness / lazy-cleanup / adoption changes.** `internal/session/lease.go` untouched; `internal/session/adopt.go` changed only for the mechanical `ResolvedAgent` passthrough. The 60s `StaleLeaseAfter` threshold is unchanged. +- **No per-workspace `session_mode`.** No new field on `TaskMeta` beyond the `AgentSpec`. +- **No user-defined named agent profiles in config.** `internal/config/configfile.go` is unchanged in Phase 2 — agent profiles live in task.yaml, not config.yaml. +- **No opencode-specific shim.** `writeBuiltinDefaults` writes `CLAUDE.md` only when `agent.Type == "claude"`. +- **No auto-modification of existing workspaces.** All new seed files are written only by `workspace.Create`. +- **No version bump, no merge.** `version.go` untouched; branch stays `feat/v0.6-multi-agent-config`. + +#### Architecture notes (worth preserving) + +- **`internal/agent.Resolve` is I/O-free.** PATH validation is deliberately NOT in `Resolve` — it lives in `shell.ExecAgent` (fail-fast at launch) and `ctask agents check`. This keeps resolution a pure function and lets `agents check` validate without launching. +- **`agent.env` precedence is intentional.** It merges AFTER ctask's `CTASK_*` exports and wins on collision (spec §5). This is a feature, not a bug — `agents check` surfaces the shadowing with a WARN so the user is not surprised. +- **Two parallel built-in-type lists, kept in sync deliberately.** `workspace.IsBuiltinAgentType` / `workspace.knownAgentTypes` and `internal/agent.BuiltinProfiles` both enumerate the built-ins. `internal/workspace` cannot import `internal/agent` (the dependency runs the other way), so the lists are mirrored, not shared. When a new built-in lands, update all three. +- **AGENTS.md is canonical; CLAUDE.md is a shim.** The shim exists only to point Claude Code at AGENTS.md. The seed-overlay rule still applies — a user seed dir's `AGENTS.md` / `CLAUDE.md` overrides the built-in. +- **cobra's default `completion` command captures the output writer once.** `InitDefaultCompletionCmd` snapshots `c.OutOrStdout()` into the `bash`/`zsh`/etc. subcommand closures on the first `Execute()` anywhere in the process. Tests that drive `rootCmd.Execute()` with a redirected output buffer and then assert on completion output must drop the pre-created completion command first. `cmd/agents_check_test.go::captureRootCmd` restores `rootCmd`'s out/err/args on cleanup to limit this class of cross-test contamination. + +### What v0.6 Phase 3 delivered (branch `feat/v0.6-multi-agent-config`, NOT merged) + +Theme: **lazy-cleanup via PID liveness** — a lease whose owner process is +confirmed dead on the local machine is treated as stale immediately, +without the 60-second wall-clock wait. Closes the v0.5.3 known limitation +(Ctrl-C / terminal-close then immediate `ctask resume`). The v0.6.0 +version bump rides along. Four commits; v0.6-spec.md §8 (context-file +scaffolding) was already delivered in Phase 2 commit `0c6ed0c`, so §9 +(PID liveness) was the only remaining Phase 3 feature. + +#### Commit list (oldest → newest) + +- `9070c42` `feat(v0.6): tri-state PID liveness probe (ProcessAlive/Dead/Unknown)` + - New `internal/session/pidcheck.go` (`ProcessState` tri-state + + `checkProcess` test seam) and build-tagged platform files: + `pidcheck_unix.go` (`syscall.Kill(pid, 0)` — `nil`/`EPERM`→Alive, + `ESRCH`→Dead, else→Unknown) and `pidcheck_windows.go` + (`syscall.OpenProcess` — opens→Alive, `ERROR_INVALID_PARAMETER`(87)→Dead, + else→Unknown). Stdlib `syscall` only; no `golang.org/x/sys` dependency, + `go.mod` unchanged. +- `f379a6d` `feat(v0.6): IsStale supplements wall-clock freshness with PID liveness` + - `IsStale(l, now, threshold)` in `lease.go`. Parameterized free + function mirroring `IsFresh` (not the spec's zero-arg method form — + deviation approved at plan review to preserve the package's + injected-clock testability). PID liveness applies only to local + leases (`l.Hostname == currentHostname()`) with `pid > 0`; remote + leases, `pid <= 0`, and `ProcessUnknown` fall back to wall-clock. + Wall-clock staleness is checked first and wins unconditionally — PID + liveness only flips fresh → stale, never stale → fresh. +- `d575ddd` `feat(v0.6): route lease-freshness callsites through IsStale` + - Four freshness consumers now route through `IsStale`: `InspectLease`, + `CleanupStaleLease`, `runActiveLeaseCheck`, and `statusAt`. + `SessionStatus` / `ctask list` / `ctask info` reflect PID liveness + automatically — only the one-line `statusAt` predicate swap was + needed; the `Status` struct and all cmd-layer rendering are + untouched. Also corrected three cmd-package session-display test + fixtures (`list_session_test.go`, `info_session_test.go`) that built + "active" leases with the local hostname but synthetic PIDs — now that + freshness is PID-aware, an honest "active" fixture must use + `os.Getpid()`. +- `beb5174` `chore(v0.6): bump version to 0.6.0` + - `cmd/root.go` `version` `0.5.4` → `0.6.0`. `ctask --version` reports + `ctask v0.6.0`. + +#### Verification (run on tip `beb5174`) + +- `go test ./... -count=1` — all 8 packages `ok`, 0 failures. +- `go vet ./...` — exit 0. +- `just build` — `ctask.exe` (PE32+ x86-64). +- `just build-linux` — `dist/ctask-linux-amd64`, statically linked ELF + (the only check that compiles `pidcheck_unix.go`). +- Binary smoke (v0.6.0 `ctask.exe`, real workspaces under a temp + `CTASK_ROOT`) — **passed**. A lease carrying a reaped, genuinely-dead + owner PID, the real host's hostname, and a 9-second-old heartbeat + (unambiguously wall-clock-fresh) was auto-cleaned by `ctask resume` + with `[ctask] Cleaned up stale session … last seen 9s ago` — no + Layer-1 "Continue anyway?" prompt, no 60s wait, resume proceeded + (exit 0). `ctask info` / `ctask list` on the same kind of lease report + `stale` (they are read-only — they do not remove it). The dead PID + alone (not wall-clock age) drove the staleness, confirming PID + liveness end-to-end in the shipped binary. +- Smoke finding worth keeping: on Windows, if another process holds an + open handle to the dead owner (e.g. a parent that has not yet reaped + it), `OpenProcess` still succeeds and `defaultCheckProcess` returns + `ProcessAlive` — the documented conservative "zombie handle" case. The + lease then falls back to wall-clock. The real Ctrl-C / terminal-close + path has no such lingering handle, so PID liveness fires as intended. + +#### Phase 3 constraints held + +- Four-layer concurrency model unchanged — PID liveness only makes Layer + 1's "is this lease stale?" question smarter. +- `StaleLeaseAfter` (60s) unchanged; PID liveness supplements it and + remains the fallback for remote leases and inconclusive checks. +- Lease creation, heartbeat, write lock, manifest, and summary shapes + unchanged. `adopt.go` untouched. +- Remote leases remain wall-clock-only (PID checks skipped when the lease + hostname differs from the current host). +- `IsFresh` retained as the pure wall-clock primitive `IsStale` builds on. +- No new agent/profile/config/template work. + +#### Architecture notes (worth preserving) + +- **`IsStale` is the single freshness predicate** for stale-detection + decisions. All four callsites route through it; `IsFresh` is now an + internal building block (still exported, still directly tested). +- **PID liveness is conservative by construction.** Only a definitive + `ProcessDead` on a local lease shortcuts the wait. `ProcessUnknown` + (permission errors, unexpected OS errors) and remote leases preserve + the pre-v0.6 wall-clock behavior exactly. +- **No `golang.org/x/sys` dependency.** Windows process probing uses + stdlib `syscall.OpenProcess`; `PROCESS_QUERY_LIMITED_INFORMATION` + (`0x1000`) and `ERROR_INVALID_PARAMETER` (`87`) are local constants. +- **Known conservative edge cases** (acceptable — they never falsely + declare a live owner dead): OS PID reuse reads the recycled PID as + alive → wall-clock fallback; a Windows zombie handle reads as alive. +- **Plan:** `docs/superpowers/plans/2026-05-15-v0.6-phase3-implementation.md`. + +### Historical: original Phase 1 plan (now shipped — kept for traceability) **Phase 1 scope (only thing to start next):** @@ -215,13 +433,13 @@ Covered in v0.4.1 notes. The exit-code gate (`childExitCode != 0 && startManifes ## Tree state at pause -- `main` clean with respect to v0.4, v0.4.1, v0.5, v0.5.1, v0.5.2, v0.5.3, v0.5.4. Latest tip is the merge commit `10b7d5a Merge branch 'feat/v0.5.4-session-visibility-polish' into main`; the v0.5.4 version-bump commit `7704cd9` sits on the merged-in branch tip. -- No tag pushed for v0.5.4 (no remote — the project is intentionally local-only per `CLAUDE.md`). v0.5.3 had `git tag v0.5.3` locally; v0.5.4 has none. Add later if a publication path opens up. -- No active branches. `feat/v0.5.4-session-visibility-polish` was deleted post-merge. -- Installed `ctask.exe` at `%LOCALAPPDATA%\ctask\bin\ctask.exe` is **v0.5.4** (refreshed via `just install` after merge). `dist/ctask-linux-amd64` is the v0.5.4 Linux cross-build (statically linked ELF, 7.5 MiB). -- Memory follow-ups (still live from v0.5.3, both relevant to v0.6 Phase 2 — see `memory/MEMORY.md`): - - `feedback_design_for_lazy_cleanup` — drives v0.6 Phase 2 work on the 60s freshness wait + PID liveness. - - `feedback_invocation_name_in_hints` — partially closed by the v0.5.4 audit (split between command-form and product-identity is now codified). Memory entry retained for the descriptive-prose question, which Phase 2 may revisit. +- `main` tip is unchanged: `10b7d5a Merge branch 'feat/v0.5.4-session-visibility-polish' into main` (v0.5.4 shipped). +- `feat/v0.6-multi-agent-config` is the active branch, 18 commits ahead of `main`. Tip `beb5174` (pre-notes-closeout). NOT merged — feature-complete for v0.6, awaiting review then merge. +- No tag pushed for v0.5.4 (no remote — the project is intentionally local-only per `CLAUDE.md`). v0.5.3 had `git tag v0.5.3` locally; v0.5.4 has none. No v0.6 tag yet — a post-merge task. +- Installed `ctask.exe` at `%LOCALAPPDATA%\ctask\bin\ctask.exe` is still **v0.5.4** — the v0.6 branch has NOT been installed. Local `ctask.exe` in the repo root is a `beb5174` build reporting `v0.6.0`. `dist/ctask-linux-amd64` is the Phase-3 Linux cross-build (statically linked ELF). +- Memory follow-ups (see `memory/MEMORY.md`): + - `feedback_design_for_lazy_cleanup` — the 60s-freshness-wait concern it raised is **addressed by v0.6 Phase 3** (PID-aware `IsStale`). The underlying principle (lifecycle UX must recover from Ctrl-C / terminal close) remains a live design value. + - `feedback_invocation_name_in_hints` — partially closed by the v0.5.4 audit (split between command-form and product-identity is now codified). Memory entry retained for the descriptive-prose question. - Untracked files (do NOT touch without asking — pre-existing session-local working docs, unchanged from this session): - `.claude/settings.local.json` (modified — Claude Code local settings) - `bugfix-provisional-workspace.md` (spec for the 2026-04-22 initial provisional fix; may be deleted or archived) @@ -237,10 +455,29 @@ Covered in v0.4.1 notes. The exit-code gate (`childExitCode != 0 && startManifes - `cmd/invocation_audit_test.go` (regression tests pinning a non-canonical invocation name) - `cmd/resume_archived_polish_test.go` (Cobra-end-to-end test that the duplicate `Error:` line is suppressed) - `docs/commands.md` (rewrite — see commit `4fd0bef`) +- Files committed on `feat/v0.6-multi-agent-config` (NOT yet merged): + - `internal/config/configfile.go` + `configfile_test.go` (strict-key YAML parser + ConfigFilePath) + - `internal/config/resolver.go` + `resolver_test.go` (Resolver + ResolvedSetting + SettingSource + 2 exported test seams) + - `internal/config/config.go` modified (5 legacy wrappers migrated to LoadResolver) + - `internal/config/config_test.go` + `config_roots_test.go` modified (config-path isolation in tests that assert Builtin defaults) + - `internal/workspace/metadata.go` modified (CurrentMetaSchemaVersion, WorkspaceSection, SchemaVersion + Workspace fields on TaskMeta, EffectiveSchemaVersion + ValidateSchemaVersion + ValidateWorkspaceMode helpers, ReadMeta validation) + - `internal/workspace/create.go` modified (writes schema_version: 1 + workspace.mode: native) + - `internal/workspace/schema_test.go` (10 tests including the two no-opportunistic-writes regressions) + - `cmd/doctor.go` modified (── Settings ── block + checkSettings + printSettingLine + formatSettingSource) + - `cmd/doctor_test.go` modified (config import + 2 tests gated by SetIsNativeWindowsForTest) + - `cmd/doctor_settings_test.go` (6 tests covering the new Settings block) + - `cmd/info.go` modified (Agent source label + new Launch session mode line + agentLineWithSource + infoSourceLabel) + - `cmd/info_attribution_test.go` (5 tests including placement assertion) + - `cmd/new_persistent_test.go` modified (config import + 1 test gated by SetIsNativeWindowsForTest) + - `cmd/entry.go` modified (emitPlatformOverrideWarningIfNeeded call at top of defaultRunWorkspaceEntry) + - `cmd/persistent.go` modified (config import + platformOverrideWarning const + emitPlatformOverrideWarningIfNeeded helper) + - `cmd/platform_warning_test.go` (5 tests covering the warning + AlwaysPersistent skip) ## How to resume -v0.5.4 is shipped; no pending follow-through. Next horizon is v0.6 Phase 1 only — see "Next: v0.6 Phase 1" above. **Do not begin Phase 2 work** (agent profiles, AGENTS.md, PID liveness, lazy-cleanup adoption) until Phase 1 is implemented, tested, and reviewed. +v0.6 Phase 1 is shipped on `feat/v0.6-multi-agent-config` and reviewed. **The branch is NOT merged into `main` yet** — Phase 2 (and Phase 3) will continue on the same branch per the v0.6 spec's "one branch, three sequential milestones" policy. The eventual merge happens at the end of Phase 3 alongside the version bump. + +**Do not begin Phase 2 work** (agent profiles, `--agent` flag, AGENTS.md + CLAUDE.md shim, `ctask agents check`) until the Phase 2 plan is written and reviewed. Phase 3 (PID liveness, lazy-cleanup adoption, context-file scaffolding) does not start until Phase 2 is implemented, tested, and reviewed. ### General resume (on `main` after a ship) @@ -278,19 +515,23 @@ ctask doctor 2>&1 | grep -E "Session mode|tmux" unset CTASK_SESSION_MODE ``` -### Starting v0.6 Phase 1 +### Starting v0.6 Phase 2 -Suggested opening session prompt (paste into a fresh ctask agent session on `main`): +Branch is already `feat/v0.6-multi-agent-config`; check it out and continue. **Read the Phase 1 ship report above first** — Phase 2 builds on the resolver and the task.yaml schema fields it landed. -> Implement v0.6 Phase 1 only, per the scope listed in `notes.md` ("Next: v0.6 Phase 1"): config file parser + resolver, `schema_version` and `workspace.mode` fields in `task.yaml`, source attribution in `ctask doctor` and `ctask info`. Start with the brainstorming skill to lock the config file format (XDG path, key names, layering rules). Do not start Phase 2 work (agent profiles, AGENTS.md, PID liveness, lazy-cleanup adoption) — those are explicitly deferred. +Suggested opening session prompt (paste into a fresh ctask agent session): -Branch creation (when ready to start coding): +> Begin v0.6 Phase 2 implementation planning only. Phase 1 is complete on `feat/v0.6-multi-agent-config` (see `notes.md`). Phase 2 scope per `v0.6-spec.md` sections 5–7: agent profile system in task.yaml (`agent.type` / `agent.command` / `agent.args` / `agent.env`; built-in types `claude` and `opencode`, escape hatch `custom`), `--agent` flag on `ctask new` with the deferred `TestCLIFlagOverridesEnvVar`, AGENTS.md canonical file + CLAUDE.md shim (claude type only — opencode shim deferred until conventions are verified), and `ctask agents check` validation command. Per spec implementation notes, the AGENTS.md template + `handoff.md` + `context/notes-archive/` scaffold land together in one seed-template change (commit 7 in the spec's ordering) — Phase 3's context-file scaffolding rides along with Phase 2's seed work to avoid rewriting templates twice. Stop after Phase 2 planning; do not start Phase 3 (PID liveness). + +Resume sanity checks: ```powershell cd C:\Users\Warren\claude_tasks\ctask -git checkout main -git pull # no-op (no remote), but cheap habit -git checkout -b feat/v0.6-phase1-config-and-schema +git checkout feat/v0.6-multi-agent-config +git log --oneline main..HEAD # expect 5 commits, tip 6182d89 +just test # all 7 packages green +just build # ctask.exe builds locally +just build-linux # dist/ctask-linux-amd64 statically linked ``` ## Load-bearing design points (don't forget)