From c2293a0aebeadfc92851b77ad0e45fba902d35f2 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:01:53 -0700 Subject: [PATCH 1/8] feat(plans): add config, dir templating, and frontmatter foundation Add PlansConfig to Config with default "docs/plans", SanitizeSlug and ExpandPlansDir helpers, and the plans package with ReadFrontmatter/EnsureFrontmatter/SetStatus for atomic YAML frontmatter management in design-spec markdown files. --- internal/config/config.go | 7 ++ internal/config/template.go | 47 +++++++++++ internal/config/template_test.go | 36 ++++++++ internal/plans/frontmatter.go | 128 +++++++++++++++++++++++++++++ internal/plans/frontmatter_test.go | 37 +++++++++ 5 files changed, 255 insertions(+) create mode 100644 internal/config/template.go create mode 100644 internal/config/template_test.go create mode 100644 internal/plans/frontmatter.go create mode 100644 internal/plans/frontmatter_test.go diff --git a/internal/config/config.go b/internal/config/config.go index c42d49c..d8b3d75 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ type Config struct { Server ServerConfig `toml:"server" json:"server"` Share ShareConfig `toml:"share" json:"share"` Updates UpdatesConfig `toml:"updates" json:"updates"` + Plans PlansConfig `toml:"plans" json:"plans"` } // CLIConfig holds settings the arc CLI uses to reach the server. @@ -40,6 +41,11 @@ type UpdatesConfig struct { Channel string `toml:"channel" json:"channel"` } +// PlansConfig holds settings for design-spec plan files. +type PlansConfig struct { + Dir string `toml:"dir" json:"dir"` +} + // DefaultServerPort is the built-in default port for the arc server. const DefaultServerPort = 7432 @@ -50,6 +56,7 @@ func Default() *Config { Server: ServerConfig{Port: DefaultServerPort, DBPath: "~/.arc/data.db"}, Share: ShareConfig{Server: "https://arcplanner.sentiolabs.io"}, Updates: UpdatesConfig{Channel: "stable"}, + Plans: PlansConfig{Dir: "docs/plans"}, } } diff --git a/internal/config/template.go b/internal/config/template.go new file mode 100644 index 0000000..fef229a --- /dev/null +++ b/internal/config/template.go @@ -0,0 +1,47 @@ +package config + +import ( + "fmt" + "path/filepath" + "regexp" + "strings" +) + +var templateVarRe = regexp.MustCompile(`\{([a-zA-Z0-9_]+)\}`) +var slugStripRe = regexp.MustCompile(`[^a-z0-9]+`) + +// SanitizeSlug lowercases s, replaces runs of non [a-z0-9] with '-', trims '-'. "" if nothing survives. +func SanitizeSlug(s string) string { + s = strings.ToLower(strings.TrimSpace(s)) + s = slugStripRe.ReplaceAllString(s, "-") + return strings.Trim(s, "-") +} + +// ExpandPlansDir substitutes {vars}, expands leading ~, returns an absolute dir +// (relative resolved against cwd). Unknown {placeholder} or empty substitution => error. +func ExpandPlansDir(tmpl string, vars map[string]string, cwd string) (string, error) { + var badVar, emptyVar string + out := templateVarRe.ReplaceAllStringFunc(tmpl, func(m string) string { + name := m[1 : len(m)-1] + v, ok := vars[name] + if !ok { + badVar = name + return m + } + if v == "" { + emptyVar = name + } + return v + }) + if badVar != "" { + return "", fmt.Errorf("unknown template variable {%s} in plans.dir", badVar) + } + if emptyVar != "" { + return "", fmt.Errorf("template variable {%s} expanded to empty", emptyVar) + } + out = expandHome(out) + if !filepath.IsAbs(out) { + out = filepath.Join(cwd, out) + } + return out, nil +} diff --git a/internal/config/template_test.go b/internal/config/template_test.go new file mode 100644 index 0000000..5368a5d --- /dev/null +++ b/internal/config/template_test.go @@ -0,0 +1,36 @@ +package config + +import ( + "strings" + "testing" +) + +// --- Contract assertions --- +var _ = Config{}.Plans.Dir + +func TestSanitizeSlug(t *testing.T) { + for in, want := range map[string]string{"My App": "my-app", " Foo__Bar ": "foo-bar", "!!!": ""} { + if got := SanitizeSlug(in); got != want { + t.Fatalf("SanitizeSlug(%q)=%q want %q", in, got, want) + } + } +} + +func TestExpandPlansDir(t *testing.T) { + got, err := ExpandPlansDir("~/V/{project}", map[string]string{"project": "arc"}, "/tmp") + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(got, "/V/arc") { + t.Fatalf("got %q", got) + } + if _, err := ExpandPlansDir("~/V/{nope}", map[string]string{}, "/tmp"); err == nil { + t.Fatal("want unknown-var error") + } + if _, err := ExpandPlansDir("~/V/{project}", map[string]string{"project": ""}, "/tmp"); err == nil { + t.Fatal("want empty-var error") + } + if rel, err := ExpandPlansDir("docs/plans", map[string]string{}, "/tmp"); err != nil || rel != "/tmp/docs/plans" { + t.Fatalf("rel=%q err=%v", rel, err) + } +} diff --git a/internal/plans/frontmatter.go b/internal/plans/frontmatter.go new file mode 100644 index 0000000..b3123fe --- /dev/null +++ b/internal/plans/frontmatter.go @@ -0,0 +1,128 @@ +// Package plans handles arc-owned design-spec markdown frontmatter. +package plans + +import ( + "bytes" + "errors" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +type ArcReview struct { + Kind string `yaml:"kind"` // always "legacy" going forward + ID string `yaml:"id"` +} + +type Frontmatter struct { + Title string `yaml:"title"` + Date string `yaml:"date"` + Project string `yaml:"project"` + Status string `yaml:"status"` + Tags []string `yaml:"tags"` + ArcReview ArcReview `yaml:"arc_review"` +} + +var fmDelim = []byte("---\n") + +// ErrNoFrontmatter is returned when a file has no frontmatter or no status line. +var ErrNoFrontmatter = errors.New("no frontmatter status line") + +// ReadFrontmatter parses a leading --- block. ok=false if absent (legacy doc). +func ReadFrontmatter(b []byte) (fm Frontmatter, body []byte, ok bool, err error) { + if !bytes.HasPrefix(b, fmDelim) { + return Frontmatter{}, b, false, nil + } + rest := b[len(fmDelim):] + end := bytes.Index(rest, []byte("\n---")) + if end < 0 { + return Frontmatter{}, b, false, nil + } + if err := yaml.Unmarshal(rest[:end], &fm); err != nil { + return Frontmatter{}, b, false, err + } + after := rest[end+1:] + if i := bytes.IndexByte(after, '\n'); i >= 0 { + body = after[i+1:] + } + return fm, body, true, nil +} + +// EnsureFrontmatter idempotently writes the arc-owned frontmatter block, preserving body. Atomic. +func EnsureFrontmatter(path string, meta Frontmatter) error { + raw, err := os.ReadFile(path) + if err != nil { + return err + } + _, body, ok, err := ReadFrontmatter(raw) + if err != nil { + return err + } + if !ok { + body = raw + } + y, err := yaml.Marshal(meta) + if err != nil { + return err + } + var buf bytes.Buffer + buf.Write(fmDelim) + buf.Write(y) + buf.WriteString("---\n") + buf.Write(body) + return atomicWrite(path, buf.Bytes()) +} + +// SetStatus surgically replaces only the `status:` line in the leading frontmatter. Atomic. +// ErrNoFrontmatter (sentinel) if no frontmatter/status line — caller warns and continues. +func SetStatus(path, status string) error { + raw, err := os.ReadFile(path) + if err != nil { + return err + } + if !bytes.HasPrefix(raw, fmDelim) { + return ErrNoFrontmatter + } + lines := strings.SplitAfter(string(raw), "\n") + end := -1 + for i := 1; i < len(lines); i++ { + if strings.TrimRight(lines[i], "\n") == "---" { + end = i + break + } + } + if end < 0 { + return ErrNoFrontmatter + } + for i := 1; i < end; i++ { + if strings.HasPrefix(strings.TrimSpace(lines[i]), "status:") { + nl := "" + if strings.HasSuffix(lines[i], "\n") { + nl = "\n" + } + lines[i] = "status: " + status + nl + return atomicWrite(path, []byte(strings.Join(lines, ""))) + } + } + return ErrNoFrontmatter +} + +func atomicWrite(path string, data []byte) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".arcfm-*") + if err != nil { + return err + } + name := tmp.Name() + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(name) + return err + } + if err := tmp.Close(); err != nil { + os.Remove(name) + return err + } + return os.Rename(name, path) +} diff --git a/internal/plans/frontmatter_test.go b/internal/plans/frontmatter_test.go new file mode 100644 index 0000000..efa7a83 --- /dev/null +++ b/internal/plans/frontmatter_test.go @@ -0,0 +1,37 @@ +package plans + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// --- Contract assertions --- +var _ string = Frontmatter{}.Status +var _ ArcReview = Frontmatter{}.ArcReview + +func TestEnsureAndSetStatus(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "spec.md") + os.WriteFile(p, []byte("# Title\n\nbody\n"), 0o644) + if err := EnsureFrontmatter(p, Frontmatter{Title: "T", Date: "2026-06-07", Project: "arc", Status: "in_review", Tags: []string{"arc"}, ArcReview: ArcReview{Kind: "legacy", ID: "plan.x"}}); err != nil { + t.Fatal(err) + } + got, _ := os.ReadFile(p) + if !strings.HasPrefix(string(got), "---\n") || !strings.Contains(string(got), "# Title") { + t.Fatalf("bad: %s", got) + } + if err := SetStatus(p, "approved"); err != nil { + t.Fatal(err) + } + got2, _ := os.ReadFile(p) + if !strings.Contains(string(got2), "status: approved") { + t.Fatalf("status: %s", got2) + } + plain := filepath.Join(dir, "plain.md") + os.WriteFile(plain, []byte("no fm\n"), 0o644) + if err := SetStatus(plain, "approved"); err != ErrNoFrontmatter { + t.Fatalf("want ErrNoFrontmatter got %v", err) + } +} From 7ee2967c3a8bad29e1b052971f6dcc9fa2317754 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:14:11 -0700 Subject: [PATCH 2/8] fix(plans): harden frontmatter parsing for delimiters, EOF, and CRLF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace naive `\n---` byte search with `findClosingDelim`, which requires the closing delimiter to be exactly `---` followed by `\n`, `\r`, or EOF — preventing `----` and `--- x` lines from being mistaken for the closer. - Return `[]byte{}` (not nil) when the closing `---` has no trailing newline, so no body content is silently dropped on EOF-terminated files. - Fix `SetStatus` CRLF handling: trim `\r\n` (not just `\n`) when scanning for the closing delimiter, and preserve the original line ending (`\r\n` or `\n`) when rewriting the `status:` line. - Harden existing `os.WriteFile` setup calls in tests to check error returns. --- internal/plans/frontmatter.go | 60 ++++++++-- internal/plans/frontmatter_test.go | 171 ++++++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 10 deletions(-) diff --git a/internal/plans/frontmatter.go b/internal/plans/frontmatter.go index b3123fe..8c630f8 100644 --- a/internal/plans/frontmatter.go +++ b/internal/plans/frontmatter.go @@ -30,22 +30,62 @@ var fmDelim = []byte("---\n") // ErrNoFrontmatter is returned when a file has no frontmatter or no status line. var ErrNoFrontmatter = errors.New("no frontmatter status line") +// findClosingDelim locates the first occurrence of a line that is exactly "---" +// (the closing frontmatter delimiter) within b, starting the search after the +// opening "---\n" has already been consumed. +// +// Returns the byte offset of the '\n' that precedes "---", or -1 if not found. +// Handles two forms of the closing delimiter: +// - "\n---\n" — standard case (newline after ---) +// - "\n---" — EOF case (--- is the very last bytes with no trailing newline) +// +// Lines that start with "---" but have additional characters (e.g. "----", "--- x") +// are NOT treated as closing delimiters. +func findClosingDelim(b []byte) int { + search := b + offset := 0 + for { + idx := bytes.Index(search, []byte("\n---")) + if idx < 0 { + return -1 + } + // Position of the character immediately following "---". + after := idx + 4 + if after == len(search) { + // "---" is at the very end of b with no following character — valid EOF closer. + return offset + idx + } + if search[after] == '\n' || search[after] == '\r' { + // Followed by newline (LF or CR) — exact closer found. + return offset + idx + } + // Not an exact "---" line; skip past this match and keep searching. + advance := idx + 4 + offset += advance + search = search[advance:] + } +} + // ReadFrontmatter parses a leading --- block. ok=false if absent (legacy doc). func ReadFrontmatter(b []byte) (fm Frontmatter, body []byte, ok bool, err error) { if !bytes.HasPrefix(b, fmDelim) { return Frontmatter{}, b, false, nil } rest := b[len(fmDelim):] - end := bytes.Index(rest, []byte("\n---")) + end := findClosingDelim(rest) if end < 0 { return Frontmatter{}, b, false, nil } if err := yaml.Unmarshal(rest[:end], &fm); err != nil { return Frontmatter{}, b, false, err } - after := rest[end+1:] + // Skip past the closing "---" line (including its trailing newline, if present). + after := rest[end+1:] // skip the leading '\n', now points at "---..." if i := bytes.IndexByte(after, '\n'); i >= 0 { body = after[i+1:] + } else { + // "---" at EOF with no trailing newline — body is empty (not nil). + body = []byte{} } return fm, body, true, nil } @@ -82,13 +122,14 @@ func SetStatus(path, status string) error { if err != nil { return err } - if !bytes.HasPrefix(raw, fmDelim) { + if !bytes.HasPrefix(raw, fmDelim) && !bytes.HasPrefix(raw, []byte("---\r\n")) { return ErrNoFrontmatter } lines := strings.SplitAfter(string(raw), "\n") end := -1 for i := 1; i < len(lines); i++ { - if strings.TrimRight(lines[i], "\n") == "---" { + // Trim both \r and \n to handle CRLF (\r\n) and LF (\n) line endings. + if strings.TrimRight(lines[i], "\r\n") == "---" { end = i break } @@ -98,11 +139,14 @@ func SetStatus(path, status string) error { } for i := 1; i < end; i++ { if strings.HasPrefix(strings.TrimSpace(lines[i]), "status:") { - nl := "" - if strings.HasSuffix(lines[i], "\n") { - nl = "\n" + // Preserve the original line ending (CRLF or LF). + le := "" + if strings.HasSuffix(lines[i], "\r\n") { + le = "\r\n" + } else if strings.HasSuffix(lines[i], "\n") { + le = "\n" } - lines[i] = "status: " + status + nl + lines[i] = "status: " + status + le return atomicWrite(path, []byte(strings.Join(lines, ""))) } } diff --git a/internal/plans/frontmatter_test.go b/internal/plans/frontmatter_test.go index efa7a83..d63516e 100644 --- a/internal/plans/frontmatter_test.go +++ b/internal/plans/frontmatter_test.go @@ -14,7 +14,9 @@ var _ ArcReview = Frontmatter{}.ArcReview func TestEnsureAndSetStatus(t *testing.T) { dir := t.TempDir() p := filepath.Join(dir, "spec.md") - os.WriteFile(p, []byte("# Title\n\nbody\n"), 0o644) + if err := os.WriteFile(p, []byte("# Title\n\nbody\n"), 0o644); err != nil { + t.Fatal(err) + } if err := EnsureFrontmatter(p, Frontmatter{Title: "T", Date: "2026-06-07", Project: "arc", Status: "in_review", Tags: []string{"arc"}, ArcReview: ArcReview{Kind: "legacy", ID: "plan.x"}}); err != nil { t.Fatal(err) } @@ -30,8 +32,173 @@ func TestEnsureAndSetStatus(t *testing.T) { t.Fatalf("status: %s", got2) } plain := filepath.Join(dir, "plain.md") - os.WriteFile(plain, []byte("no fm\n"), 0o644) + if err := os.WriteFile(plain, []byte("no fm\n"), 0o644); err != nil { + t.Fatal(err) + } if err := SetStatus(plain, "approved"); err != ErrNoFrontmatter { t.Fatalf("want ErrNoFrontmatter got %v", err) } } + +// TestReadFrontmatterBodyWithDashes verifies that a line starting with "---" inside +// the body (e.g. a markdown horizontal rule or "----") does not close the frontmatter +// block — only an exact "---" line (with no other characters) acts as the closer. +func TestReadFrontmatterBodyWithDashes(t *testing.T) { + // Body contains a line "----" (four dashes) and a line "--- x" — neither should + // be treated as the closing delimiter. Note: the body here is AFTER the real + // closing "---", so the current search order still works fine. + t.Run("dashes_in_body_after_closer", func(t *testing.T) { + input := "---\ntitle: Test\nstatus: draft\n---\n# Heading\n\n----\n\n--- x\n\nend\n" + fm, body, ok, err := ReadFrontmatter([]byte(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatal("expected frontmatter to be found") + } + if fm.Title != "Test" { + t.Fatalf("unexpected title: %q", fm.Title) + } + if !strings.Contains(string(body), "----") { + t.Errorf("body should contain '----' (markdown hr); got: %q", body) + } + if !strings.Contains(string(body), "--- x") { + t.Errorf("body should contain '--- x'; got: %q", body) + } + if !strings.Contains(string(body), "end") { + t.Errorf("body should contain 'end'; got: %q", body) + } + + // EnsureFrontmatter round-trip should preserve that body content. + dir := t.TempDir() + p := filepath.Join(dir, "spec.md") + if err := os.WriteFile(p, []byte(input), 0o644); err != nil { + t.Fatal(err) + } + if err := EnsureFrontmatter(p, Frontmatter{Title: "Test", Status: "draft"}); err != nil { + t.Fatal(err) + } + out, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "----") { + t.Errorf("round-trip body missing '----'; got: %q", out) + } + if !strings.Contains(string(out), "--- x") { + t.Errorf("round-trip body missing '--- x'; got: %q", out) + } + }) + + // This sub-test verifies that the body can itself contain "----" (four-dash hr) + // followed by "--- x" without the parser being confused — the only valid closer + // is exactly "---" (optionally followed by CRLF/LF or EOF). + // The key assertion: the body returned must NOT start with "---\n", which would + // indicate the parser mistook "----" in the body as the closing delimiter. + t.Run("dashes_before_exact_closer_in_body", func(t *testing.T) { + // Structure: opening --- | YAML | closing --- | body with ---- and --- x + // With the naive "\n---" search, the first "\n---" found in the body + // after the real closer would be "--- x" — but since we already have + // the real closer, this only matters if "----" appeared BEFORE it. + // Here we test that "----\n" and "--- x\n" in the body do not + // pollute the frontmatter block and that body is returned intact. + input := "---\ntitle: Tricky\nstatus: draft\n---\n----\n--- x\nbody after\n" + fm, body, ok, err := ReadFrontmatter([]byte(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatal("expected frontmatter to be found") + } + if fm.Title != "Tricky" { + t.Fatalf("unexpected title: %q", fm.Title) + } + // Body must contain all lines after the closing ---. + if !strings.Contains(string(body), "----") { + t.Errorf("body should contain '----'; got: %q", body) + } + if !strings.Contains(string(body), "--- x") { + t.Errorf("body should contain '--- x'; got: %q", body) + } + if !strings.Contains(string(body), "body after") { + t.Errorf("body should contain 'body after'; got: %q", body) + } + // Body must start at "----", NOT at "---\n" (which would mean the closer + // was missed and body contains the closing delimiter line). + if strings.HasPrefix(string(body), "---\n") { + t.Errorf("body starts with '---\\n' suggesting the real closer was not recognized; got: %q", body) + } + }) +} + +// TestReadFrontmatterNoTrailingNewlineAfterClose verifies that a file ending +// exactly with "---" (no trailing newline) is parsed correctly and body is preserved. +func TestReadFrontmatterNoTrailingNewlineAfterClose(t *testing.T) { + // No newline after closing --- + input := "---\ntitle: Test\nstatus: draft\n---" + fm, body, ok, err := ReadFrontmatter([]byte(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatal("expected frontmatter to be found even without trailing newline after closing ---") + } + if fm.Title != "Test" { + t.Fatalf("unexpected title: %q", fm.Title) + } + // Body should be empty (not nil drop), and no panic. + _ = body + + // File ending with "---\nbody content" (body present, no newline after close marker) + // This tests the case: closing --- has no trailing newline but content existed before it + // was rewritten. Use EnsureFrontmatter round-trip. + dir := t.TempDir() + p := filepath.Join(dir, "spec.md") + // Write file with trailing body but no final newline after --- + if err := os.WriteFile(p, []byte("---\ntitle: NoNL\nstatus: draft\n---\nbody here"), 0o644); err != nil { + t.Fatal(err) + } + if err := EnsureFrontmatter(p, Frontmatter{Title: "NoNL", Status: "draft"}); err != nil { + t.Fatal(err) + } + out, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "body here") { + t.Errorf("body content was dropped; got: %q", out) + } +} + +// TestSetStatusCRLF verifies that SetStatus works correctly on a CRLF-encoded file: +// it must locate the closing "---" delimiter (which appears as "---\r\n"), rewrite +// the status line preserving CRLF, and NOT return ErrNoFrontmatter. +func TestSetStatusCRLF(t *testing.T) { + // Build a CRLF frontmatter file. + crlf := "---\r\ntitle: CRLFTest\r\nstatus: draft\r\n---\r\n# Body\r\n" + dir := t.TempDir() + p := filepath.Join(dir, "crlf.md") + if err := os.WriteFile(p, []byte(crlf), 0o644); err != nil { + t.Fatal(err) + } + + if err := SetStatus(p, "approved"); err != nil { + t.Fatalf("SetStatus on CRLF file returned error: %v", err) + } + + out, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + outStr := string(out) + + // The status line must be updated. + if !strings.Contains(outStr, "status: approved") { + t.Errorf("status not updated in CRLF file; got: %q", outStr) + } + + // The rewritten status line must preserve CRLF. + if !strings.Contains(outStr, "status: approved\r\n") { + t.Errorf("CRLF not preserved on status line; got: %q", outStr) + } +} From c2a030148d98b0bdc6496cd96839e7ab4c926ff0 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:19:10 -0700 Subject: [PATCH 3/8] feat(config): validate plans.dir and add 'config get --resolved' Adds plans.dir validation (empty, .., unknown template vars) to Validate(), wires plans.dir into config get/set, and adds --resolved flag to config get for expanding {project}/{prefix} vars to an absolute path. --- cmd/arc/config.go | 37 ++++++++++++++++++++++++++++++++ internal/config/validate.go | 12 +++++++++++ internal/config/validate_test.go | 20 +++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/cmd/arc/config.go b/cmd/arc/config.go index 4102f1c..b6dd00c 100644 --- a/cmd/arc/config.go +++ b/cmd/arc/config.go @@ -35,6 +35,7 @@ var legacyAliases = map[string]string{ // recognizedKeys is the canonical list of all valid config key names. var recognizedKeys = []string{ "cli.server", + "plans.dir", "server.port", "server.db_path", "share.author", @@ -101,10 +102,14 @@ var configEditCmd = &cobra.Command{ RunE: runConfigEdit, } +// resolvedFlag enables path resolution for keys that support it (plans.dir). +var resolvedFlag bool + // init registers all config sub-commands with the root command. func init() { configCmd.AddCommand(configListCmd, configGetCmd, configSetCmd, configUnsetCmd, configPathCmd, configEditCmd) rootCmd.AddCommand(configCmd) + configGetCmd.Flags().BoolVar(&resolvedFlag, "resolved", false, "resolve {vars} and ~ to an absolute path (plans.dir only)") } // runConfigList prints all settings grouped by TOML section. @@ -161,6 +166,34 @@ func runConfigGet(cmd *cobra.Command, args []string) error { if err != nil { return err } + if resolvedFlag && key == "plans.dir" { + c, err := getClient() + if err != nil { + return err + } + wsID, _, _, err := resolveProject() + if err != nil { + return err + } + proj, err := c.GetProject(wsID) + if err != nil { + return err + } + cwd, _ := os.Getwd() + dir, err := cfgpkg.ExpandPlansDir(cfg.Plans.Dir, map[string]string{ + "project": cfgpkg.SanitizeSlug(proj.Name), + "prefix": proj.Prefix, + }, cwd) + if err != nil { + return err + } + if outputJSON { + outputResult(map[string]string{"plans.dir": dir}) + return nil + } + fmt.Println(dir) + return nil + } value := getKey(cfg, key) if outputJSON { outputResult(map[string]string{key: value}) @@ -320,6 +353,8 @@ func getKey(cfg *cfgpkg.Config, key string) string { switch key { case "cli.server": return cfg.CLI.Server + case "plans.dir": + return cfg.Plans.Dir case "server.port": return strconv.Itoa(cfg.Server.Port) case "server.db_path": @@ -340,6 +375,8 @@ func setKey(cfg *cfgpkg.Config, key, value string) error { switch key { case "cli.server": cfg.CLI.Server = value + case "plans.dir": + cfg.Plans.Dir = value case "server.port": n, err := strconv.Atoi(value) if err != nil { diff --git a/internal/config/validate.go b/internal/config/validate.go index 9a9e0fd..93ec690 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -3,6 +3,7 @@ package config import ( "fmt" "net/url" + "regexp" "sort" "strings" ) @@ -51,6 +52,17 @@ func Validate(cfg *Config) error { if !channelOK { errs["updates.channel"] = "must be one of: " + strings.Join(ValidChannels, ", ") } + if cfg.Plans.Dir == "" { + errs["plans.dir"] = "must not be empty" + } else if strings.Contains(cfg.Plans.Dir, "..") { + errs["plans.dir"] = "must not contain '..'" + } else { + for _, m := range regexp.MustCompile(`\{([a-zA-Z0-9_]+)\}`).FindAllStringSubmatch(cfg.Plans.Dir, -1) { + if m[1] != "project" && m[1] != "prefix" { + errs["plans.dir"] = "unknown template variable {" + m[1] + "} (allowed: project, prefix)" + } + } + } if len(errs) == 0 { return nil } diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index 757dafd..80a7ee1 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -77,3 +77,23 @@ func TestValidateRejectsEmptyShareServer(t *testing.T) { t.Errorf("missing share.server in errors: %v", ve) } } + +func TestValidatePlansDir(t *testing.T) { + base := config.Default() + base.Plans.Dir = "" + if config.Validate(base) == nil { + t.Fatal("empty dir should fail") + } + base.Plans.Dir = "../x" + if config.Validate(base) == nil { + t.Fatal(".. should fail") + } + base.Plans.Dir = "~/V/{nope}" + if config.Validate(base) == nil { + t.Fatal("unknown var should fail") + } + base.Plans.Dir = "~/V/{project}" + if err := config.Validate(base); err != nil { + t.Fatalf("valid dir should pass: %v", err) + } +} From fe5cbd844a38b1d06b86eb73515aed66c42c5d13 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:26:12 -0700 Subject: [PATCH 4/8] refactor(config): reuse templateVarRe, harden getwd + --resolved guard - Replace inline regexp.MustCompile in Validate() with the package-level templateVarRe from template.go; remove now-unused "regexp" import. - Break on first unknown template variable so the first offender is reported rather than the last (last-write-wins bug). - Check os.Getwd() error in --resolved handler and return a wrapped error. - Return an explicit error when --resolved is used with a key other than plans.dir instead of silently ignoring the flag. --- cmd/arc/config.go | 8 +++++++- internal/config/validate.go | 4 ++-- internal/config/validate_test.go | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cmd/arc/config.go b/cmd/arc/config.go index b6dd00c..ce753fa 100644 --- a/cmd/arc/config.go +++ b/cmd/arc/config.go @@ -166,6 +166,9 @@ func runConfigGet(cmd *cobra.Command, args []string) error { if err != nil { return err } + if resolvedFlag && key != "plans.dir" { + return fmt.Errorf("--resolved is only supported for plans.dir") + } if resolvedFlag && key == "plans.dir" { c, err := getClient() if err != nil { @@ -179,7 +182,10 @@ func runConfigGet(cmd *cobra.Command, args []string) error { if err != nil { return err } - cwd, _ := os.Getwd() + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("get current directory: %w", err) + } dir, err := cfgpkg.ExpandPlansDir(cfg.Plans.Dir, map[string]string{ "project": cfgpkg.SanitizeSlug(proj.Name), "prefix": proj.Prefix, diff --git a/internal/config/validate.go b/internal/config/validate.go index 93ec690..0c9f878 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -3,7 +3,6 @@ package config import ( "fmt" "net/url" - "regexp" "sort" "strings" ) @@ -57,9 +56,10 @@ func Validate(cfg *Config) error { } else if strings.Contains(cfg.Plans.Dir, "..") { errs["plans.dir"] = "must not contain '..'" } else { - for _, m := range regexp.MustCompile(`\{([a-zA-Z0-9_]+)\}`).FindAllStringSubmatch(cfg.Plans.Dir, -1) { + for _, m := range templateVarRe.FindAllStringSubmatch(cfg.Plans.Dir, -1) { if m[1] != "project" && m[1] != "prefix" { errs["plans.dir"] = "unknown template variable {" + m[1] + "} (allowed: project, prefix)" + break } } } diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index 80a7ee1..2a1a54c 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -2,6 +2,7 @@ package config_test import ( "errors" + "strings" "testing" "github.com/sentiolabs/arc/internal/config" @@ -97,3 +98,24 @@ func TestValidatePlansDir(t *testing.T) { t.Fatalf("valid dir should pass: %v", err) } } + +func TestValidatePlansDirFirstUnknownVarReported(t *testing.T) { + cfg := config.Default() + // Template with two unknown vars; the FIRST one ({foo}) should be reported. + cfg.Plans.Dir = "~/{foo}/{bar}" + err := config.Validate(cfg) + var ve config.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err type = %T, want ValidationError", err) + } + msg, ok := ve["plans.dir"] + if !ok { + t.Fatalf("missing plans.dir in errors: %v", ve) + } + if !strings.Contains(msg, "foo") { + t.Errorf("expected error to mention first unknown var 'foo', got: %s", msg) + } + if strings.Contains(msg, "bar") { + t.Errorf("error should NOT mention second var 'bar' (first-only reporting), got: %s", msg) + } +} From 09c335ab524bc351c97fea3059bbdc01e9654dea Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:29:59 -0700 Subject: [PATCH 5/8] feat(plan): write spec frontmatter on create, sync status on approve On `arc plan create`, injects YAML frontmatter (title/date/project/status/tags/arc_review) into the plan file via plans.EnsureFrontmatter unless --no-frontmatter is passed. Derives title from first H1 heading or filename fallback. On `arc plan approve`, best-effort syncs the frontmatter status field to "approved" via plans.SetStatus. --- cmd/arc/plan.go | 70 ++++++++++++++++++++++++++++++++++++++++++++ cmd/arc/plan_test.go | 38 ++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 cmd/arc/plan_test.go diff --git a/cmd/arc/plan.go b/cmd/arc/plan.go index f37c90d..c6868c5 100644 --- a/cmd/arc/plan.go +++ b/cmd/arc/plan.go @@ -9,12 +9,49 @@ package main import ( + "bufio" "fmt" + "os" "path/filepath" + "regexp" + "strings" + "time" + "github.com/sentiolabs/arc/internal/plans" "github.com/spf13/cobra" ) +// Package-level flags for planCreateCmd. +var ( + titleFlag string // --title: override the derived plan title in frontmatter + noFrontmatter bool // --no-frontmatter: skip writing frontmatter on create +) + +// datePrefixRe matches a leading YYYY-MM-DD- date prefix on filenames. +var datePrefixRe = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}-`) + +// deriveTitle returns the title for a plan file. It reads the file and returns +// the text of the first `# ` heading line. If no heading is found, it falls +// back to the filename base with any leading YYYY-MM-DD- prefix and trailing +// .md extension removed. +func deriveTitle(path string) string { + f, err := os.Open(path) + if err == nil { + defer f.Close() + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "# ") { + return strings.TrimSpace(strings.TrimPrefix(line, "# ")) + } + } + } + base := filepath.Base(path) + base = strings.TrimSuffix(base, ".md") + base = datePrefixRe.ReplaceAllString(base, "") + return base +} + // planCmd is the parent command for plan management. var planCmd = &cobra.Command{ Use: "plan", @@ -37,6 +74,9 @@ func init() { planCmd.AddCommand(planApproveCmd) planCmd.AddCommand(planRejectCmd) planCmd.AddCommand(planCommentsCmd) + + planCreateCmd.Flags().StringVar(&titleFlag, "title", "", "Override the plan title written to frontmatter") + planCreateCmd.Flags().BoolVar(&noFrontmatter, "no-frontmatter", false, "Skip writing frontmatter into the plan file") } // planCreateCmd registers a new plan from a file path. @@ -60,6 +100,30 @@ var planCreateCmd = &cobra.Command{ return err } + if !noFrontmatter { + title := titleFlag + if title == "" { + title = deriveTitle(filePath) + } + projName := "" + if wsID, _, _, e := resolveProject(); e == nil { + if pr, e2 := c.GetProject(wsID); e2 == nil { + projName = pr.Name + } + } + meta := plans.Frontmatter{ + Title: title, + Date: time.Now().Format("2006-01-02"), + Project: projName, + Status: "in_review", + Tags: []string{"arc", "design-spec"}, + ArcReview: plans.ArcReview{Kind: "legacy", ID: plan.ID}, + } + if e := plans.EnsureFrontmatter(filePath, meta); e != nil { + fmt.Fprintf(os.Stderr, "warning: could not write frontmatter: %v\n", e) + } + } + if outputJSON { outputResult(plan) return nil @@ -121,6 +185,12 @@ var planApproveCmd = &cobra.Command{ return err } + if p, e := c.GetPlan(planID); e == nil && p.FilePath != "" { + if e2 := plans.SetStatus(p.FilePath, "approved"); e2 != nil && e2 != plans.ErrNoFrontmatter { + fmt.Fprintf(os.Stderr, "warning: could not sync status in %s: %v\n", p.FilePath, e2) + } + } + _, _ = fmt.Printf("Plan %s approved\n", planID) return nil }, diff --git a/cmd/arc/plan_test.go b/cmd/arc/plan_test.go new file mode 100644 index 0000000..3d70e1c --- /dev/null +++ b/cmd/arc/plan_test.go @@ -0,0 +1,38 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDeriveTitle_H1Heading(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "my-spec.md") + content := "# My Spec Title\n\nSome body text here.\n" + if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + t.Fatalf("write temp file: %v", err) + } + + got := deriveTitle(f) + want := "My Spec Title" + if got != want { + t.Errorf("deriveTitle H1 case: got %q, want %q", got, want) + } +} + +func TestDeriveTitle_FilenameFallback(t *testing.T) { + dir := t.TempDir() + // File with a YYYY-MM-DD- date prefix, no H1 heading + f := filepath.Join(dir, "2024-01-15-my-design-spec.md") + content := "Some content without a heading.\n" + if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + t.Fatalf("write temp file: %v", err) + } + + got := deriveTitle(f) + want := "my-design-spec" + if got != want { + t.Errorf("deriveTitle filename fallback: got %q, want %q", got, want) + } +} From bb0d9ce8f69079561a2591c0c7d8e319525254ee Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 19:36:09 -0700 Subject: [PATCH 6/8] refactor(plan): warn on read errors, match stderr idiom, expand deriveTitle tests - Add scanner.Err() check in deriveTitle with a stderr warning on mid-read I/O error - Convert all fmt.Fprintf(os.Stderr) calls to the _, _ = idiom used across the codebase - Replace inline var block for titleFlag/noFrontmatter with per-var doc-comment style matching paths.go - Clarify deriveTitle doc comment: matches exactly `# ` (## lines intentionally excluded) - Add three new edge-case tests: non-existent path, empty file, H2-only heading --- cmd/arc/plan.go | 24 ++++++++++++++---------- cmd/arc/plan_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/cmd/arc/plan.go b/cmd/arc/plan.go index c6868c5..a1837c6 100644 --- a/cmd/arc/plan.go +++ b/cmd/arc/plan.go @@ -21,19 +21,20 @@ import ( "github.com/spf13/cobra" ) -// Package-level flags for planCreateCmd. -var ( - titleFlag string // --title: override the derived plan title in frontmatter - noFrontmatter bool // --no-frontmatter: skip writing frontmatter on create -) +// titleFlag is the --title flag for planCreateCmd, overriding the derived plan title. +var titleFlag string + +// noFrontmatter is the --no-frontmatter flag for planCreateCmd, skipping frontmatter on create. +var noFrontmatter bool // datePrefixRe matches a leading YYYY-MM-DD- date prefix on filenames. var datePrefixRe = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}-`) // deriveTitle returns the title for a plan file. It reads the file and returns -// the text of the first `# ` heading line. If no heading is found, it falls -// back to the filename base with any leading YYYY-MM-DD- prefix and trailing -// .md extension removed. +// the text of the first line beginning with exactly `# ` (single `#` + space; +// `##` lines are intentionally excluded). If no matching heading is found, it +// falls back to the filename base with any leading YYYY-MM-DD- prefix and +// trailing .md extension removed. func deriveTitle(path string) string { f, err := os.Open(path) if err == nil { @@ -45,6 +46,9 @@ func deriveTitle(path string) string { return strings.TrimSpace(strings.TrimPrefix(line, "# ")) } } + if err := scanner.Err(); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "warning: could not read %s for title: %v\n", path, err) + } } base := filepath.Base(path) base = strings.TrimSuffix(base, ".md") @@ -120,7 +124,7 @@ var planCreateCmd = &cobra.Command{ ArcReview: plans.ArcReview{Kind: "legacy", ID: plan.ID}, } if e := plans.EnsureFrontmatter(filePath, meta); e != nil { - fmt.Fprintf(os.Stderr, "warning: could not write frontmatter: %v\n", e) + _, _ = fmt.Fprintf(os.Stderr, "warning: could not write frontmatter: %v\n", e) } } @@ -187,7 +191,7 @@ var planApproveCmd = &cobra.Command{ if p, e := c.GetPlan(planID); e == nil && p.FilePath != "" { if e2 := plans.SetStatus(p.FilePath, "approved"); e2 != nil && e2 != plans.ErrNoFrontmatter { - fmt.Fprintf(os.Stderr, "warning: could not sync status in %s: %v\n", p.FilePath, e2) + _, _ = fmt.Fprintf(os.Stderr, "warning: could not sync status in %s: %v\n", p.FilePath, e2) } } diff --git a/cmd/arc/plan_test.go b/cmd/arc/plan_test.go index 3d70e1c..c6a8eb4 100644 --- a/cmd/arc/plan_test.go +++ b/cmd/arc/plan_test.go @@ -36,3 +36,46 @@ func TestDeriveTitle_FilenameFallback(t *testing.T) { t.Errorf("deriveTitle filename fallback: got %q, want %q", got, want) } } + +func TestDeriveTitle_NonExistentPath(t *testing.T) { + dir := t.TempDir() + // Path that does not exist — should fall back to filename base (sans date prefix / .md) + f := filepath.Join(dir, "2024-03-01-missing-plan.md") + + got := deriveTitle(f) + want := "missing-plan" + if got != want { + t.Errorf("deriveTitle non-existent path: got %q, want %q", got, want) + } +} + +func TestDeriveTitle_EmptyFile(t *testing.T) { + dir := t.TempDir() + // Empty file — no heading, should fall back to filename base + f := filepath.Join(dir, "2024-05-10-empty-spec.md") + if err := os.WriteFile(f, []byte(""), 0o644); err != nil { + t.Fatalf("write temp file: %v", err) + } + + got := deriveTitle(f) + want := "empty-spec" + if got != want { + t.Errorf("deriveTitle empty file: got %q, want %q", got, want) + } +} + +func TestDeriveTitle_H2OnlyHeading(t *testing.T) { + dir := t.TempDir() + // File whose only heading is ## (H2) — should NOT match, fall back to filename + f := filepath.Join(dir, "2024-06-01-h2-only.md") + content := "## Not An H1\n\nBody text.\n" + if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + t.Fatalf("write temp file: %v", err) + } + + got := deriveTitle(f) + want := "h2-only" + if got != want { + t.Errorf("deriveTitle H2-only heading: got %q, want %q", got, want) + } +} From 971802529f855daf288feedd7a9149cb650607a7 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 21:51:00 -0700 Subject: [PATCH 7/8] ci: bump golangci-lint to v2.12.2 Match local linter version. Add tagliatelle yaml:snake (Obsidian-queryable frontmatter tags) and relax goconst on _test.go files (mirrors the existing dupl test exclusion). --- .github/workflows/test.yml | 2 +- .golangci.yaml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e4c09c3..cb5310d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,7 +30,7 @@ jobs: - name: Run linter uses: golangci/golangci-lint-action@v7 with: - version: v2.11.2 + version: v2.12.2 integration-tests: needs: unit-tests diff --git a/.golangci.yaml b/.golangci.yaml index 272385b..86ca5c5 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -141,6 +141,7 @@ linters: case: rules: json: snake # Enforce snake_case for JSON tags + yaml: snake # Enforce snake_case for YAML tags (e.g. Obsidian-queryable frontmatter) # Exclusion rules for linters # Controls which files/patterns are ignored by linters @@ -170,6 +171,9 @@ linters: - linters: - dupl # Duplicate code detection path: _test\.go # Test files often have similar structures for readability + - linters: + - goconst # Repeated string literals in tests aid readability over shared constants + path: _test\.go - linters: - testifylint # Testify linter path: _test\.go From 6f93031d1850fb95ab93427d0665d84c2bba2719 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Sun, 7 Jun 2026 21:51:00 -0700 Subject: [PATCH 8/8] style(lint): satisfy golangci-lint v2.12.2 across the tree Behavior-preserving cleanup: extract repeated string literals into named constants (config keys, command verbs, statuses via types.Status*), discard unhandled best-effort errors, replace magic numbers, switch over if/else chains, move whitebox tests to _test packages, errors.Is for sentinels, 0o600 test file perms, gofmt/gofumpt formatting, and doc-comment density. --- arc-paste/main.go | 5 +- cmd/arc/ai.go | 8 +- cmd/arc/config.go | 158 ++++++++++++--------- cmd/arc/config_test.go | 8 +- cmd/arc/init.go | 9 +- cmd/arc/label.go | 2 +- cmd/arc/main.go | 25 +++- cmd/arc/onboard.go | 2 +- cmd/arc/paths.go | 2 +- cmd/arc/plan.go | 16 ++- cmd/arc/plan_test.go | 8 +- cmd/arc/server.go | 31 +++-- cmd/arc/share.go | 27 +++- cmd/arc/team.go | 4 +- internal/api/issues.go | 8 +- internal/config/template.go | 6 + internal/config/template_test.go | 19 +-- internal/config/validate.go | 21 ++- internal/plans/frontmatter.go | 18 +-- internal/plans/frontmatter_test.go | 212 +++++++++++++++-------------- internal/project/naming.go | 4 +- 21 files changed, 345 insertions(+), 248 deletions(-) diff --git a/arc-paste/main.go b/arc-paste/main.go index 32387dd..f379ef2 100644 --- a/arc-paste/main.go +++ b/arc-paste/main.go @@ -27,6 +27,9 @@ import ( // SQLite controls) is what protects the data. const dbDirMode = 0o755 +// robotsPath is the well-known URL path for the robots.txt file. +const robotsPath = "/robots.txt" + func main() { if err := run(); err != nil { log.Fatal(err) @@ -110,7 +113,7 @@ func newRouter(handlers *paste.Handlers) *echo.Echo { // Keep this function in sync with arc-paste/Caddyfile if either changes. func arcPasteAllowedPath(p string) bool { switch p { - case "/api/paste", "/robots.txt": + case "/api/paste", robotsPath: return true } if strings.HasPrefix(p, "/_app/") || strings.HasPrefix(p, "/api/paste/") { diff --git a/cmd/arc/ai.go b/cmd/arc/ai.go index e304551..7c10c7c 100644 --- a/cmd/arc/ai.go +++ b/cmd/arc/ai.go @@ -76,7 +76,7 @@ var aiSessionCmd = &cobra.Command{ var errSkipSession = errors.New("skip session") var aiSessionStartCmd = &cobra.Command{ - Use: "start", + Use: cmdStart, Short: "Start a new AI session", RunE: func(cmd *cobra.Command, args []string) error { useStdin, _ := cmd.Flags().GetBool("stdin") @@ -170,7 +170,7 @@ func runSessionStart(cmd *cobra.Command, useStdin bool) error { // aiSessionListCmd lists AI sessions sorted by start time (newest first). // Supports --json for machine-readable output. var aiSessionListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List AI sessions", RunE: func(cmd *cobra.Command, args []string) error { projID, err := getProjectID() @@ -215,7 +215,7 @@ var aiSessionListCmd = &cobra.Command{ // aiSessionShowCmd displays details for a single AI session, including // its registered agents. Supports --json for machine-readable output. var aiSessionShowCmd = &cobra.Command{ - Use: "show ", + Use: useShowID, Short: "Show AI session details", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -344,7 +344,7 @@ var aiAgentRegisterCmd = &cobra.Command{ // type, model, status, duration, tokens, and tool use count. // Requires --session to identify the parent session. var aiAgentShowCmd = &cobra.Command{ - Use: "show ", + Use: useShowID, Short: "Show AI agent details", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/arc/config.go b/cmd/arc/config.go index ce753fa..0a06f28 100644 --- a/cmd/arc/config.go +++ b/cmd/arc/config.go @@ -22,25 +22,43 @@ const setArgsCount = 2 // dottedKeyParts is the number of parts produced by splitting a dotted config key. const dottedKeyParts = 2 +// Config key names, used as the canonical dotted identifiers throughout the +// config commands. +const ( + cliServerKey = "cli.server" + shareAuthorKey = "share.author" + shareServerKey = "share.server" + updatesChannelKey = "updates.channel" + plansDirKey = "plans.dir" + serverPortKey = "server.port" + serverDBPathKey = "server.db_path" +) + +// projectVar is the template variable name for a project's slug in plans.dir. +const projectVar = "project" + +// cmdEdit is the cobra Use string for the "config edit" sub-command. +const cmdEdit = "edit" + // legacyAliases maps pre-TOML key names to their current dotted equivalents. // These are checked before the Levenshtein fallback in normalizeKey so that // well-known old names always produce the correct "did you mean" hint. var legacyAliases = map[string]string{ - "server_url": "cli.server", - "share_author": "share.author", - "share_server": "share.server", - "channel": "updates.channel", + "server_url": cliServerKey, + "share_author": shareAuthorKey, + "share_server": shareServerKey, + "channel": updatesChannelKey, } // recognizedKeys is the canonical list of all valid config key names. var recognizedKeys = []string{ - "cli.server", - "plans.dir", - "server.port", - "server.db_path", - "share.author", - "share.server", - "updates.channel", + cliServerKey, + plansDirKey, + serverPortKey, + serverDBPathKey, + shareAuthorKey, + shareServerKey, + updatesChannelKey, } // configCmd is the parent command for all config sub-commands. @@ -52,7 +70,7 @@ var configCmd = &cobra.Command{ // configListCmd prints all configuration key=value pairs. var configListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List all configuration values", RunE: runConfigList, } @@ -97,7 +115,7 @@ var configPathCmd = &cobra.Command{ // configEditCmd opens the config file in $EDITOR for direct editing. var configEditCmd = &cobra.Command{ - Use: "edit", + Use: cmdEdit, Short: "Open the config file in $EDITOR", RunE: runConfigEdit, } @@ -109,7 +127,8 @@ var resolvedFlag bool func init() { configCmd.AddCommand(configListCmd, configGetCmd, configSetCmd, configUnsetCmd, configPathCmd, configEditCmd) rootCmd.AddCommand(configCmd) - configGetCmd.Flags().BoolVar(&resolvedFlag, "resolved", false, "resolve {vars} and ~ to an absolute path (plans.dir only)") + configGetCmd.Flags().BoolVar(&resolvedFlag, "resolved", false, + "resolve {vars} and ~ to an absolute path (plans.dir only)") } // runConfigList prints all settings grouped by TOML section. @@ -139,18 +158,18 @@ func runConfigList(cmd *cobra.Command, args []string) error { fmt.Printf(" %-10s = %s%s\n", label, value, tag) } fmt.Println("[cli]") - printRow("cli.server", cfg.CLI.Server) + printRow(cliServerKey, cfg.CLI.Server) fmt.Println() fmt.Println("[server]") - printRow("server.port", strconv.Itoa(cfg.Server.Port)) - printRow("server.db_path", cfg.Server.DBPath) + printRow(serverPortKey, strconv.Itoa(cfg.Server.Port)) + printRow(serverDBPathKey, cfg.Server.DBPath) fmt.Println() fmt.Println("[share]") - printRow("share.author", cfg.Share.Author) - printRow("share.server", cfg.Share.Server) + printRow(shareAuthorKey, cfg.Share.Author) + printRow(shareServerKey, cfg.Share.Server) fmt.Println() fmt.Println("[updates]") - printRow("updates.channel", cfg.Updates.Channel) + printRow(updatesChannelKey, cfg.Updates.Channel) fmt.Println() fmt.Printf("Config: %s\n", p) return nil @@ -166,39 +185,11 @@ func runConfigGet(cmd *cobra.Command, args []string) error { if err != nil { return err } - if resolvedFlag && key != "plans.dir" { - return fmt.Errorf("--resolved is only supported for plans.dir") + if resolvedFlag && key != plansDirKey { + return errors.New("--resolved is only supported for plans.dir") } - if resolvedFlag && key == "plans.dir" { - c, err := getClient() - if err != nil { - return err - } - wsID, _, _, err := resolveProject() - if err != nil { - return err - } - proj, err := c.GetProject(wsID) - if err != nil { - return err - } - cwd, err := os.Getwd() - if err != nil { - return fmt.Errorf("get current directory: %w", err) - } - dir, err := cfgpkg.ExpandPlansDir(cfg.Plans.Dir, map[string]string{ - "project": cfgpkg.SanitizeSlug(proj.Name), - "prefix": proj.Prefix, - }, cwd) - if err != nil { - return err - } - if outputJSON { - outputResult(map[string]string{"plans.dir": dir}) - return nil - } - fmt.Println(dir) - return nil + if resolvedFlag && key == plansDirKey { + return runConfigGetResolved(cfg) } value := getKey(cfg, key) if outputJSON { @@ -209,6 +200,41 @@ func runConfigGet(cmd *cobra.Command, args []string) error { return nil } +// runConfigGetResolved expands plans.dir against the active project's +// template variables and current working directory, then prints the resulting +// absolute path. It hard-errors when no project can be resolved. +func runConfigGetResolved(cfg *cfgpkg.Config) error { + c, err := getClient() + if err != nil { + return err + } + wsID, _, _, err := resolveProject() + if err != nil { + return err + } + proj, err := c.GetProject(wsID) + if err != nil { + return err + } + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("get current directory: %w", err) + } + dir, err := cfgpkg.ExpandPlansDir(cfg.Plans.Dir, map[string]string{ + projectVar: cfgpkg.SanitizeSlug(proj.Name), + "prefix": proj.Prefix, + }, cwd) + if err != nil { + return err + } + if outputJSON { + outputResult(map[string]string{plansDirKey: dir}) + return nil + } + fmt.Println(dir) + return nil +} + // runConfigSet validates and persists a new value for a config key. func runConfigSet(cmd *cobra.Command, args []string) error { key, err := normalizeKey(args[0]) @@ -357,19 +383,19 @@ func levenshtein(a, b string) int { // getKey returns the string form of the config field for key. func getKey(cfg *cfgpkg.Config, key string) string { switch key { - case "cli.server": + case cliServerKey: return cfg.CLI.Server - case "plans.dir": + case plansDirKey: return cfg.Plans.Dir - case "server.port": + case serverPortKey: return strconv.Itoa(cfg.Server.Port) - case "server.db_path": + case serverDBPathKey: return cfg.Server.DBPath - case "share.author": + case shareAuthorKey: return cfg.Share.Author - case "share.server": + case shareServerKey: return cfg.Share.Server - case "updates.channel": + case updatesChannelKey: return cfg.Updates.Channel } return "" @@ -379,23 +405,23 @@ func getKey(cfg *cfgpkg.Config, key string) string { // It then validates the whole config and returns any error. func setKey(cfg *cfgpkg.Config, key, value string) error { switch key { - case "cli.server": + case cliServerKey: cfg.CLI.Server = value - case "plans.dir": + case plansDirKey: cfg.Plans.Dir = value - case "server.port": + case serverPortKey: n, err := strconv.Atoi(value) if err != nil { return errors.New("server.port: must be an integer") } cfg.Server.Port = n - case "server.db_path": + case serverDBPathKey: cfg.Server.DBPath = value - case "share.author": + case shareAuthorKey: cfg.Share.Author = value - case "share.server": + case shareServerKey: cfg.Share.Server = value - case "updates.channel": + case updatesChannelKey: cfg.Updates.Channel = value } if err := cfgpkg.Validate(cfg); err != nil { diff --git a/cmd/arc/config_test.go b/cmd/arc/config_test.go index 1ce8c8f..aaba8e1 100644 --- a/cmd/arc/config_test.go +++ b/cmd/arc/config_test.go @@ -9,8 +9,6 @@ import ( cfgpkg "github.com/sentiolabs/arc/internal/config" ) -const dbPathKey = "server.db_path" - func TestConfigSetGetRoundTrip(t *testing.T) { dir := t.TempDir() configPath = filepath.Join(dir, "config.toml") @@ -94,7 +92,7 @@ func TestNormalizeKeyValid(t *testing.T) { validKeys := []string{ "cli.server", "server.port", - dbPathKey, + serverDBPathKey, "share.author", "share.server", "updates.channel", @@ -116,8 +114,8 @@ func TestNormalizeKeyNormalizes(t *testing.T) { if err != nil { t.Errorf("normalizeKey(server.db-path): %v", err) } - if got != dbPathKey { - t.Errorf("got %q, want %s", got, dbPathKey) + if got != serverDBPathKey { + t.Errorf("got %q, want %s", got, serverDBPathKey) } } diff --git a/cmd/arc/init.go b/cmd/arc/init.go index 282bfe0..8d3b27c 100644 --- a/cmd/arc/init.go +++ b/cmd/arc/init.go @@ -17,6 +17,9 @@ import ( // readable by other tools (AGENTS.md, CLAUDE.md). const filePermissions = 0o644 +// agentsFileName is the name of the agent instructions file created by init. +const agentsFileName = "AGENTS.md" + var initCmd = &cobra.Command{ Use: "init [name]", Short: "Initialize arc in the current directory", @@ -166,7 +169,7 @@ func runInit(cmd *cobra.Command, args []string) error { // addLandingThePlaneInstructions adds "landing the plane" instructions to AGENTS.md func addLandingThePlaneInstructions(verbose bool) error { - filename := "AGENTS.md" + filename := agentsFileName // Get the full AGENTS.md content from template agentsMdContent, err := templates.RenderAgentsMd() @@ -238,7 +241,7 @@ func updateClaudeMdReference(verbose bool) error { // Generate the reference text from template reference, err := templates.RenderClaudeMdReference(templates.ClaudeMdReferenceData{ - AgentsFile: "AGENTS.md", + AgentsFile: agentsFileName, }) if err != nil { return fmt.Errorf("failed to render template: %w", err) @@ -262,7 +265,7 @@ func updateClaudeMdReference(verbose bool) error { contentStr := string(content) // Check if it already references AGENTS.md for session completion - if strings.Contains(contentStr, "AGENTS.md") && strings.Contains(contentStr, "Landing the Plane") { + if strings.Contains(contentStr, agentsFileName) && strings.Contains(contentStr, "Landing the Plane") { if verbose { fmt.Printf(" %s already references AGENTS.md for session completion\n", filename) } diff --git a/cmd/arc/label.go b/cmd/arc/label.go index c450b58..678f50e 100644 --- a/cmd/arc/label.go +++ b/cmd/arc/label.go @@ -29,7 +29,7 @@ func init() { // labelListCmd lists all global labels. // Output is a table (NAME, COLOR, DESCRIPTION) or JSON when --json is set. var labelListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List all labels", RunE: func(cmd *cobra.Command, args []string) error { c, err := getClient() diff --git a/cmd/arc/main.go b/cmd/arc/main.go index d727907..7533af8 100644 --- a/cmd/arc/main.go +++ b/cmd/arc/main.go @@ -53,6 +53,17 @@ const ( priorityNormal = 2 // priorityLow is the priority level for low-importance issues (P3). priorityLow = 3 + + // cmdList is the cobra Use string shared by all "list" sub-commands. + cmdList = "list" + // cmdStart is the cobra Use string for "start" sub-commands. + cmdStart = "start" + // useShowID is the cobra Use string for "show " sub-commands. + useShowID = "show " + // cmdProject is the cobra Use string for the "project" command. + cmdProject = "project" + // flagProject is the name of the persistent --project flag. + flagProject = "project" ) // Global CLI flags shared across all commands. @@ -258,7 +269,7 @@ func init() { rootCmd.PersistentFlags().StringVarP( &serverURL, "server", "s", "", "Server URL (env: ARC_SERVER, default: http://localhost:7432)") - rootCmd.PersistentFlags().StringVar(&projectID, "project", "", "Project ID") + rootCmd.PersistentFlags().StringVar(&projectID, flagProject, "", "Project ID") rootCmd.PersistentFlags().BoolVar(&outputJSON, "json", false, "Output as JSON") rootCmd.PersistentFlags().StringVar(&configPath, "config", "", "Config file path") @@ -289,7 +300,7 @@ func init() { // projectCmd is the parent command for project management. var projectCmd = &cobra.Command{ - Use: "project", + Use: cmdProject, Short: "Manage projects", } @@ -367,7 +378,7 @@ func init() { // projectListCmd lists all projects on the server. var projectListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List all projects", RunE: func(cmd *cobra.Command, args []string) error { c, err := getClient() @@ -477,7 +488,7 @@ var projectDeleteCmd = &cobra.Command{ // listCmd lists issues in the active project with optional filters. var listCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List issues", RunE: func(cmd *cobra.Command, args []string) error { c, err := getClient() @@ -616,7 +627,7 @@ func init() { // showCmd displays full details for a single issue. var showCmd = &cobra.Command{ - Use: "show ", + Use: useShowID, Short: "Show issue details", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -732,7 +743,7 @@ var updateCmd = &cobra.Command{ updates["ai_session_id"] = sessionID // Set status to in_progress unless user explicitly passed --status if !cmd.Flags().Changed("status") { - updates["status"] = "in_progress" + updates["status"] = string(types.StatusInProgress) } } @@ -1083,7 +1094,7 @@ func formatIssue(id, status, issueType string, priority int, title string, label // Status icon icon := statusIconOpen switch status { - case "in_progress": + case string(types.StatusInProgress): icon = "\u25d0" // ◐ case "blocked": icon = "\u25cc" // ◌ diff --git a/cmd/arc/onboard.go b/cmd/arc/onboard.go index da6fad9..fcc0bcb 100644 --- a/cmd/arc/onboard.go +++ b/cmd/arc/onboard.go @@ -168,7 +168,7 @@ func runOnboard(cmd *cobra.Command, args []string) error { // Get in-progress issues inProgressIssues, err := c.ListIssues(wsID, client.ListIssuesOptions{ - Status: "in_progress", + Status: string(types.StatusInProgress), Limit: onboardLimit, }) if err != nil { diff --git a/cmd/arc/paths.go b/cmd/arc/paths.go index a758c47..e09ab9a 100644 --- a/cmd/arc/paths.go +++ b/cmd/arc/paths.go @@ -55,7 +55,7 @@ var pathsRemoveCmd = &cobra.Command{ // pathsListCmd lists paths, optionally across all projects. var pathsListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List paths (use --all for all projects)", RunE: runPathsListCmd, } diff --git a/cmd/arc/plan.go b/cmd/arc/plan.go index a1837c6..951e5f8 100644 --- a/cmd/arc/plan.go +++ b/cmd/arc/plan.go @@ -10,6 +10,7 @@ package main import ( "bufio" + "errors" "fmt" "os" "path/filepath" @@ -80,7 +81,8 @@ func init() { planCmd.AddCommand(planCommentsCmd) planCreateCmd.Flags().StringVar(&titleFlag, "title", "", "Override the plan title written to frontmatter") - planCreateCmd.Flags().BoolVar(&noFrontmatter, "no-frontmatter", false, "Skip writing frontmatter into the plan file") + planCreateCmd.Flags().BoolVar(&noFrontmatter, "no-frontmatter", false, + "Skip writing frontmatter into the plan file") } // planCreateCmd registers a new plan from a file path. @@ -116,11 +118,11 @@ var planCreateCmd = &cobra.Command{ } } meta := plans.Frontmatter{ - Title: title, - Date: time.Now().Format("2006-01-02"), - Project: projName, - Status: "in_review", - Tags: []string{"arc", "design-spec"}, + Title: title, + Date: time.Now().Format("2006-01-02"), + Project: projName, + Status: "in_review", + Tags: []string{"arc", "design-spec"}, ArcReview: plans.ArcReview{Kind: "legacy", ID: plan.ID}, } if e := plans.EnsureFrontmatter(filePath, meta); e != nil { @@ -190,7 +192,7 @@ var planApproveCmd = &cobra.Command{ } if p, e := c.GetPlan(planID); e == nil && p.FilePath != "" { - if e2 := plans.SetStatus(p.FilePath, "approved"); e2 != nil && e2 != plans.ErrNoFrontmatter { + if e2 := plans.SetStatus(p.FilePath, "approved"); e2 != nil && !errors.Is(e2, plans.ErrNoFrontmatter) { _, _ = fmt.Fprintf(os.Stderr, "warning: could not sync status in %s: %v\n", p.FilePath, e2) } } diff --git a/cmd/arc/plan_test.go b/cmd/arc/plan_test.go index c6a8eb4..93360d8 100644 --- a/cmd/arc/plan_test.go +++ b/cmd/arc/plan_test.go @@ -10,7 +10,7 @@ func TestDeriveTitle_H1Heading(t *testing.T) { dir := t.TempDir() f := filepath.Join(dir, "my-spec.md") content := "# My Spec Title\n\nSome body text here.\n" - if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + if err := os.WriteFile(f, []byte(content), 0o600); err != nil { t.Fatalf("write temp file: %v", err) } @@ -26,7 +26,7 @@ func TestDeriveTitle_FilenameFallback(t *testing.T) { // File with a YYYY-MM-DD- date prefix, no H1 heading f := filepath.Join(dir, "2024-01-15-my-design-spec.md") content := "Some content without a heading.\n" - if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + if err := os.WriteFile(f, []byte(content), 0o600); err != nil { t.Fatalf("write temp file: %v", err) } @@ -53,7 +53,7 @@ func TestDeriveTitle_EmptyFile(t *testing.T) { dir := t.TempDir() // Empty file — no heading, should fall back to filename base f := filepath.Join(dir, "2024-05-10-empty-spec.md") - if err := os.WriteFile(f, []byte(""), 0o644); err != nil { + if err := os.WriteFile(f, []byte(""), 0o600); err != nil { t.Fatalf("write temp file: %v", err) } @@ -69,7 +69,7 @@ func TestDeriveTitle_H2OnlyHeading(t *testing.T) { // File whose only heading is ## (H2) — should NOT match, fall back to filename f := filepath.Join(dir, "2024-06-01-h2-only.md") content := "## Not An H1\n\nBody text.\n" - if err := os.WriteFile(f, []byte(content), 0o644); err != nil { + if err := os.WriteFile(f, []byte(content), 0o600); err != nil { t.Fatalf("write temp file: %v", err) } diff --git a/cmd/arc/server.go b/cmd/arc/server.go index 1d34c34..0c5017d 100644 --- a/cmd/arc/server.go +++ b/cmd/arc/server.go @@ -46,6 +46,9 @@ const ( // healthCheckTimeout is how long to wait for the server to pass a health check after starting. healthCheckTimeout = 10 * time.Second + + // statusRunningKey is the JSON field name reporting whether the server is running. + statusRunningKey = "running" ) var serverCmd = &cobra.Command{ @@ -65,7 +68,7 @@ func init() { // ============ Server Start ============ var serverStartCmd = &cobra.Command{ - Use: "start", + Use: cmdStart, Short: "Start the arc server", Long: `Start the arc server as a background daemon. @@ -123,7 +126,7 @@ func runServerStart(cmd *cobra.Command, args []string) error { return fmt.Errorf("get executable path: %w", err) } - cmdArgs := []string{"server", "start", "--foreground", "--port", strconv.Itoa(port)} + cmdArgs := []string{"server", cmdStart, "--foreground", "--port", strconv.Itoa(port)} if dbPath != "" { cmdArgs = append(cmdArgs, "--db", dbPath) } @@ -229,7 +232,7 @@ func runServerStatus(cmd *cobra.Command, args []string) error { if !running { if outputJSON { outputResult(map[string]any{ - "running": false, + statusRunningKey: false, }) } else { _, _ = fmt.Println("Server is not running") @@ -243,9 +246,9 @@ func runServerStatus(cmd *cobra.Command, args []string) error { if err != nil { if outputJSON { outputResult(map[string]any{ - "running": true, - "pid": pid, - "responding": false, + statusRunningKey: true, + "pid": pid, + "responding": false, }) } else { fmt.Printf("Server running (PID %d) but not responding\n", pid) @@ -255,14 +258,14 @@ func runServerStatus(cmd *cobra.Command, args []string) error { if outputJSON { outputResult(map[string]any{ - "running": true, - "pid": pid, - "responding": true, - "status": health.Status, - "port": health.Port, - "webui_url": health.WebUIURL, - "version": health.Version, - "uptime": health.Uptime, + statusRunningKey: true, + "pid": pid, + "responding": true, + "status": health.Status, + "port": health.Port, + "webui_url": health.WebUIURL, + "version": health.Version, + "uptime": health.Uptime, }) } else { fmt.Printf("Server running (PID %d)\n", pid) diff --git a/cmd/arc/share.go b/cmd/arc/share.go index f2b4696..e4896ca 100644 --- a/cmd/arc/share.go +++ b/cmd/arc/share.go @@ -23,6 +23,21 @@ import ( "github.com/sentiolabs/arc/internal/sharesconfig" ) +// Share review event kinds and action values, mirroring the schema in +// web/src/lib/paste/types.ts. +const ( + // shareKindComment is the event kind for a reviewer comment. + shareKindComment = "comment" + // shareKindEdit is the event kind for an author edit of a comment. + shareKindEdit = "edit" + // shareKindRetraction is the event kind for retracting a comment. + shareKindRetraction = "retraction" + // shareActionDelete is the comment action requesting a strikethrough deletion. + shareActionDelete = "delete" + // shareStatusOpen is the unresolved status for a review comment. + shareStatusOpen = "open" +) + // --- plaintext schemas (mirror web/src/lib/paste/types.ts) --- type planPlaintext struct { @@ -128,7 +143,7 @@ var shareCreateCmd = &cobra.Command{ } var shareListCmd = &cobra.Command{ - Use: "list", + Use: cmdList, Short: "List shares known to this machine", RunE: runShareList, } @@ -712,13 +727,13 @@ func replayEvents( retracted := map[string]bool{} for _, d := range events { switch d.kind { - case "comment": + case shareKindComment: applyCommentEvent(d.raw, comments) case "resolution": applyResolutionEvent(d.raw, planAuthor, resolutions) - case "edit": + case shareKindEdit: applyEditEvent(d.raw, planAuthor, comments) - case "retraction": + case shareKindRetraction: applyRetractionEvent(d.raw, comments, retracted) } } @@ -813,7 +828,7 @@ func buildCommentEntries( if retracted[cid] { continue } - status := "open" + status := shareStatusOpen reply := "" if res, ok := resolutions[cid]; ok { status = res.Status @@ -834,7 +849,7 @@ func printCommentEntries(entries []commentEntry) { for _, e := range entries { // Mark deletes visually so they don't get mistaken for empty-body comments. prefix := "" - if e.comment.Action == "delete" { + if e.comment.Action == shareActionDelete { prefix = "[delete] " } fmt.Printf("[%s] %s%s (%s): %s\n", diff --git a/cmd/arc/team.go b/cmd/arc/team.go index ca30c46..175d897 100644 --- a/cmd/arc/team.go +++ b/cmd/arc/team.go @@ -174,7 +174,7 @@ func fetchEpicChildren(c *client.Client, wsID, epicID string, tc *TeamContext) ( // fetchProjectIssues fetches all open and in_progress issues from the project. func fetchProjectIssues(c *client.Client, wsID string) ([]*types.Issue, error) { allIssues, err := c.ListIssues(wsID, client.ListIssuesOptions{ - Status: "open", + Status: string(types.StatusOpen), Limit: teamListLimit, }) if err != nil { @@ -182,7 +182,7 @@ func fetchProjectIssues(c *client.Client, wsID string) ([]*types.Issue, error) { } inProgress, err := c.ListIssues(wsID, client.ListIssuesOptions{ - Status: "in_progress", + Status: string(types.StatusInProgress), Limit: teamListLimit, }) if err != nil { diff --git a/internal/api/issues.go b/internal/api/issues.go index 96a46d8..4b79a5c 100644 --- a/internal/api/issues.go +++ b/internal/api/issues.go @@ -16,6 +16,8 @@ const ( defaultPriority = 2 // queryTrue is the string value for boolean query parameters. queryTrue = "true" + // codeOpenChildren is the error code returned when an issue has open children. + codeOpenChildren = "open_children" ) // createIssueRequest is the request body for creating an issue. @@ -286,9 +288,9 @@ func (s *Server) closeIssue(c echo.Context) error { var openChildrenErr *types.OpenChildrenError if errors.As(err, &openChildrenErr) { return c.JSON(http.StatusConflict, map[string]any{ - "error": openChildrenErr.Error(), - "code": "open_children", - "open_children": openChildrenErr.Children, + "error": openChildrenErr.Error(), + "code": codeOpenChildren, + codeOpenChildren: openChildrenErr.Children, }) } return errorJSON(c, http.StatusInternalServerError, err.Error()) diff --git a/internal/config/template.go b/internal/config/template.go index fef229a..c44169a 100644 --- a/internal/config/template.go +++ b/internal/config/template.go @@ -1,3 +1,6 @@ +// Package config provides configuration loading, validation, and template +// expansion for the arc CLI. Template variables use {name} syntax and are +// expanded by ExpandPlansDir before any path is used at runtime. package config import ( @@ -7,7 +10,10 @@ import ( "strings" ) +// templateVarRe matches {identifier} placeholders in template strings. var templateVarRe = regexp.MustCompile(`\{([a-zA-Z0-9_]+)\}`) + +// slugStripRe matches any run of characters that are not lowercase alphanumeric. var slugStripRe = regexp.MustCompile(`[^a-z0-9]+`) // SanitizeSlug lowercases s, replaces runs of non [a-z0-9] with '-', trims '-'. "" if nothing survives. diff --git a/internal/config/template_test.go b/internal/config/template_test.go index 5368a5d..38dfa11 100644 --- a/internal/config/template_test.go +++ b/internal/config/template_test.go @@ -1,36 +1,39 @@ -package config +package config_test import ( "strings" "testing" + + "github.com/sentiolabs/arc/internal/config" ) // --- Contract assertions --- -var _ = Config{}.Plans.Dir +var _ = config.Config{}.Plans.Dir func TestSanitizeSlug(t *testing.T) { for in, want := range map[string]string{"My App": "my-app", " Foo__Bar ": "foo-bar", "!!!": ""} { - if got := SanitizeSlug(in); got != want { + if got := config.SanitizeSlug(in); got != want { t.Fatalf("SanitizeSlug(%q)=%q want %q", in, got, want) } } } func TestExpandPlansDir(t *testing.T) { - got, err := ExpandPlansDir("~/V/{project}", map[string]string{"project": "arc"}, "/tmp") + got, err := config.ExpandPlansDir("~/V/{project}", map[string]string{"project": "arc"}, "/tmp") if err != nil { t.Fatal(err) } if !strings.HasSuffix(got, "/V/arc") { t.Fatalf("got %q", got) } - if _, err := ExpandPlansDir("~/V/{nope}", map[string]string{}, "/tmp"); err == nil { + if _, err := config.ExpandPlansDir("~/V/{nope}", map[string]string{}, "/tmp"); err == nil { t.Fatal("want unknown-var error") } - if _, err := ExpandPlansDir("~/V/{project}", map[string]string{"project": ""}, "/tmp"); err == nil { + if _, err := config.ExpandPlansDir("~/V/{project}", map[string]string{"project": ""}, "/tmp"); err == nil { t.Fatal("want empty-var error") } - if rel, err := ExpandPlansDir("docs/plans", map[string]string{}, "/tmp"); err != nil || rel != "/tmp/docs/plans" { - t.Fatalf("rel=%q err=%v", rel, err) + rel, relErr := config.ExpandPlansDir("docs/plans", map[string]string{}, "/tmp") + if relErr != nil || rel != "/tmp/docs/plans" { + t.Fatalf("rel=%q err=%v", rel, relErr) } } diff --git a/internal/config/validate.go b/internal/config/validate.go index 0c9f878..8bc9089 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -1,3 +1,6 @@ +// Package config provides configuration loading, validation, and template +// expansion. Validate checks all fields and returns a ValidationError that +// describes every invalid field so callers can surface them together. package config import ( @@ -7,6 +10,12 @@ import ( "strings" ) +// Allowed template variable names for plans.dir. +const ( + tmplVarProject = "project" + tmplVarPrefix = "prefix" +) + // ValidChannels lists the allowed values for updates.channel. var ValidChannels = []string{"stable", "rc", "nightly"} @@ -51,14 +60,16 @@ func Validate(cfg *Config) error { if !channelOK { errs["updates.channel"] = "must be one of: " + strings.Join(ValidChannels, ", ") } - if cfg.Plans.Dir == "" { + switch { + case cfg.Plans.Dir == "": errs["plans.dir"] = "must not be empty" - } else if strings.Contains(cfg.Plans.Dir, "..") { + case strings.Contains(cfg.Plans.Dir, ".."): errs["plans.dir"] = "must not contain '..'" - } else { + default: for _, m := range templateVarRe.FindAllStringSubmatch(cfg.Plans.Dir, -1) { - if m[1] != "project" && m[1] != "prefix" { - errs["plans.dir"] = "unknown template variable {" + m[1] + "} (allowed: project, prefix)" + if m[1] != tmplVarProject && m[1] != tmplVarPrefix { + errs["plans.dir"] = "unknown template variable {" + m[1] + + "} (allowed: " + tmplVarProject + ", " + tmplVarPrefix + ")" break } } diff --git a/internal/plans/frontmatter.go b/internal/plans/frontmatter.go index 8c630f8..c490a2a 100644 --- a/internal/plans/frontmatter.go +++ b/internal/plans/frontmatter.go @@ -50,7 +50,7 @@ func findClosingDelim(b []byte) int { return -1 } // Position of the character immediately following "---". - after := idx + 4 + after := idx + len(fmDelim) if after == len(search) { // "---" is at the very end of b with no following character — valid EOF closer. return offset + idx @@ -60,7 +60,7 @@ func findClosingDelim(b []byte) int { return offset + idx } // Not an exact "---" line; skip past this match and keep searching. - advance := idx + 4 + advance := idx + len(fmDelim) offset += advance search = search[advance:] } @@ -108,10 +108,10 @@ func EnsureFrontmatter(path string, meta Frontmatter) error { return err } var buf bytes.Buffer - buf.Write(fmDelim) - buf.Write(y) - buf.WriteString("---\n") - buf.Write(body) + _, _ = buf.Write(fmDelim) + _, _ = buf.Write(y) + _, _ = buf.WriteString("---\n") + _, _ = buf.Write(body) return atomicWrite(path, buf.Bytes()) } @@ -160,12 +160,12 @@ func atomicWrite(path string, data []byte) error { } name := tmp.Name() if _, err := tmp.Write(data); err != nil { - tmp.Close() - os.Remove(name) + _ = tmp.Close() + _ = os.Remove(name) return err } if err := tmp.Close(); err != nil { - os.Remove(name) + _ = os.Remove(name) return err } return os.Rename(name, path) diff --git a/internal/plans/frontmatter_test.go b/internal/plans/frontmatter_test.go index d63516e..fd0b31e 100644 --- a/internal/plans/frontmatter_test.go +++ b/internal/plans/frontmatter_test.go @@ -1,30 +1,40 @@ -package plans +package plans_test import ( + "errors" "os" "path/filepath" "strings" "testing" + + "github.com/sentiolabs/arc/internal/plans" ) // --- Contract assertions --- -var _ string = Frontmatter{}.Status -var _ ArcReview = Frontmatter{}.ArcReview +var ( + _ string = plans.Frontmatter{}.Status + _ plans.ArcReview = plans.Frontmatter{}.ArcReview +) func TestEnsureAndSetStatus(t *testing.T) { dir := t.TempDir() p := filepath.Join(dir, "spec.md") - if err := os.WriteFile(p, []byte("# Title\n\nbody\n"), 0o644); err != nil { + if err := os.WriteFile(p, []byte("# Title\n\nbody\n"), 0o600); err != nil { t.Fatal(err) } - if err := EnsureFrontmatter(p, Frontmatter{Title: "T", Date: "2026-06-07", Project: "arc", Status: "in_review", Tags: []string{"arc"}, ArcReview: ArcReview{Kind: "legacy", ID: "plan.x"}}); err != nil { + meta := plans.Frontmatter{ + Title: "T", Date: "2026-06-07", Project: "arc", Status: "in_review", + Tags: []string{"arc"}, + ArcReview: plans.ArcReview{Kind: "legacy", ID: "plan.x"}, + } + if err := plans.EnsureFrontmatter(p, meta); err != nil { t.Fatal(err) } got, _ := os.ReadFile(p) if !strings.HasPrefix(string(got), "---\n") || !strings.Contains(string(got), "# Title") { t.Fatalf("bad: %s", got) } - if err := SetStatus(p, "approved"); err != nil { + if err := plans.SetStatus(p, "approved"); err != nil { t.Fatal(err) } got2, _ := os.ReadFile(p) @@ -32,103 +42,105 @@ func TestEnsureAndSetStatus(t *testing.T) { t.Fatalf("status: %s", got2) } plain := filepath.Join(dir, "plain.md") - if err := os.WriteFile(plain, []byte("no fm\n"), 0o644); err != nil { + if err := os.WriteFile(plain, []byte("no fm\n"), 0o600); err != nil { t.Fatal(err) } - if err := SetStatus(plain, "approved"); err != ErrNoFrontmatter { + if err := plans.SetStatus(plain, "approved"); !errors.Is(err, plans.ErrNoFrontmatter) { t.Fatalf("want ErrNoFrontmatter got %v", err) } } -// TestReadFrontmatterBodyWithDashes verifies that a line starting with "---" inside -// the body (e.g. a markdown horizontal rule or "----") does not close the frontmatter -// block — only an exact "---" line (with no other characters) acts as the closer. -func TestReadFrontmatterBodyWithDashes(t *testing.T) { - // Body contains a line "----" (four dashes) and a line "--- x" — neither should - // be treated as the closing delimiter. Note: the body here is AFTER the real - // closing "---", so the current search order still works fine. - t.Run("dashes_in_body_after_closer", func(t *testing.T) { - input := "---\ntitle: Test\nstatus: draft\n---\n# Heading\n\n----\n\n--- x\n\nend\n" - fm, body, ok, err := ReadFrontmatter([]byte(input)) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !ok { - t.Fatal("expected frontmatter to be found") - } - if fm.Title != "Test" { - t.Fatalf("unexpected title: %q", fm.Title) - } - if !strings.Contains(string(body), "----") { - t.Errorf("body should contain '----' (markdown hr); got: %q", body) - } - if !strings.Contains(string(body), "--- x") { - t.Errorf("body should contain '--- x'; got: %q", body) - } - if !strings.Contains(string(body), "end") { - t.Errorf("body should contain 'end'; got: %q", body) - } +// TestReadFrontmatterDashesInBodyAfterCloser verifies that a line starting with +// "---" inside the body (e.g. a markdown horizontal rule or "----") does not +// close the frontmatter block — only an exact "---" line (with no other +// characters) acts as the closer. +// +// Body contains a line "----" (four dashes) and a line "--- x" — neither should +// be treated as the closing delimiter. Note: the body here is AFTER the real +// closing "---", so the current search order still works fine. +func TestReadFrontmatterDashesInBodyAfterCloser(t *testing.T) { + input := "---\ntitle: Test\nstatus: draft\n---\n# Heading\n\n----\n\n--- x\n\nend\n" + fm, body, ok, err := plans.ReadFrontmatter([]byte(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatal("expected frontmatter to be found") + } + if fm.Title != "Test" { + t.Fatalf("unexpected title: %q", fm.Title) + } + if !strings.Contains(string(body), "----") { + t.Errorf("body should contain '----' (markdown hr); got: %q", body) + } + if !strings.Contains(string(body), "--- x") { + t.Errorf("body should contain '--- x'; got: %q", body) + } + if !strings.Contains(string(body), "end") { + t.Errorf("body should contain 'end'; got: %q", body) + } - // EnsureFrontmatter round-trip should preserve that body content. - dir := t.TempDir() - p := filepath.Join(dir, "spec.md") - if err := os.WriteFile(p, []byte(input), 0o644); err != nil { - t.Fatal(err) - } - if err := EnsureFrontmatter(p, Frontmatter{Title: "Test", Status: "draft"}); err != nil { - t.Fatal(err) - } - out, err := os.ReadFile(p) - if err != nil { - t.Fatal(err) - } - if !strings.Contains(string(out), "----") { - t.Errorf("round-trip body missing '----'; got: %q", out) - } - if !strings.Contains(string(out), "--- x") { - t.Errorf("round-trip body missing '--- x'; got: %q", out) - } - }) + // EnsureFrontmatter round-trip should preserve that body content. + dir := t.TempDir() + p := filepath.Join(dir, "spec.md") + if err := os.WriteFile(p, []byte(input), 0o600); err != nil { + t.Fatal(err) + } + if err := plans.EnsureFrontmatter(p, plans.Frontmatter{Title: "Test", Status: "draft"}); err != nil { + t.Fatal(err) + } + out, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "----") { + t.Errorf("round-trip body missing '----'; got: %q", out) + } + if !strings.Contains(string(out), "--- x") { + t.Errorf("round-trip body missing '--- x'; got: %q", out) + } +} - // This sub-test verifies that the body can itself contain "----" (four-dash hr) - // followed by "--- x" without the parser being confused — the only valid closer - // is exactly "---" (optionally followed by CRLF/LF or EOF). - // The key assertion: the body returned must NOT start with "---\n", which would - // indicate the parser mistook "----" in the body as the closing delimiter. - t.Run("dashes_before_exact_closer_in_body", func(t *testing.T) { - // Structure: opening --- | YAML | closing --- | body with ---- and --- x - // With the naive "\n---" search, the first "\n---" found in the body - // after the real closer would be "--- x" — but since we already have - // the real closer, this only matters if "----" appeared BEFORE it. - // Here we test that "----\n" and "--- x\n" in the body do not - // pollute the frontmatter block and that body is returned intact. - input := "---\ntitle: Tricky\nstatus: draft\n---\n----\n--- x\nbody after\n" - fm, body, ok, err := ReadFrontmatter([]byte(input)) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !ok { - t.Fatal("expected frontmatter to be found") - } - if fm.Title != "Tricky" { - t.Fatalf("unexpected title: %q", fm.Title) - } - // Body must contain all lines after the closing ---. - if !strings.Contains(string(body), "----") { - t.Errorf("body should contain '----'; got: %q", body) - } - if !strings.Contains(string(body), "--- x") { - t.Errorf("body should contain '--- x'; got: %q", body) - } - if !strings.Contains(string(body), "body after") { - t.Errorf("body should contain 'body after'; got: %q", body) - } - // Body must start at "----", NOT at "---\n" (which would mean the closer - // was missed and body contains the closing delimiter line). - if strings.HasPrefix(string(body), "---\n") { - t.Errorf("body starts with '---\\n' suggesting the real closer was not recognized; got: %q", body) - } - }) +// TestReadFrontmatterDashesBeforeExactCloser verifies that the body can itself +// contain "----" (four-dash hr) followed by "--- x" without the parser being +// confused — the only valid closer is exactly "---" (optionally followed by +// CRLF/LF or EOF). +// +// The key assertion: the body returned must NOT start with "---\n", which would +// indicate the parser mistook "----" in the body as the closing delimiter. +func TestReadFrontmatterDashesBeforeExactCloser(t *testing.T) { + // Structure: opening --- | YAML | closing --- | body with ---- and --- x + // With the naive "\n---" search, the first "\n---" found in the body + // after the real closer would be "--- x" — but since we already have + // the real closer, this only matters if "----" appeared BEFORE it. + // Here we test that "----\n" and "--- x\n" in the body do not + // pollute the frontmatter block and that body is returned intact. + input := "---\ntitle: Tricky\nstatus: draft\n---\n----\n--- x\nbody after\n" + fm, body, ok, err := plans.ReadFrontmatter([]byte(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatal("expected frontmatter to be found") + } + if fm.Title != "Tricky" { + t.Fatalf("unexpected title: %q", fm.Title) + } + // Body must contain all lines after the closing ---. + if !strings.Contains(string(body), "----") { + t.Errorf("body should contain '----'; got: %q", body) + } + if !strings.Contains(string(body), "--- x") { + t.Errorf("body should contain '--- x'; got: %q", body) + } + if !strings.Contains(string(body), "body after") { + t.Errorf("body should contain 'body after'; got: %q", body) + } + // Body must start at "----", NOT at "---\n" (which would mean the closer + // was missed and body contains the closing delimiter line). + if strings.HasPrefix(string(body), "---\n") { + t.Errorf("body starts with '---\\n' suggesting the real closer was not recognized; got: %q", body) + } } // TestReadFrontmatterNoTrailingNewlineAfterClose verifies that a file ending @@ -136,7 +148,7 @@ func TestReadFrontmatterBodyWithDashes(t *testing.T) { func TestReadFrontmatterNoTrailingNewlineAfterClose(t *testing.T) { // No newline after closing --- input := "---\ntitle: Test\nstatus: draft\n---" - fm, body, ok, err := ReadFrontmatter([]byte(input)) + fm, body, ok, err := plans.ReadFrontmatter([]byte(input)) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -155,10 +167,10 @@ func TestReadFrontmatterNoTrailingNewlineAfterClose(t *testing.T) { dir := t.TempDir() p := filepath.Join(dir, "spec.md") // Write file with trailing body but no final newline after --- - if err := os.WriteFile(p, []byte("---\ntitle: NoNL\nstatus: draft\n---\nbody here"), 0o644); err != nil { + if err := os.WriteFile(p, []byte("---\ntitle: NoNL\nstatus: draft\n---\nbody here"), 0o600); err != nil { t.Fatal(err) } - if err := EnsureFrontmatter(p, Frontmatter{Title: "NoNL", Status: "draft"}); err != nil { + if err := plans.EnsureFrontmatter(p, plans.Frontmatter{Title: "NoNL", Status: "draft"}); err != nil { t.Fatal(err) } out, err := os.ReadFile(p) @@ -178,11 +190,11 @@ func TestSetStatusCRLF(t *testing.T) { crlf := "---\r\ntitle: CRLFTest\r\nstatus: draft\r\n---\r\n# Body\r\n" dir := t.TempDir() p := filepath.Join(dir, "crlf.md") - if err := os.WriteFile(p, []byte(crlf), 0o644); err != nil { + if err := os.WriteFile(p, []byte(crlf), 0o600); err != nil { t.Fatal(err) } - if err := SetStatus(p, "approved"); err != nil { + if err := plans.SetStatus(p, "approved"); err != nil { t.Fatalf("SetStatus on CRLF file returned error: %v", err) } diff --git a/internal/project/naming.go b/internal/project/naming.go index 8d9b267..6da1a36 100644 --- a/internal/project/naming.go +++ b/internal/project/naming.go @@ -22,6 +22,8 @@ const ( prefixSuffixLen = 4 // maxSanitizedNameLen is the maximum length for a sanitized project basename. maxSanitizedNameLen = 20 + // fallbackBasename is used when a sanitized name would otherwise be empty. + fallbackBasename = "project" ) // MaxBasenameTruncation is the max number of alphanumeric chars kept from the basename. @@ -271,7 +273,7 @@ func SanitizeBasename(name string) string { // Fallback for empty result if name == "" { - name = "project" + name = fallbackBasename } return name