From f32c8fd93ac4dcea04d4835e128f61fd57029721 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 15:05:57 -0700 Subject: [PATCH 1/9] feat(validate): require body flags on decision/learning add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase SK tasks 1 and 2 — tighten the capture surface so ctx decision add and ctx learning add reject submissions that lack body content. Behavior: - ctx decision add now requires --context, --rationale, --consequence to be present and non-placeholder. - ctx learning add now requires --context, --lesson, --application to be present and non-placeholder. - Placeholder values rejected case-insensitively after trim: tbd, n/a, na, none, see chat, see above, see below, pending, to be done, plus whitespace-only. - Substring matches are NOT placeholders ("we deferred the rationale as TBD originally" passes; "TBD" alone does not). Layout follows the project conventions: - internal/config/validate/ holds the placeholder constants. - internal/err/cli/ gains FlagEmpty, FlagPlaceholder, FlagUnregistered, MarkRequiredFailed; format strings in errors.yaml under err.validation.*. - internal/cli/add/core/validate/ wires required-flag marking and the placeholder-rejection PreRunE wrapper. The noun-level Cmd() in decision/cmd/add and learning/cmd/add calls RequireBodyFlags with the noun's three body flags. Tests cover: each placeholder value, whitespace, substring acceptance, missing-flag rejection, and PreRunE chaining. Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- .context/TASKS.md | 6 +- internal/assets/commands/text/errors.yaml | 8 + internal/cli/add/core/validate/doc.go | 25 +++ internal/cli/add/core/validate/required.go | 79 +++++++ .../cli/add/core/validate/required_test.go | 202 +++++++++++++++++ internal/cli/decision/cmd/add/add_test.go | 97 +++++++- internal/cli/decision/cmd/add/cmd.go | 17 +- .../cli/decision/cmd/add/testmain_test.go | 23 ++ internal/cli/learning/cmd/add/add_test.go | 93 +++++++- internal/cli/learning/cmd/add/cmd.go | 15 +- .../cli/learning/cmd/add/testmain_test.go | 23 ++ internal/config/embed/text/err_validate.go | 13 ++ internal/config/validate/doc.go | 17 ++ internal/config/validate/placeholder.go | 48 ++++ internal/err/cli/cli.go | 64 ++++++ specs/skill-surface-polish.md | 208 ++++++++++++++++++ 16 files changed, 925 insertions(+), 13 deletions(-) create mode 100644 internal/cli/add/core/validate/doc.go create mode 100644 internal/cli/add/core/validate/required.go create mode 100644 internal/cli/add/core/validate/required_test.go create mode 100644 internal/cli/decision/cmd/add/testmain_test.go create mode 100644 internal/cli/learning/cmd/add/testmain_test.go create mode 100644 internal/config/validate/doc.go create mode 100644 internal/config/validate/placeholder.go create mode 100644 specs/skill-surface-polish.md diff --git a/.context/TASKS.md b/.context/TASKS.md index bd4c60819..42a788c21 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -1715,12 +1715,12 @@ Spec: `specs/ceremony-profiles.md` ### Phase SK: Skill Surface Polish (Phase 0a; prerequisite for Phase KB) `#priority:high #added:2026-05-09` -Spec: TBD (design ref: `ideas/002-editorial-pipeline-and-skill-rigor.md` §3 "Reframing the wishy-washy skills") +Spec: `specs/skill-surface-polish.md` (design ref: `ideas/002-editorial-pipeline-and-skill-rigor.md` §3 "Reframing the wishy-washy skills") Tightens existing capture skills to sibling-project rigor before the editorial pipeline (Phase KB) lifts that pattern wholesale. Independent of Phase RG; both can ship in parallel. -- [ ] Add `MarkFlagRequired` to `ctx decision add` for `--context`, `--rationale`, `--consequence`; reject placeholder values (`TBD`, `see chat`, whitespace-only) at CLI level -- [ ] Add `MarkFlagRequired` to `ctx learning add` for `--context`, `--lesson`, `--application`; same placeholder rejection +- [x] Add `MarkFlagRequired` to `ctx decision add` for `--context`, `--rationale`, `--consequence`; reject placeholder values (`TBD`, `see chat`, whitespace-only) at CLI level +- [x] Add `MarkFlagRequired` to `ctx learning add` for `--context`, `--lesson`, `--application`; same placeholder rejection - [ ] Add `--brief ` flag to `/ctx-spec` skill: when present, read the file as authoritative source per the sibling's authority order (frozen contracts > recorded decisions > debrief > agent inference labeled `TBD`); skip the fresh template Q&A - [ ] Update `/ctx-plan` skill to always offer to write the debated brief to `.context/briefs/-.md` at the end of an interview (creating `.context/briefs/` if absent) - [ ] Add an `Authority boundary (vs other skills)` section to `/ctx-decision-add`, `/ctx-learning-add`, `/ctx-task-add`, `/ctx-convention-add` skill files (prevent silent promotion handover→decision, learning→convention, etc., without explicit user ask) diff --git a/internal/assets/commands/text/errors.yaml b/internal/assets/commands/text/errors.yaml index 73ffc8791..939b6bfc7 100644 --- a/internal/assets/commands/text/errors.yaml +++ b/internal/assets/commands/text/errors.yaml @@ -574,6 +574,14 @@ err.validation.parse-file: short: 'failed to parse %s: %w' err.validation.working-directory: short: 'failed to get working directory: %w' +err.validation.flag-empty: + short: 'flag --%s is required and cannot be empty or whitespace-only' +err.validation.flag-placeholder: + short: 'flag --%s rejected placeholder value %q; provide concrete content' +err.validation.flag-unregistered: + short: 'flag %q not registered on command %q' +err.validation.mark-required: + short: 'mark flag %q required: %w' err.hub.generate-token: short: 'generate token: %w' err.hub.internal: diff --git a/internal/cli/add/core/validate/doc.go b/internal/cli/add/core/validate/doc.go new file mode 100644 index 000000000..2eafc4e01 --- /dev/null +++ b/internal/cli/add/core/validate/doc.go @@ -0,0 +1,25 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Package validate enforces noun-specific body-flag contracts on +// add subcommands. It centralises two pieces of policy: +// +// - Required flags: noun-level commands declare which body +// flags must be present. RequireBodyFlags wires +// [cobra.Command.MarkFlagRequired] for each so cobra rejects +// missing flags before RunE runs. +// +// - Placeholder rejection: a closed set of placeholder values +// (TBD, see chat, n/a, etc., plus whitespace-only) is rejected +// at PreRunE time with a clear error naming the flag and the +// offending value. Substring matches are not treated as +// placeholders so legitimate prose containing the word "TBD" +// still passes. +// +// The package is internal to the add core; noun-level subcommands +// (decision, learning) call RequireBodyFlags after constructing +// their command via [build.Cmd]. +package validate diff --git a/internal/cli/add/core/validate/required.go b/internal/cli/add/core/validate/required.go new file mode 100644 index 000000000..1a2a8f716 --- /dev/null +++ b/internal/cli/add/core/validate/required.go @@ -0,0 +1,79 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "strings" + + "github.com/spf13/cobra" + + cfgValidate "github.com/ActiveMemory/ctx/internal/config/validate" + errCli "github.com/ActiveMemory/ctx/internal/err/cli" +) + +// RequireBodyFlags marks each named flag as cobra-required and +// wraps the command's PreRunE to reject placeholder values +// (TBD, see chat, n/a, etc., plus whitespace-only) on those +// flags. Existing PreRunE is preserved and runs after the +// placeholder check. +// +// Parameters: +// - c: cobra command to mutate +// - flags: names of body flags to require and policy-check +// +// Returns: +// - error: non-nil if any named flag does not exist on c, in +// which case the command is left unmodified +func RequireBodyFlags(c *cobra.Command, flags ...string) error { + for _, name := range flags { + if c.Flag(name) == nil { + return errCli.FlagUnregistered(name, c.Name()) + } + } + for _, name := range flags { + if markErr := c.MarkFlagRequired(name); markErr != nil { + return errCli.MarkRequiredFailed(name, markErr) + } + } + prev := c.PreRunE + c.PreRunE = func(cmd *cobra.Command, args []string) error { + for _, name := range flags { + value, _ := cmd.Flags().GetString(name) + if rejectErr := RejectPlaceholder( + name, value, + ); rejectErr != nil { + return rejectErr + } + } + if prev != nil { + return prev(cmd, args) + } + return nil + } + return nil +} + +// RejectPlaceholder returns an error if value is a placeholder +// (exact case-insensitive match against the closed set, plus +// whitespace-only). Substring matches are not rejected. +// +// Parameters: +// - flag: name of the flag, used in the error message +// - value: raw flag value as received from cobra +// +// Returns: +// - error: non-nil when value is a placeholder; nil otherwise +func RejectPlaceholder(flag, value string) error { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return errCli.FlagEmpty(flag) + } + if _, hit := cfgValidate.Placeholders[strings.ToLower(trimmed)]; hit { + return errCli.FlagPlaceholder(flag, value) + } + return nil +} diff --git a/internal/cli/add/core/validate/required_test.go b/internal/cli/add/core/validate/required_test.go new file mode 100644 index 000000000..0b11c73d4 --- /dev/null +++ b/internal/cli/add/core/validate/required_test.go @@ -0,0 +1,202 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestRejectPlaceholderAcceptsLegitimate(t *testing.T) { + for _, v := range []string{ + "a real rationale", + "because PostgreSQL is well-supported", + "we need TBD-style markers in the body but the field is real", + "see above the line break", + } { + if err := RejectPlaceholder("context", v); err != nil { + t.Errorf("RejectPlaceholder(%q) = %v, want nil", v, err) + } + } +} + +func TestRejectPlaceholderRejectsExactMatches(t *testing.T) { + for _, v := range []string{ + "TBD", "tbd", "Tbd", + "N/A", "n/a", "na", + "see chat", "See Chat", + "see above", "see below", + "pending", "PENDING", + "none", "to be done", + } { + if err := RejectPlaceholder("context", v); err == nil { + t.Errorf("RejectPlaceholder(%q) = nil, want error", v) + } + } +} + +func TestRejectPlaceholderRejectsWhitespace(t *testing.T) { + for _, v := range []string{ + "", + " ", + "\t", + "\n", + " \t \n ", + } { + err := RejectPlaceholder("rationale", v) + if err == nil { + t.Errorf("RejectPlaceholder(%q) = nil, want error", v) + } + if !strings.Contains(err.Error(), "rationale") { + t.Errorf("error should name flag 'rationale': %v", err) + } + } +} + +func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { + // " TBD " is still a placeholder after trim. + err := RejectPlaceholder("consequence", " TBD ") + if err == nil { + t.Error("padded placeholder should still be rejected") + } +} + +func TestRequireBodyFlagsRejectsMissingFlag(t *testing.T) { + c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} + c.Flags().String("context", "", "") + err := RequireBodyFlags(c, "context", "nonexistent") + if err == nil { + t.Fatal("expected error for unregistered flag") + } +} + +func TestRequireBodyFlagsMarksRequired(t *testing.T) { + c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} + c.Flags().String("context", "", "") + c.Flags().String("rationale", "", "") + if err := RequireBodyFlags(c, "context", "rationale"); err != nil { + t.Fatalf("RequireBodyFlags error: %v", err) + } + // Missing --context and --rationale: PreRunE fires first and + // reports the empty value before cobra's required-flag check. + c.SetArgs([]string{}) + c.SetOut(&strings.Builder{}) + c.SetErr(&strings.Builder{}) + err := c.Execute() + if err == nil { + t.Fatal("expected error for missing required body flags") + } + if !strings.Contains(err.Error(), "context") { + t.Errorf("error should name the empty flag: %v", err) + } +} + +// TestRequireBodyFlagsCobraRequiredAnnotation verifies that +// MarkFlagRequired's annotation is set on each flag. The +// PreRunE shadow normally fires first; with PreRunE bypassed, +// cobra still rejects the missing flag at validateRequiredFlags. +func TestRequireBodyFlagsCobraRequiredAnnotation(t *testing.T) { + c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} + c.Flags().String("context", "", "") + if err := RequireBodyFlags(c, "context"); err != nil { + t.Fatalf("RequireBodyFlags error: %v", err) + } + flag := c.Flag("context") + if flag == nil { + t.Fatal("context flag missing") + } + if _, ok := flag.Annotations[cobra.BashCompOneRequiredFlag]; !ok { + t.Error("expected cobra required annotation on --context") + } +} + +func TestRequireBodyFlagsRejectsPlaceholderViaPreRunE(t *testing.T) { + ran := false + c := &cobra.Command{ + Use: "test", + RunE: func(_ *cobra.Command, _ []string) error { + ran = true + return nil + }, + } + c.Flags().String("context", "", "") + c.Flags().String("rationale", "", "") + if err := RequireBodyFlags(c, "context", "rationale"); err != nil { + t.Fatalf("RequireBodyFlags error: %v", err) + } + c.SetArgs([]string{ + "--context", "TBD", + "--rationale", "good reason", + }) + c.SetOut(&strings.Builder{}) + c.SetErr(&strings.Builder{}) + err := c.Execute() + if err == nil { + t.Fatal("expected placeholder rejection") + } + if !strings.Contains(err.Error(), "context") { + t.Errorf("error should name the offending flag: %v", err) + } + if ran { + t.Error("RunE should not have executed after PreRunE rejected input") + } +} + +func TestRequireBodyFlagsAllowsLegitimateInput(t *testing.T) { + ran := false + c := &cobra.Command{ + Use: "test", + RunE: func(_ *cobra.Command, _ []string) error { + ran = true + return nil + }, + } + c.Flags().String("context", "", "") + c.Flags().String("rationale", "", "") + if err := RequireBodyFlags(c, "context", "rationale"); err != nil { + t.Fatalf("RequireBodyFlags error: %v", err) + } + c.SetArgs([]string{ + "--context", "real context", + "--rationale", "real rationale", + }) + c.SetOut(&strings.Builder{}) + c.SetErr(&strings.Builder{}) + if err := c.Execute(); err != nil { + t.Fatalf("legitimate input rejected: %v", err) + } + if !ran { + t.Error("RunE should have executed") + } +} + +func TestRequireBodyFlagsPreservesExistingPreRunE(t *testing.T) { + preRan := false + c := &cobra.Command{ + Use: "test", + PreRunE: func(_ *cobra.Command, _ []string) error { + preRan = true + return nil + }, + RunE: func(_ *cobra.Command, _ []string) error { return nil }, + } + c.Flags().String("context", "", "") + if err := RequireBodyFlags(c, "context"); err != nil { + t.Fatalf("RequireBodyFlags error: %v", err) + } + c.SetArgs([]string{"--context", "legitimate"}) + c.SetOut(&strings.Builder{}) + c.SetErr(&strings.Builder{}) + if err := c.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !preRan { + t.Error("existing PreRunE should still execute") + } +} diff --git a/internal/cli/decision/cmd/add/add_test.go b/internal/cli/decision/cmd/add/add_test.go index afeb1e308..00c7ba2f3 100644 --- a/internal/cli/decision/cmd/add/add_test.go +++ b/internal/cli/decision/cmd/add/add_test.go @@ -70,7 +70,10 @@ func TestDecisionAdd(t *testing.T) { } // TestDecisionAddRequiresFlags verifies that omitting -// required flags produces an error. +// required body flags produces an error. The placeholder +// check fires first (PreRunE runs before cobra's required- +// flag validation), so the message names the first empty +// body flag rather than --session-id. func TestDecisionAddRequiresFlags(t *testing.T) { tmpDir, err := os.MkdirTemp("", "cli-decision-add-req-*") if err != nil { @@ -98,7 +101,95 @@ func TestDecisionAddRequiresFlags(t *testing.T) { if err == nil { t.Fatal("expected error when adding decision without required flags") } - if !strings.Contains(err.Error(), "--session-id") { - t.Errorf("error should mention missing --session-id flag: %v", err) + if !strings.Contains(err.Error(), "--context") { + t.Errorf("error should mention missing --context flag: %v", err) + } +} + +// TestDecisionAddRejectsPlaceholderRationale verifies that +// passing a placeholder body-flag value (TBD, see chat, etc.) +// fails fast at PreRunE. +func TestDecisionAddRejectsPlaceholderRationale(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cli-decision-add-ph-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + origDir, _ := os.Getwd() + if err = os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + testctx.Declare(t, tmpDir) + + initCmd := initialize.Cmd() + initCmd.SetArgs([]string{}) + if err = initCmd.Execute(); err != nil { + t.Fatalf("init failed: %v", err) + } + + addCmd := Cmd() + addCmd.SetArgs([]string{ + "Decision body", + "--session-id", "test1234", + "--branch", "main", + "--commit", "abc123", + "--context", "real context", + "--rationale", "TBD", + "--consequence", "real consequence", + }) + err = addCmd.Execute() + if err == nil { + t.Fatal("expected placeholder rejection for --rationale=TBD") + } + if !strings.Contains(err.Error(), "rationale") { + t.Errorf("error should name --rationale: %v", err) + } + if !strings.Contains(err.Error(), "placeholder") { + t.Errorf("error should explain placeholder rejection: %v", err) + } +} + +// TestDecisionAddRejectsWhitespaceContext verifies whitespace- +// only values are rejected at PreRunE. +func TestDecisionAddRejectsWhitespaceContext(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cli-decision-add-ws-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + origDir, _ := os.Getwd() + if err = os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + testctx.Declare(t, tmpDir) + + initCmd := initialize.Cmd() + initCmd.SetArgs([]string{}) + if err = initCmd.Execute(); err != nil { + t.Fatalf("init failed: %v", err) + } + + addCmd := Cmd() + addCmd.SetArgs([]string{ + "Decision body", + "--session-id", "test1234", + "--branch", "main", + "--commit", "abc123", + "--context", " ", + "--rationale", "real rationale", + "--consequence", "real consequence", + }) + err = addCmd.Execute() + if err == nil { + t.Fatal("expected rejection for whitespace --context") + } + if !strings.Contains(err.Error(), "context") { + t.Errorf("error should name --context: %v", err) } } diff --git a/internal/cli/decision/cmd/add/cmd.go b/internal/cli/decision/cmd/add/cmd.go index 8df16e67d..d8a06e215 100644 --- a/internal/cli/decision/cmd/add/cmd.go +++ b/internal/cli/decision/cmd/add/cmd.go @@ -10,18 +10,31 @@ import ( "github.com/spf13/cobra" "github.com/ActiveMemory/ctx/internal/cli/add/core/build" + "github.com/ActiveMemory/ctx/internal/cli/add/core/validate" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" "github.com/ActiveMemory/ctx/internal/config/entry" + cFlag "github.com/ActiveMemory/ctx/internal/config/flag" ) // Cmd returns the "ctx decision add" subcommand. // // Adds a new decision entry to DECISIONS.md with the // required provenance, context, rationale, and consequence -// flags. Implementation lives in the shared add core. +// flags. Implementation lives in the shared add core; this +// noun-level constructor additionally enforces that the +// three body flags are present and non-placeholder. // // Returns: // - *cobra.Command: Configured decision add subcommand func Cmd() *cobra.Command { - return build.Cmd(entry.Decision, cmd.DescKeyDecisionAdd, cmd.UseDecisionAdd) + c := build.Cmd(entry.Decision, cmd.DescKeyDecisionAdd, cmd.UseDecisionAdd) + if err := validate.RequireBodyFlags( + c, cFlag.Context, cFlag.Rationale, cFlag.Consequence, + ); err != nil { + // Programming error — every body flag we name must exist + // on the shared add command. Panic at command-construction + // time rather than silently shipping a half-wired command. + panic(err) + } + return c } diff --git a/internal/cli/decision/cmd/add/testmain_test.go b/internal/cli/decision/cmd/add/testmain_test.go new file mode 100644 index 000000000..c895decf2 --- /dev/null +++ b/internal/cli/decision/cmd/add/testmain_test.go @@ -0,0 +1,23 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package add + +import ( + "os" + "testing" + + "github.com/ActiveMemory/ctx/internal/assets/read/lookup" +) + +// TestMain initialises the embedded asset lookup before any +// test runs. Tests that exercise error paths through +// internal/err/cli depend on desc.Text returning the parsed +// format strings rather than the empty default. +func TestMain(m *testing.M) { + lookup.Init() + os.Exit(m.Run()) +} diff --git a/internal/cli/learning/cmd/add/add_test.go b/internal/cli/learning/cmd/add/add_test.go index 6ede31fa9..f81666fc8 100644 --- a/internal/cli/learning/cmd/add/add_test.go +++ b/internal/cli/learning/cmd/add/add_test.go @@ -71,7 +71,10 @@ func TestLearningAdd(t *testing.T) { } // TestLearningAddRequiresFlags verifies that omitting -// required flags produces an error. +// required body flags produces an error. The placeholder +// check fires first (PreRunE runs before cobra's required- +// flag validation), so the message names the first empty +// body flag rather than --session-id. func TestLearningAddRequiresFlags(t *testing.T) { tmpDir, err := os.MkdirTemp("", "cli-learning-add-req-*") if err != nil { @@ -99,8 +102,92 @@ func TestLearningAddRequiresFlags(t *testing.T) { if err == nil { t.Fatal("expected error when adding learning without required flags") } - if !strings.Contains(err.Error(), "--session-id") { - t.Errorf("error should mention missing --session-id flag: %v", err) + if !strings.Contains(err.Error(), "--context") { + t.Errorf("error should mention missing --context flag: %v", err) + } +} + +// TestLearningAddRejectsPlaceholderLesson verifies placeholder +// rejection on --lesson. +func TestLearningAddRejectsPlaceholderLesson(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cli-learning-add-ph-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + origDir, _ := os.Getwd() + if err = os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + testctx.Declare(t, tmpDir) + + initCmd := initialize.Cmd() + initCmd.SetArgs([]string{}) + if err = initCmd.Execute(); err != nil { + t.Fatalf("init failed: %v", err) + } + + addCmd := Cmd() + addCmd.SetArgs([]string{ + "Learning body", + "--session-id", "test1234", + "--branch", "main", + "--commit", "abc123", + "--context", "real context", + "--lesson", "see chat", + "--application", "real application", + }) + err = addCmd.Execute() + if err == nil { + t.Fatal("expected placeholder rejection for --lesson=\"see chat\"") + } + if !strings.Contains(err.Error(), "lesson") { + t.Errorf("error should name --lesson: %v", err) + } + if !strings.Contains(err.Error(), "placeholder") { + t.Errorf("error should explain placeholder rejection: %v", err) + } +} + +// TestLearningAddAcceptsSubstringMatchingPlaceholder verifies +// that values containing a placeholder word as a substring are +// NOT rejected (only exact whole-value matches are placeholders). +func TestLearningAddAcceptsSubstringMatchingPlaceholder(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cli-learning-add-substr-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + origDir, _ := os.Getwd() + if err = os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + testctx.Declare(t, tmpDir) + + initCmd := initialize.Cmd() + initCmd.SetArgs([]string{}) + if err = initCmd.Execute(); err != nil { + t.Fatalf("init failed: %v", err) + } + + addCmd := Cmd() + addCmd.SetArgs([]string{ + "Learning body", + "--session-id", "test1234", + "--branch", "main", + "--commit", "abc123", + "--context", "we left this as TBD originally then resolved it", + "--lesson", "real lesson", + "--application", "real application", + }) + if err = addCmd.Execute(); err != nil { + t.Errorf("substring match should be accepted: %v", err) } } diff --git a/internal/cli/learning/cmd/add/cmd.go b/internal/cli/learning/cmd/add/cmd.go index 49ca03f20..ec3dd42d3 100644 --- a/internal/cli/learning/cmd/add/cmd.go +++ b/internal/cli/learning/cmd/add/cmd.go @@ -10,18 +10,29 @@ import ( "github.com/spf13/cobra" "github.com/ActiveMemory/ctx/internal/cli/add/core/build" + "github.com/ActiveMemory/ctx/internal/cli/add/core/validate" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" "github.com/ActiveMemory/ctx/internal/config/entry" + cFlag "github.com/ActiveMemory/ctx/internal/config/flag" ) // Cmd returns the "ctx learning add" subcommand. // // Adds a new learning entry to LEARNINGS.md with the // required provenance, context, lesson, and application -// flags. Implementation lives in the shared add core. +// flags. Implementation lives in the shared add core; this +// noun-level constructor additionally enforces that the +// three body flags are present and non-placeholder. // // Returns: // - *cobra.Command: Configured learning add subcommand func Cmd() *cobra.Command { - return build.Cmd(entry.Learning, cmd.DescKeyLearningAdd, cmd.UseLearningAdd) + c := build.Cmd(entry.Learning, cmd.DescKeyLearningAdd, cmd.UseLearningAdd) + if err := validate.RequireBodyFlags( + c, cFlag.Context, cFlag.Lesson, cFlag.Application, + ); err != nil { + // Programming error — see decision/cmd/add for rationale. + panic(err) + } + return c } diff --git a/internal/cli/learning/cmd/add/testmain_test.go b/internal/cli/learning/cmd/add/testmain_test.go new file mode 100644 index 000000000..c895decf2 --- /dev/null +++ b/internal/cli/learning/cmd/add/testmain_test.go @@ -0,0 +1,23 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package add + +import ( + "os" + "testing" + + "github.com/ActiveMemory/ctx/internal/assets/read/lookup" +) + +// TestMain initialises the embedded asset lookup before any +// test runs. Tests that exercise error paths through +// internal/err/cli depend on desc.Text returning the parsed +// format strings rather than the empty default. +func TestMain(m *testing.M) { + lookup.Init() + os.Exit(m.Run()) +} diff --git a/internal/config/embed/text/err_validate.go b/internal/config/embed/text/err_validate.go index 06606a798..4bfbe3707 100644 --- a/internal/config/embed/text/err_validate.go +++ b/internal/config/embed/text/err_validate.go @@ -38,4 +38,17 @@ const ( // DescKeyErrValidateWorkingDirectory is the text key for err validate working // directory messages. DescKeyErrValidateWorkingDirectory = "err.validation.working-directory" + // DescKeyErrValidateFlagEmpty is the text key for an empty or + // whitespace-only required body flag. + DescKeyErrValidateFlagEmpty = "err.validation.flag-empty" + // DescKeyErrValidateFlagPlaceholder is the text key for a + // placeholder-valued required body flag. + DescKeyErrValidateFlagPlaceholder = "err.validation.flag-placeholder" + // DescKeyErrValidateFlagUnregistered is the text key for a + // programming-error case where RequireBodyFlags is asked to + // require a flag that does not exist on the command. + DescKeyErrValidateFlagUnregistered = "err.validation.flag-unregistered" + // DescKeyErrValidateMarkRequired is the text key for a failure + // when calling cobra.Command.MarkFlagRequired. + DescKeyErrValidateMarkRequired = "err.validation.mark-required" ) diff --git a/internal/config/validate/doc.go b/internal/config/validate/doc.go new file mode 100644 index 000000000..394c12d00 --- /dev/null +++ b/internal/config/validate/doc.go @@ -0,0 +1,17 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Package validate holds constants used by the +// internal/cli/add/core/validate layer that enforces +// noun-specific body-flag contracts on add subcommands. +// +// The closed set of placeholder values rejected from +// required body flags lives here so call sites stay free +// of magic strings and so the policy can be reviewed in +// one place. Comparison against this set is exact and +// case-insensitive after whitespace trimming; substring +// matches are explicitly allowed. +package validate diff --git a/internal/config/validate/placeholder.go b/internal/config/validate/placeholder.go new file mode 100644 index 000000000..1409f4651 --- /dev/null +++ b/internal/config/validate/placeholder.go @@ -0,0 +1,48 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package validate + +// Placeholder values rejected from required body flags on +// ctx decision add / ctx learning add. Matching is exact, +// case-insensitive, after whitespace trimming. Substring +// matches are allowed (a real sentence may legitimately +// contain the word "TBD" inside longer prose). +const ( + // PlaceholderTBD matches the canonical "to be defined" marker. + PlaceholderTBD = "tbd" + // PlaceholderNA matches the "not applicable" marker (slash form). + PlaceholderNA = "n/a" + // PlaceholderNAShort matches the "not applicable" marker (compact). + PlaceholderNAShort = "na" + // PlaceholderNone matches a bare "none" answer. + PlaceholderNone = "none" + // PlaceholderSeeChat matches deferral to chat transcript. + PlaceholderSeeChat = "see chat" + // PlaceholderSeeAbove matches deferral to an earlier passage. + PlaceholderSeeAbove = "see above" + // PlaceholderSeeBelow matches deferral to a later passage. + PlaceholderSeeBelow = "see below" + // PlaceholderPending matches a "will be filled in later" marker. + PlaceholderPending = "pending" + // PlaceholderToBeDone matches the long form of the TBD marker. + PlaceholderToBeDone = "to be done" +) + +// Placeholders is the closed set used by the rejection check. +// Keys are stored lowercase; callers must lowercase the trimmed +// input before lookup. +var Placeholders = map[string]struct{}{ + PlaceholderTBD: {}, + PlaceholderNA: {}, + PlaceholderNAShort: {}, + PlaceholderNone: {}, + PlaceholderSeeChat: {}, + PlaceholderSeeAbove: {}, + PlaceholderSeeBelow: {}, + PlaceholderPending: {}, + PlaceholderToBeDone: {}, +} diff --git a/internal/err/cli/cli.go b/internal/err/cli/cli.go index 4c8d2dc40..1bcaff8c8 100644 --- a/internal/err/cli/cli.go +++ b/internal/err/cli/cli.go @@ -79,3 +79,67 @@ func NoToolSpecified() error { desc.Text(text.DescKeyErrCliNoToolSpecified), ) } + +// FlagEmpty returns an error for a required body flag that was +// empty or whitespace-only after trimming. +// +// Parameters: +// - name: the flag name (without leading dashes) +// +// Returns: +// - error: "flag -- is required and cannot be empty or +// whitespace-only" +func FlagEmpty(name string) error { + return fmt.Errorf( + desc.Text(text.DescKeyErrValidateFlagEmpty), name, + ) +} + +// FlagPlaceholder returns an error for a required body flag whose +// value matched the closed placeholder set. +// +// Parameters: +// - name: the flag name (without leading dashes) +// - value: the offending value, included verbatim in the message +// +// Returns: +// - error: "flag -- rejected placeholder value ; +// provide concrete content" +func FlagPlaceholder(name, value string) error { + return fmt.Errorf( + desc.Text(text.DescKeyErrValidateFlagPlaceholder), + name, value, + ) +} + +// FlagUnregistered returns an error for a programming-time mistake +// where a body-flag enforcement helper is asked to require a flag +// that the command does not register. +// +// Parameters: +// - flag: the unregistered flag name +// - cmd: the command name +// +// Returns: +// - error: "flag not registered on command " +func FlagUnregistered(flag, cmd string) error { + return fmt.Errorf( + desc.Text(text.DescKeyErrValidateFlagUnregistered), + flag, cmd, + ) +} + +// MarkRequiredFailed wraps a failure from cobra's MarkFlagRequired. +// +// Parameters: +// - flag: the flag name +// - cause: the underlying cobra error +// +// Returns: +// - error: "mark flag required: " +func MarkRequiredFailed(flag string, cause error) error { + return fmt.Errorf( + desc.Text(text.DescKeyErrValidateMarkRequired), + flag, cause, + ) +} diff --git a/specs/skill-surface-polish.md b/specs/skill-surface-polish.md new file mode 100644 index 000000000..7bf32e2e4 --- /dev/null +++ b/specs/skill-surface-polish.md @@ -0,0 +1,208 @@ +--- +title: Skill Surface Polish (Phase SK) +date: 2026-05-10 +status: ready +owner: jose +scope: CLI flag enforcement + skill file edits + doc additions +design-ref: ideas/002-editorial-pipeline-and-skill-rigor.md §3 "Reframing the wishy-washy skills" +phase: SK (prerequisite for Phase KB; independent of Phase RG) +--- + +# Spec: Skill Surface Polish (Phase SK) + +Tightens the existing capture skills to sibling-project rigor before +the editorial pipeline (Phase KB) lifts that pattern wholesale. +Independent of Phase RG; both can ship in parallel. + +## Problem + +Three concrete gaps in the current capture surface: + +1. **`ctx decision add` and `ctx learning add` accept missing body + fields.** Today the CLI binds `--context`, `--rationale`, + `--consequence` (decision) and `--context`, `--lesson`, + `--application` (learning) as optional strings. Users (and + agents) can submit decisions with no rationale at all, or with + placeholder values like `TBD` or `see chat`. The captured entry + is then lower-fidelity than the skill ceremony implies. + +2. **`/ctx-spec` skill has no `--brief ` flag.** The sibling + project ships a path-required spec contract that reads an + authoritative brief and skips the fresh-template Q&A. Without it, + every spec session re-runs interview questions even when the + brief already exists. + +3. **`/ctx-plan` skill does not offer to persist the debated brief.** + Adversarial-interview output evaporates at session end. The + sibling writes it to `.context/briefs/-.md` as the next + spec's authoritative source. + +Plus three smaller alignment items: + +4. Skill files lack an "Authority boundary (vs other skills)" + section, allowing silent promotion (handover→decision, + learning→convention) without explicit user ask. +5. "Light compression for clarity is allowed; new facts are not" + wording is inconsistent across capture skills. +6. The `--brief` contract is not documented in `docs/skills.md`. + +## Approach + +### CLI changes + +Add a small helper at `internal/cli/add/core/validate/required.go`: + +```go +// RequireBodyFlags marks the listed flags as cobra-required AND +// rejects placeholder values via a PreRunE wrapper. Placeholder +// values are matched case-insensitively against a closed set +// (TBD, see chat, n/a, etc.) plus whitespace-only. +func RequireBodyFlags(c *cobra.Command, flags ...string) error +``` + +Wire it in: + +- `internal/cli/decision/cmd/add/cmd.go` — require `--context`, + `--rationale`, `--consequence` after `build.Cmd(...)`. +- `internal/cli/learning/cmd/add/cmd.go` — require `--context`, + `--lesson`, `--application` after `build.Cmd(...)`. + +Cobra's `MarkFlagRequired` returns "required flag(s) not set" +errors and exits non-zero. Placeholder rejection returns a typed +error explaining which flag and which value pattern matched. + +### Skill changes + +- `/ctx-spec` (`internal/assets/claude/skills/ctx-spec/SKILL.md`): + add `--brief ` flag. When present, the skill reads the + file as authoritative source per the authority order + (frozen contracts > recorded decisions > debrief > agent + inference labeled `TBD`); skips the fresh template Q&A. + +- `/ctx-plan` (`.../ctx-plan/SKILL.md`): always offer to write the + debated brief to `.context/briefs/-.md` at the end of + an adversarial-interview pass (create `.context/briefs/` if + absent). + +- `/ctx-decision-add`, `/ctx-learning-add`, `/ctx-task-add`, + `/ctx-convention-add` (`.../ctx-*/SKILL.md`): add + "Authority boundary (vs other skills)" section explicitly listing + what each skill does **not** promote without explicit user ask. + +- Capture skills (decide / learn primarily; handover later in + Phase KB): standardize on the wording + *"Light compression for clarity is allowed; new facts are not."* + +### Documentation + +- `docs/skills.md` — document the `--brief` contract. + +## Behavior + +### Happy path + +- `ctx decision add --context X --rationale Y --consequence Z body` — + works as today. +- `ctx decision add body` — exits non-zero with cobra's + "required flag(s) ... not set" message. +- `ctx decision add --context TBD --rationale Y --consequence Z body` — + exits non-zero with placeholder-rejection error. +- `/ctx-spec --brief ideas/003-foo.md` — reads the brief and skips + template Q&A. + +### Edge cases + +| Case | Expected | +|------|----------| +| `--context " "` (whitespace-only) | placeholder error | +| `--context "see chat"` | placeholder error | +| `--context "TBD"` (exact case) | placeholder error | +| `--context "tbd"` (lowercase) | placeholder error | +| `--context "rationale to be defined later"` (substring) | accepted — only exact-value matches rejected, not substrings | +| `/ctx-spec --brief nonexistent.md` | skill reports file-not-found and stops | +| `/ctx-plan` ending with no edits | still offers to write the brief | + +## Interface + +### CLI + +`ctx decision add` — same flags as today; now three are required +and reject placeholder values. + +`ctx learning add` — same. + +`/ctx-spec` skill — new flag-style argument `--brief `. + +### Helper + +```go +// in internal/cli/add/core/validate/ +func RequireBodyFlags(c *cobra.Command, flags ...string) error +``` + +## Implementation + +### Files to create + +| File | Purpose | +|------|---------| +| `internal/cli/add/core/validate/required.go` | `RequireBodyFlags` helper | +| `internal/cli/add/core/validate/doc.go` | package doc | +| `internal/cli/add/core/validate/required_test.go` | unit tests | + +### Files to modify + +| File | Change | +|------|--------| +| `internal/cli/decision/cmd/add/cmd.go` | call `RequireBodyFlags` | +| `internal/cli/learning/cmd/add/cmd.go` | call `RequireBodyFlags` | +| `internal/cli/decision/cmd/add/add_test.go` | tests for required + placeholder rejection | +| `internal/cli/learning/cmd/add/add_test.go` | same | +| `internal/assets/claude/skills/ctx-spec/SKILL.md` | `--brief` contract | +| `internal/assets/claude/skills/ctx-plan/SKILL.md` | brief-save offer | +| `internal/assets/claude/skills/ctx-decision-add/SKILL.md` | authority boundary | +| `internal/assets/claude/skills/ctx-learning-add/SKILL.md` | authority boundary | +| `internal/assets/claude/skills/ctx-task-add/SKILL.md` | authority boundary | +| `internal/assets/claude/skills/ctx-convention-add/SKILL.md` | authority boundary | +| `docs/skills.md` | document `--brief` contract | + +### Placeholder set + +Exact, case-insensitive matches plus whitespace-only: + +- `TBD` +- `tbd` +- `n/a`, `N/A`, `na` +- `see chat` +- `see above` +- `pending` +- whitespace-only (regex `^\s*$`) + +Substring matches are NOT placeholders (a legitimate value can +contain the word "TBD" inside a longer sentence). + +## Testing + +- Unit: `RequireBodyFlags` rejects each placeholder; accepts + legitimate strings; accepts strings that contain placeholder + substrings. +- Unit: cobra-level required-flag error fires when the flag is + missing entirely. +- Integration: `ctx decision add` + `ctx learning add` end-to-end + with each failure mode. +- No regression in existing add tests. + +## Non-Goals + +- Changing flag names, short forms, or ordering. +- Requiring body flags on `ctx task add` or `ctx convention add` + (they have different field semantics). +- Implementing Phase KB closeout/fold logic — that's tracked under + `specs/kb-editorial-pipeline.md`. +- Auto-rewriting old entries that already contain placeholder + values — only new entries are gated. + +## Open Questions + +None — all design decisions are pinned in `ideas/002` §3 and the +debated brief at `ideas/003`. From 55acbd815c57d2d86b8af06cdcab245387468a70 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 15:09:33 -0700 Subject: [PATCH 2/9] feat(skills): /ctx-spec --brief, authority boundaries, plan brief offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase SK tasks 3–7 — tighten the skill surface so the capture and design skills match the rigor of the sibling editorial pipeline. - /ctx-spec gains a --brief flag. When supplied, the skill reads the file as authoritative and skips the interactive Q&A. Authority order when sources disagree: frozen docs > recorded DECISIONS > the brief > agent inference (labeled TBD). Light compression for clarity is allowed; new facts are not. - /ctx-plan always offers to save the debated brief to .context/briefs/-.md after the interview. The brief is the canonical handoff to /ctx-spec --brief. - /ctx-decision-add, /ctx-learning-add, /ctx-task-add, /ctx-convention-add gain an "Authority boundary (vs other skills)" section. Each lists the cross-promotions it refuses to perform silently (learning→decision, learning→convention, casual remark→task, one-off choice→convention, etc.) so promotion only happens on explicit user ask. - The "light compression for clarity is allowed; new facts are not" wording is now standardized across the four capture skills; the same wording lands in /ctx-handover when Phase KB ships. - docs/reference/skills.md documents the --brief contract in the /ctx-spec entry, including the authority order. Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- .context/TASKS.md | 10 ++--- docs/reference/skills.md | 22 ++++++++++ .../claude/skills/ctx-convention-add/SKILL.md | 18 ++++++++ .../claude/skills/ctx-decision-add/SKILL.md | 20 +++++++++ .../claude/skills/ctx-learning-add/SKILL.md | 17 ++++++++ .../assets/claude/skills/ctx-plan/SKILL.md | 16 +++++++- .../assets/claude/skills/ctx-spec/SKILL.md | 41 ++++++++++++++++++- .../claude/skills/ctx-task-add/SKILL.md | 18 ++++++++ 8 files changed, 155 insertions(+), 7 deletions(-) diff --git a/.context/TASKS.md b/.context/TASKS.md index 42a788c21..59141cc2c 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -1721,11 +1721,11 @@ Tightens existing capture skills to sibling-project rigor before the editorial p - [x] Add `MarkFlagRequired` to `ctx decision add` for `--context`, `--rationale`, `--consequence`; reject placeholder values (`TBD`, `see chat`, whitespace-only) at CLI level - [x] Add `MarkFlagRequired` to `ctx learning add` for `--context`, `--lesson`, `--application`; same placeholder rejection -- [ ] Add `--brief ` flag to `/ctx-spec` skill: when present, read the file as authoritative source per the sibling's authority order (frozen contracts > recorded decisions > debrief > agent inference labeled `TBD`); skip the fresh template Q&A -- [ ] Update `/ctx-plan` skill to always offer to write the debated brief to `.context/briefs/-.md` at the end of an interview (creating `.context/briefs/` if absent) -- [ ] Add an `Authority boundary (vs other skills)` section to `/ctx-decision-add`, `/ctx-learning-add`, `/ctx-task-add`, `/ctx-convention-add` skill files (prevent silent promotion handover→decision, learning→convention, etc., without explicit user ask) -- [ ] Standardize "light compression for clarity is allowed; new facts are not" wording across capture skills (decide / learn primarily); same wording lands in `/ctx-handover` once Phase KB ships -- [ ] Document the `--brief` contract in `docs/skills.md` +- [x] Add `--brief ` flag to `/ctx-spec` skill: when present, read the file as authoritative source per the sibling's authority order (frozen contracts > recorded decisions > debrief > agent inference labeled `TBD`); skip the fresh template Q&A +- [x] Update `/ctx-plan` skill to always offer to write the debated brief to `.context/briefs/-.md` at the end of an interview (creating `.context/briefs/` if absent) +- [x] Add an `Authority boundary (vs other skills)` section to `/ctx-decision-add`, `/ctx-learning-add`, `/ctx-task-add`, `/ctx-convention-add` skill files (prevent silent promotion handover→decision, learning→convention, etc., without explicit user ask) +- [x] Standardize "light compression for clarity is allowed; new facts are not" wording across capture skills (decide / learn primarily); same wording lands in `/ctx-handover` once Phase KB ships +- [x] Document the `--brief` contract in `docs/skills.md` (landed in `docs/reference/skills.md` — the actual location) ### Phase RG: Require Git as Architectural Precondition (Phase 0b; prerequisite for Phase KB) `#priority:high #added:2026-05-09` diff --git a/docs/reference/skills.md b/docs/reference/skills.md index a829291a0..1532ab448 100644 --- a/docs/reference/skills.md +++ b/docs/reference/skills.md @@ -481,8 +481,30 @@ optionally chains to `/ctx-task-add` **Trigger phrases**: "spec this out", "write a spec", "create a spec", "design document" +#### `--brief ` flag + +When invoked as `/ctx-spec --brief `, the skill treats the +file at `` as the authoritative source and skips the +interactive Q&A. Use this when a prior `/ctx-plan` session +produced a debated brief that already covers the design. + +The skill enforces this **authority order** when sources disagree: + +1. Frozen contracts in `docs/` (release notes, public CLI docs) +2. Recorded decisions in `.context/DECISIONS.md` +3. The brief at `` +4. Agent inference — only when 1–3 are silent, and labeled `TBD` + in the resulting spec so it stands out for review. + +Light compression for clarity is allowed; new facts are not. +Where the brief is silent, the spec writes `TBD` rather than +filling the gap from inference. If the brief contradicts a +frozen contract, the contradiction is surfaced to the user +rather than silently followed. + **See also**: [`/ctx-brainstorm`](#ctx-brainstorm), +[`/ctx-plan`](#ctx-plan), [`/ctx-plan-import`](#ctx-plan-import) --- diff --git a/internal/assets/claude/skills/ctx-convention-add/SKILL.md b/internal/assets/claude/skills/ctx-convention-add/SKILL.md index 8d1dec33c..6fe16f3ba 100644 --- a/internal/assets/claude/skills/ctx-convention-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-convention-add/SKILL.md @@ -54,6 +54,24 @@ ctx convention add "Colocate test files with implementation (*_test.go next to * If no `--section` is provided, the convention is appended to the end of the file. Prefer specifying a section for organization. +## Authority boundary (vs other skills) + +This skill records standardised patterns the project follows. It +does not unilaterally promote material from adjacent skills: + +- **Do not promote a one-off choice into a convention.** A single + decision with rationale is `/ctx-decision-add`'s territory; a + convention requires the pattern to recur and warrant codifying. + Ask before generalising. +- **Do not promote a learning into a convention.** "Always do X + going forward" is a convention; "we got bitten by Y" is a + learning. The user must explicitly say "codify that" before + the cross-promotion happens. +- **Do not duplicate.** If a similar rule already exists, surface + it and ask whether to update or add alongside. + +Light compression for clarity is allowed; new facts are not. + ## Quality Checklist Before recording, verify: diff --git a/internal/assets/claude/skills/ctx-decision-add/SKILL.md b/internal/assets/claude/skills/ctx-decision-add/SKILL.md index 26b185ca3..0934c36a8 100644 --- a/internal/assets/claude/skills/ctx-decision-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-decision-add/SKILL.md @@ -94,6 +94,26 @@ ctx decision add "Use PostgreSQL for primary database" \ --consequence "Single database handles transactions and search. Team needs PostgreSQL-specific training." ``` +## Authority boundary (vs other skills) + +This skill records architectural decisions — moments where a +trade-off between alternatives was deliberately resolved. It does +not unilaterally promote material from adjacent skills: + +- **Do not promote a learning into a decision.** A gotcha or + debugging insight is a learning; if the user wants it elevated + to a decision, they must say so. Pattern-cross-promotion drifts + the file's authority over time. +- **Do not promote a handover or wrap-up note into a decision.** + Session-end summaries can mention decisions, but those decisions + must have been captured at the time they were made. Backfilling + silently rewrites the trade-off record. +- **Do not invent alternatives.** If the user did not consider an + alternative, do not fabricate one to fill the section. Ask, or + use the Y-statement format that does not require alternatives. + +Light compression for clarity is allowed; new facts are not. + ## Quality Checklist Before recording, verify: diff --git a/internal/assets/claude/skills/ctx-learning-add/SKILL.md b/internal/assets/claude/skills/ctx-learning-add/SKILL.md index f281efdb7..996b325f1 100644 --- a/internal/assets/claude/skills/ctx-learning-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-learning-add/SKILL.md @@ -79,6 +79,23 @@ ctx learning add "ctx init overwrites user content without guard" \ --application "Skip existing files by default, only overwrite with --force" ``` +## Authority boundary (vs other skills) + +This skill records principle-level lessons discovered through real +work. It does not unilaterally promote material from adjacent skills: + +- **Do not promote a learning into a convention.** A learning is + "this gotcha cost us time" — generalising it into "we always do + X" is `/ctx-convention-add`'s job and requires explicit user ask. +- **Do not promote a learning into a decision.** Even when the + lesson clarifies a trade-off, the trade-off itself belongs in + `/ctx-decision-add` if the user wants it elevated. +- **Do not record general programming knowledge.** Anything + Googleable in five minutes is not a learning for this codebase + (the "Before Recording" check enforces this). + +Light compression for clarity is allowed; new facts are not. + ## Quality Checklist Before recording, verify: diff --git a/internal/assets/claude/skills/ctx-plan/SKILL.md b/internal/assets/claude/skills/ctx-plan/SKILL.md index f41a58687..9db1d560a 100644 --- a/internal/assets/claude/skills/ctx-plan/SKILL.md +++ b/internal/assets/claude/skills/ctx-plan/SKILL.md @@ -52,4 +52,18 @@ Stop when the user can describe, without your help: - the cheapest way to validate the bet - what becomes expensive to unwind -Then offer to write the debated brief. +## Always offer to save the debated brief + +After the interview concludes, always offer to write the debated +brief to `.context/briefs/-.md` (create `.context/briefs/` +if absent). The brief is the canonical handoff to `/ctx-spec +--brief ` and the next session's starting point. + +The brief is not a paraphrase of the conversation. It is a +written record of the *bet, the rejections, the failure modes, +the validation route, and the unwind cost* — in the user's +words, lightly compressed for clarity. New facts are not added. + +If the user declines to save, do not push. The bet still lives +in their head; the brief is for the next session, and they may +not need one. diff --git a/internal/assets/claude/skills/ctx-spec/SKILL.md b/internal/assets/claude/skills/ctx-spec/SKILL.md index d5fb8cadc..d56073369 100644 --- a/internal/assets/claude/skills/ctx-spec/SKILL.md +++ b/internal/assets/claude/skills/ctx-spec/SKILL.md @@ -26,9 +26,48 @@ each section with the user to produce a complete design document. /ctx-spec /ctx-spec (session checkpointing) /ctx-spec (rss feed generation) +/ctx-spec --brief ideas/003-editorial-pipeline-debated-brief.md ``` -## Process +## --brief contract + +When invoked with `--brief `, the skill treats the file at +`` as the authoritative source and skips the fresh-template +Q&A. Two preconditions and an authority order govern the read: + +**Preconditions** + +- The brief file must exist; if it does not, stop and report the + missing path without falling back to the interactive flow. +- The brief file should be the output of a prior `/ctx-plan` + session or a hand-written equivalent. A casual idea note is not + a brief. + +**Authority order** when the brief, recorded decisions, frozen +docs, or your inference disagree: + +1. Frozen contracts in `docs/` (release notes, public CLI docs) +2. Recorded decisions in `.context/DECISIONS.md` +3. The brief at `` +4. Your own inference — only when steps 1–3 are silent, and + labeled `TBD` in the spec so it stands out for review. + +Never invert this order. If the brief contradicts a frozen +contract, surface the contradiction to the user; do not silently +follow the brief. + +**Flow when `--brief` is set** + +1. Read the brief in full. Do not paraphrase it back to the user. +2. Read `specs/tpl/spec-template.md` to get the section list. +3. For each template section, lift content from the brief + verbatim where the brief speaks to it. Light compression for + clarity is allowed; new facts are not. +4. Where the brief is silent, write `TBD` rather than inventing. +5. Write the spec to `specs/{feature-name}.md` and surface the + `TBD` entries for the user to fill in next. + +## Process (interactive, when `--brief` is absent) ### 1. Gather the Feature Name diff --git a/internal/assets/claude/skills/ctx-task-add/SKILL.md b/internal/assets/claude/skills/ctx-task-add/SKILL.md index 97a53f988..7c464284e 100644 --- a/internal/assets/claude/skills/ctx-task-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-task-add/SKILL.md @@ -85,6 +85,24 @@ ctx task add "Authentication" # That's a topic, not a task # Also bad: missing --session-id, --branch, --commit ``` +## Authority boundary (vs other skills) + +This skill records actionable follow-up work. It does not +unilaterally promote material from adjacent skills: + +- **Do not promote a casual "we should..." into a task.** If the + user hasn't agreed it's worth tracking, ask before recording. + Speculative TODOs clutter the file and degrade everyone's trust + in it. +- **Do not duplicate.** If the user describes work already covered + by an open task (even loosely), reference the existing task + instead of adding a near-duplicate. Drift accumulates fast here. +- **Do not silently promote a decision or learning into a task.** + "We should write this up" is a different ask from "track this + work item"; route to the correct skill. + +Light compression for clarity is allowed; new facts are not. + ## Quality Checklist Before recording, verify: From 92507039c504025ccfbd41e1cb5ea105f1f08fa3 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 15:46:44 -0700 Subject: [PATCH 3/9] refactor(validate): single PreRunE enforcement, no panic, no swallowed errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier f32c8fd9 wired RequireBodyFlags via panic; the first attempt at fixing it (in the prior amend of this commit) replaced the panic with `_ = c.MarkFlagRequired(name)` and `value, _ := cmd.Flags().GetString(name)` — both silent error discards. That violates the project's "handle every error" convention (existing `_ =` discards elsewhere in the codebase are tech debt, not authorisation to add more). This refactor takes the right approach: PreRunE is the single enforcement point. Cobra defaults string flags to "", so the empty-value check catches missing flags through the same code path as placeholder rejection. MarkFlagRequired is dropped entirely (its only contribution was a "(required)" help-text annotation, which is redundant when PreRunE already enforces). GetString's error is propagated, not swallowed. Changes from f32c8fd9: - RequireBodyFlags returns nothing, calls no MarkFlagRequired, handles GetString error explicitly via `if getErr != nil`. - decision/cmd/add and learning/cmd/add stop panicking. - internal/err/cli loses FlagUnregistered and MarkRequiredFailed (unused after this change), plus their DescKeys and yaml entries. - Hook-driven spell corrections (generalising → generalizing, standardised → standardized) accepted in the same two Authority Boundary blocks. Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- .../claude/skills/ctx-convention-add/SKILL.md | 4 +- .../claude/skills/ctx-learning-add/SKILL.md | 2 +- internal/assets/commands/text/errors.yaml | 4 - internal/cli/add/core/validate/doc.go | 34 ++++---- internal/cli/add/core/validate/required.go | 40 ++++----- .../cli/add/core/validate/required_test.go | 84 ++++++------------- internal/cli/decision/cmd/add/cmd.go | 9 +- internal/cli/learning/cmd/add/cmd.go | 7 +- internal/config/embed/text/err_validate.go | 7 -- internal/err/cli/cli.go | 32 ------- 10 files changed, 69 insertions(+), 154 deletions(-) diff --git a/internal/assets/claude/skills/ctx-convention-add/SKILL.md b/internal/assets/claude/skills/ctx-convention-add/SKILL.md index 6fe16f3ba..9fad001fc 100644 --- a/internal/assets/claude/skills/ctx-convention-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-convention-add/SKILL.md @@ -56,13 +56,13 @@ of the file. Prefer specifying a section for organization. ## Authority boundary (vs other skills) -This skill records standardised patterns the project follows. It +This skill records standardized patterns the project follows. It does not unilaterally promote material from adjacent skills: - **Do not promote a one-off choice into a convention.** A single decision with rationale is `/ctx-decision-add`'s territory; a convention requires the pattern to recur and warrant codifying. - Ask before generalising. + Ask before generalizing. - **Do not promote a learning into a convention.** "Always do X going forward" is a convention; "we got bitten by Y" is a learning. The user must explicitly say "codify that" before diff --git a/internal/assets/claude/skills/ctx-learning-add/SKILL.md b/internal/assets/claude/skills/ctx-learning-add/SKILL.md index 996b325f1..ba30d7167 100644 --- a/internal/assets/claude/skills/ctx-learning-add/SKILL.md +++ b/internal/assets/claude/skills/ctx-learning-add/SKILL.md @@ -85,7 +85,7 @@ This skill records principle-level lessons discovered through real work. It does not unilaterally promote material from adjacent skills: - **Do not promote a learning into a convention.** A learning is - "this gotcha cost us time" — generalising it into "we always do + "this gotcha cost us time" — generalizing it into "we always do X" is `/ctx-convention-add`'s job and requires explicit user ask. - **Do not promote a learning into a decision.** Even when the lesson clarifies a trade-off, the trade-off itself belongs in diff --git a/internal/assets/commands/text/errors.yaml b/internal/assets/commands/text/errors.yaml index 939b6bfc7..fa57f3503 100644 --- a/internal/assets/commands/text/errors.yaml +++ b/internal/assets/commands/text/errors.yaml @@ -578,10 +578,6 @@ err.validation.flag-empty: short: 'flag --%s is required and cannot be empty or whitespace-only' err.validation.flag-placeholder: short: 'flag --%s rejected placeholder value %q; provide concrete content' -err.validation.flag-unregistered: - short: 'flag %q not registered on command %q' -err.validation.mark-required: - short: 'mark flag %q required: %w' err.hub.generate-token: short: 'generate token: %w' err.hub.internal: diff --git a/internal/cli/add/core/validate/doc.go b/internal/cli/add/core/validate/doc.go index 2eafc4e01..e10932e8e 100644 --- a/internal/cli/add/core/validate/doc.go +++ b/internal/cli/add/core/validate/doc.go @@ -4,22 +4,26 @@ // \ Copyright 2026-present Context contributors. // SPDX-License-Identifier: Apache-2.0 -// Package validate enforces noun-specific body-flag contracts on -// add subcommands. It centralises two pieces of policy: +// Package validate enforces body-flag contracts on add +// subcommands at PreRunE time. It centralises one policy: // -// - Required flags: noun-level commands declare which body -// flags must be present. RequireBodyFlags wires -// [cobra.Command.MarkFlagRequired] for each so cobra rejects -// missing flags before RunE runs. +// - A closed set of placeholder values (TBD, see chat, +// n/a, etc., plus whitespace-only) is rejected with a +// clear error naming the flag and the offending value. +// Cobra defaults string flags to "", which the same +// empty-value check rejects — so omitting a required +// body flag and supplying a placeholder fail through +// the same code path. Substring matches are not +// treated as placeholders so legitimate prose +// containing the word "TBD" still passes. // -// - Placeholder rejection: a closed set of placeholder values -// (TBD, see chat, n/a, etc., plus whitespace-only) is rejected -// at PreRunE time with a clear error naming the flag and the -// offending value. Substring matches are not treated as -// placeholders so legitimate prose containing the word "TBD" -// still passes. +// The package does not call [cobra.Command.MarkFlagRequired]: +// PreRunE is the single enforcement point, and routing +// missing-flag and placeholder-flag through the same path +// avoids both the discarded-error wart on MarkFlagRequired +// and the divergent error messages it would produce. // -// The package is internal to the add core; noun-level subcommands -// (decision, learning) call RequireBodyFlags after constructing -// their command via [build.Cmd]. +// The package is internal to the add core; noun-level +// subcommands (decision, learning) call [RequireBodyFlags] +// after constructing their command via [build.Cmd]. package validate diff --git a/internal/cli/add/core/validate/required.go b/internal/cli/add/core/validate/required.go index 1a2a8f716..14755a05c 100644 --- a/internal/cli/add/core/validate/required.go +++ b/internal/cli/add/core/validate/required.go @@ -15,34 +15,29 @@ import ( errCli "github.com/ActiveMemory/ctx/internal/err/cli" ) -// RequireBodyFlags marks each named flag as cobra-required and -// wraps the command's PreRunE to reject placeholder values -// (TBD, see chat, n/a, etc., plus whitespace-only) on those -// flags. Existing PreRunE is preserved and runs after the -// placeholder check. +// RequireBodyFlags wraps the command's PreRunE so each named flag +// is read and rejected when its value is empty, whitespace-only, +// or matches the closed placeholder set (TBD, see chat, n/a, +// etc.). Existing PreRunE is preserved and runs after the check. +// +// The check is the single enforcement point: there is no +// [cobra.Command.MarkFlagRequired] call, so help text does not +// gain a "(required)" annotation. Cobra defaults string flags to +// the empty string, which the empty check rejects with a clear +// message — making the marker redundant and the discarded error +// it returns avoidable. // // Parameters: // - c: cobra command to mutate -// - flags: names of body flags to require and policy-check -// -// Returns: -// - error: non-nil if any named flag does not exist on c, in -// which case the command is left unmodified -func RequireBodyFlags(c *cobra.Command, flags ...string) error { - for _, name := range flags { - if c.Flag(name) == nil { - return errCli.FlagUnregistered(name, c.Name()) - } - } - for _, name := range flags { - if markErr := c.MarkFlagRequired(name); markErr != nil { - return errCli.MarkRequiredFailed(name, markErr) - } - } +// - flags: names of body flags to read and policy-check +func RequireBodyFlags(c *cobra.Command, flags ...string) { prev := c.PreRunE c.PreRunE = func(cmd *cobra.Command, args []string) error { for _, name := range flags { - value, _ := cmd.Flags().GetString(name) + value, getErr := cmd.Flags().GetString(name) + if getErr != nil { + return getErr + } if rejectErr := RejectPlaceholder( name, value, ); rejectErr != nil { @@ -54,7 +49,6 @@ func RequireBodyFlags(c *cobra.Command, flags ...string) error { } return nil } - return nil } // RejectPlaceholder returns an error if value is a placeholder diff --git a/internal/cli/add/core/validate/required_test.go b/internal/cli/add/core/validate/required_test.go index 0b11c73d4..f6fef0045 100644 --- a/internal/cli/add/core/validate/required_test.go +++ b/internal/cli/add/core/validate/required_test.go @@ -67,55 +67,6 @@ func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { } } -func TestRequireBodyFlagsRejectsMissingFlag(t *testing.T) { - c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} - c.Flags().String("context", "", "") - err := RequireBodyFlags(c, "context", "nonexistent") - if err == nil { - t.Fatal("expected error for unregistered flag") - } -} - -func TestRequireBodyFlagsMarksRequired(t *testing.T) { - c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} - c.Flags().String("context", "", "") - c.Flags().String("rationale", "", "") - if err := RequireBodyFlags(c, "context", "rationale"); err != nil { - t.Fatalf("RequireBodyFlags error: %v", err) - } - // Missing --context and --rationale: PreRunE fires first and - // reports the empty value before cobra's required-flag check. - c.SetArgs([]string{}) - c.SetOut(&strings.Builder{}) - c.SetErr(&strings.Builder{}) - err := c.Execute() - if err == nil { - t.Fatal("expected error for missing required body flags") - } - if !strings.Contains(err.Error(), "context") { - t.Errorf("error should name the empty flag: %v", err) - } -} - -// TestRequireBodyFlagsCobraRequiredAnnotation verifies that -// MarkFlagRequired's annotation is set on each flag. The -// PreRunE shadow normally fires first; with PreRunE bypassed, -// cobra still rejects the missing flag at validateRequiredFlags. -func TestRequireBodyFlagsCobraRequiredAnnotation(t *testing.T) { - c := &cobra.Command{Use: "test", Run: func(_ *cobra.Command, _ []string) {}} - c.Flags().String("context", "", "") - if err := RequireBodyFlags(c, "context"); err != nil { - t.Fatalf("RequireBodyFlags error: %v", err) - } - flag := c.Flag("context") - if flag == nil { - t.Fatal("context flag missing") - } - if _, ok := flag.Annotations[cobra.BashCompOneRequiredFlag]; !ok { - t.Error("expected cobra required annotation on --context") - } -} - func TestRequireBodyFlagsRejectsPlaceholderViaPreRunE(t *testing.T) { ran := false c := &cobra.Command{ @@ -127,9 +78,7 @@ func TestRequireBodyFlagsRejectsPlaceholderViaPreRunE(t *testing.T) { } c.Flags().String("context", "", "") c.Flags().String("rationale", "", "") - if err := RequireBodyFlags(c, "context", "rationale"); err != nil { - t.Fatalf("RequireBodyFlags error: %v", err) - } + RequireBodyFlags(c, "context", "rationale") c.SetArgs([]string{ "--context", "TBD", "--rationale", "good reason", @@ -159,9 +108,7 @@ func TestRequireBodyFlagsAllowsLegitimateInput(t *testing.T) { } c.Flags().String("context", "", "") c.Flags().String("rationale", "", "") - if err := RequireBodyFlags(c, "context", "rationale"); err != nil { - t.Fatalf("RequireBodyFlags error: %v", err) - } + RequireBodyFlags(c, "context", "rationale") c.SetArgs([]string{ "--context", "real context", "--rationale", "real rationale", @@ -187,9 +134,7 @@ func TestRequireBodyFlagsPreservesExistingPreRunE(t *testing.T) { RunE: func(_ *cobra.Command, _ []string) error { return nil }, } c.Flags().String("context", "", "") - if err := RequireBodyFlags(c, "context"); err != nil { - t.Fatalf("RequireBodyFlags error: %v", err) - } + RequireBodyFlags(c, "context") c.SetArgs([]string{"--context", "legitimate"}) c.SetOut(&strings.Builder{}) c.SetErr(&strings.Builder{}) @@ -200,3 +145,26 @@ func TestRequireBodyFlagsPreservesExistingPreRunE(t *testing.T) { t.Error("existing PreRunE should still execute") } } + +// TestRequireBodyFlagsRejectsMissingFlag verifies that omitting +// the body flag entirely is rejected by PreRunE — cobra's default +// for string flags is "", which trips the empty-value check. No +// separate MarkFlagRequired is needed. +func TestRequireBodyFlagsRejectsMissingFlag(t *testing.T) { + c := &cobra.Command{ + Use: "test", + RunE: func(_ *cobra.Command, _ []string) error { return nil }, + } + c.Flags().String("context", "", "") + RequireBodyFlags(c, "context") + c.SetArgs([]string{}) + c.SetOut(&strings.Builder{}) + c.SetErr(&strings.Builder{}) + err := c.Execute() + if err == nil { + t.Fatal("expected rejection when --context is missing") + } + if !strings.Contains(err.Error(), "context") { + t.Errorf("error should name --context: %v", err) + } +} diff --git a/internal/cli/decision/cmd/add/cmd.go b/internal/cli/decision/cmd/add/cmd.go index d8a06e215..2bbfafa68 100644 --- a/internal/cli/decision/cmd/add/cmd.go +++ b/internal/cli/decision/cmd/add/cmd.go @@ -28,13 +28,8 @@ import ( // - *cobra.Command: Configured decision add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Decision, cmd.DescKeyDecisionAdd, cmd.UseDecisionAdd) - if err := validate.RequireBodyFlags( + validate.RequireBodyFlags( c, cFlag.Context, cFlag.Rationale, cFlag.Consequence, - ); err != nil { - // Programming error — every body flag we name must exist - // on the shared add command. Panic at command-construction - // time rather than silently shipping a half-wired command. - panic(err) - } + ) return c } diff --git a/internal/cli/learning/cmd/add/cmd.go b/internal/cli/learning/cmd/add/cmd.go index ec3dd42d3..a57472a15 100644 --- a/internal/cli/learning/cmd/add/cmd.go +++ b/internal/cli/learning/cmd/add/cmd.go @@ -28,11 +28,8 @@ import ( // - *cobra.Command: Configured learning add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Learning, cmd.DescKeyLearningAdd, cmd.UseLearningAdd) - if err := validate.RequireBodyFlags( + validate.RequireBodyFlags( c, cFlag.Context, cFlag.Lesson, cFlag.Application, - ); err != nil { - // Programming error — see decision/cmd/add for rationale. - panic(err) - } + ) return c } diff --git a/internal/config/embed/text/err_validate.go b/internal/config/embed/text/err_validate.go index 4bfbe3707..6ac29d635 100644 --- a/internal/config/embed/text/err_validate.go +++ b/internal/config/embed/text/err_validate.go @@ -44,11 +44,4 @@ const ( // DescKeyErrValidateFlagPlaceholder is the text key for a // placeholder-valued required body flag. DescKeyErrValidateFlagPlaceholder = "err.validation.flag-placeholder" - // DescKeyErrValidateFlagUnregistered is the text key for a - // programming-error case where RequireBodyFlags is asked to - // require a flag that does not exist on the command. - DescKeyErrValidateFlagUnregistered = "err.validation.flag-unregistered" - // DescKeyErrValidateMarkRequired is the text key for a failure - // when calling cobra.Command.MarkFlagRequired. - DescKeyErrValidateMarkRequired = "err.validation.mark-required" ) diff --git a/internal/err/cli/cli.go b/internal/err/cli/cli.go index 1bcaff8c8..25f41d3ce 100644 --- a/internal/err/cli/cli.go +++ b/internal/err/cli/cli.go @@ -111,35 +111,3 @@ func FlagPlaceholder(name, value string) error { name, value, ) } - -// FlagUnregistered returns an error for a programming-time mistake -// where a body-flag enforcement helper is asked to require a flag -// that the command does not register. -// -// Parameters: -// - flag: the unregistered flag name -// - cmd: the command name -// -// Returns: -// - error: "flag not registered on command " -func FlagUnregistered(flag, cmd string) error { - return fmt.Errorf( - desc.Text(text.DescKeyErrValidateFlagUnregistered), - flag, cmd, - ) -} - -// MarkRequiredFailed wraps a failure from cobra's MarkFlagRequired. -// -// Parameters: -// - flag: the flag name -// - cause: the underlying cobra error -// -// Returns: -// - error: "mark flag required: " -func MarkRequiredFailed(flag string, cause error) error { - return fmt.Errorf( - desc.Text(text.DescKeyErrValidateMarkRequired), - flag, cause, - ) -} From 1156e44a6ff026d612148c1cc258ef3d691a45e0 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 16:14:59 -0700 Subject: [PATCH 4/9] docs(blog): align thought-piece bylines Standardise the author byline on thought-piece posts. Release recaps and first-person dev stories keep their existing byline. Signed-off-by: Jose Alekhinne --- docs/blog/2026-02-03-the-attention-budget.md | 4 ++-- docs/blog/2026-02-04-skills-that-fight-the-platform.md | 4 ++-- docs/blog/2026-02-05-you-cant-import-expertise.md | 4 ++-- docs/blog/2026-02-09-defense-in-depth-securing-ai-agents.md | 4 ++-- docs/blog/2026-02-12-how-deep-is-too-deep.md | 4 ++-- docs/blog/2026-02-14-irc-as-context.md | 2 +- docs/blog/2026-02-17-code-is-cheap-judgment-is-not.md | 4 ++-- docs/blog/2026-02-17-context-as-infrastructure.md | 4 ++-- ...el-agents-merge-debt-and-the-myth-of-overnight-progress.md | 4 ++-- docs/blog/2026-02-17-the-3-1-ratio.md | 4 ++-- .../blog/2026-02-17-when-a-system-starts-explaining-itself.md | 4 ++-- docs/blog/2026-02-25-the-homework-problem.md | 4 ++-- docs/blog/2026-02-28-the-last-question.md | 4 ++-- docs/blog/2026-03-04-agent-memory-is-infrastructure.md | 4 ++-- docs/blog/2026-03-23-we-broke-the-3-1-rule.md | 4 ++-- docs/blog/2026-04-02-code-structure-as-an-agent-interface.md | 4 ++-- docs/blog/2026-04-06-the-watermelon-rind-anti-pattern.md | 4 ++-- 17 files changed, 33 insertions(+), 33 deletions(-) diff --git a/docs/blog/2026-02-03-the-attention-budget.md b/docs/blog/2026-02-03-the-attention-budget.md index 0751ee251..bcb6cba50 100644 --- a/docs/blog/2026-02-03-the-attention-budget.md +++ b/docs/blog/2026-02-03-the-attention-budget.md @@ -1,7 +1,7 @@ --- title: "The Attention Budget: Why Your AI Forgets What You Just Told It" date: 2026-02-03 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - attention mechanics @@ -24,7 +24,7 @@ topics: ## Why Your AI Forgets What You Just Told It -*Jose Alekhinne / 2026-02-03* +*Volkan Özçelik / 2026-02-03* !!! question "Ever Wondered Why AI Gets Worse the Longer You Talk?" You paste a 2000-line file, explain the bug in detail, provide three diff --git a/docs/blog/2026-02-04-skills-that-fight-the-platform.md b/docs/blog/2026-02-04-skills-that-fight-the-platform.md index eeca176f6..377ed778a 100644 --- a/docs/blog/2026-02-04-skills-that-fight-the-platform.md +++ b/docs/blog/2026-02-04-skills-that-fight-the-platform.md @@ -1,7 +1,7 @@ --- title: "Skills That Fight the Platform" date: 2026-02-04 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - context engineering @@ -17,7 +17,7 @@ topics: ## When Your Custom Prompts Work against You -*Jose Alekhinne / 2026-02-04* +*Volkan Özçelik / 2026-02-04* !!! question "Have You Ever Written a Skill That Made Your AI Worse?" You craft detailed instructions. You add examples. You build elaborate diff --git a/docs/blog/2026-02-05-you-cant-import-expertise.md b/docs/blog/2026-02-05-you-cant-import-expertise.md index 879c6ef50..d603cb707 100644 --- a/docs/blog/2026-02-05-you-cant-import-expertise.md +++ b/docs/blog/2026-02-05-you-cant-import-expertise.md @@ -1,7 +1,7 @@ --- title: "You Can't Import Expertise" date: 2026-02-05 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - skill adaptation @@ -17,7 +17,7 @@ topics: ## Why Good Skills Can't Be Copy-Pasted -*Jose Alekhinne / 2026-02-05* +*Volkan Özçelik / 2026-02-05* !!! question "Have You Ever Dropped a Well-Crafted Template into a Project and Had It Do... Nothing Useful?" * The template was **thorough**, diff --git a/docs/blog/2026-02-09-defense-in-depth-securing-ai-agents.md b/docs/blog/2026-02-09-defense-in-depth-securing-ai-agents.md index 0855e5bfe..5fcb90130 100644 --- a/docs/blog/2026-02-09-defense-in-depth-securing-ai-agents.md +++ b/docs/blog/2026-02-09-defense-in-depth-securing-ai-agents.md @@ -1,7 +1,7 @@ --- title: "Defense in Depth: Securing AI Agents" date: 2026-02-09 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - agent security @@ -17,7 +17,7 @@ topics: ## When Markdown Is Not a Security Boundary -*Jose Alekhinne / 2026-02-09* +*Volkan Özçelik / 2026-02-09* !!! question "What Happens When Your AI Agent Runs Overnight and Nobody Is Watching?" It follows instructions: **That is the problem**. diff --git a/docs/blog/2026-02-12-how-deep-is-too-deep.md b/docs/blog/2026-02-12-how-deep-is-too-deep.md index e7f157ade..119a84f6f 100644 --- a/docs/blog/2026-02-12-how-deep-is-too-deep.md +++ b/docs/blog/2026-02-12-how-deep-is-too-deep.md @@ -1,7 +1,7 @@ --- title: "How Deep Is Too Deep?" date: 2026-02-12 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - AI foundations @@ -17,7 +17,7 @@ topics: ## When "Master ML" Is the Wrong Next Step -*Jose Alekhinne / 2026-02-12* +*Volkan Özçelik / 2026-02-12* !!! question "Have You Ever Felt like You Should Understand More of the Stack beneath You?" You can talk about transformers at a whiteboard. diff --git a/docs/blog/2026-02-14-irc-as-context.md b/docs/blog/2026-02-14-irc-as-context.md index ff0b07adb..affb2af8c 100644 --- a/docs/blog/2026-02-14-irc-as-context.md +++ b/docs/blog/2026-02-14-irc-as-context.md @@ -1,7 +1,7 @@ --- title: "Before Context Windows, We Had Bouncers" date: 2026-02-14 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - context engineering diff --git a/docs/blog/2026-02-17-code-is-cheap-judgment-is-not.md b/docs/blog/2026-02-17-code-is-cheap-judgment-is-not.md index b356ad609..05ecc2347 100644 --- a/docs/blog/2026-02-17-code-is-cheap-judgment-is-not.md +++ b/docs/blog/2026-02-17-code-is-cheap-judgment-is-not.md @@ -7,7 +7,7 @@ title: "Code Is Cheap. Judgment Is Not." date: 2026-02-17 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - AI and expertise @@ -23,7 +23,7 @@ topics: ## Why AI Replaces Effort, Not Expertise -*Jose Alekhinne / February 17, 2026* +*Volkan Özçelik / February 17, 2026* !!! question "Are You Worried about AI Taking Your Job?" You might be confusing the thing that's *cheap* with the diff --git a/docs/blog/2026-02-17-context-as-infrastructure.md b/docs/blog/2026-02-17-context-as-infrastructure.md index c0c20ac43..60212ba8b 100644 --- a/docs/blog/2026-02-17-context-as-infrastructure.md +++ b/docs/blog/2026-02-17-context-as-infrastructure.md @@ -7,7 +7,7 @@ title: "Context as Infrastructure" date: 2026-02-17 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - context engineering @@ -23,7 +23,7 @@ topics: ## Why Your AI Needs a Filesystem, Not a Prompt -*Jose Alekhinne / February 17, 2026* +*Volkan Özçelik / February 17, 2026* !!! question "Where Does Your AI's Knowledge Live between Sessions?" If the answer is "in a prompt I paste at the start," you are treating diff --git a/docs/blog/2026-02-17-parallel-agents-merge-debt-and-the-myth-of-overnight-progress.md b/docs/blog/2026-02-17-parallel-agents-merge-debt-and-the-myth-of-overnight-progress.md index 42ec905eb..9ad268b15 100644 --- a/docs/blog/2026-02-17-parallel-agents-merge-debt-and-the-myth-of-overnight-progress.md +++ b/docs/blog/2026-02-17-parallel-agents-merge-debt-and-the-myth-of-overnight-progress.md @@ -1,7 +1,7 @@ --- title: "Parallel Agents, Merge Debt, and the Myth of Overnight Progress" date: 2026-02-17 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - agent workflows @@ -17,7 +17,7 @@ topics: ## When the Screen Looks like Progress -*Jose Alekhinne / 2026-02-17* +*Volkan Özçelik / 2026-02-17* !!! question "How Many Terminals Are Too Many?" You discover agents can run in parallel. diff --git a/docs/blog/2026-02-17-the-3-1-ratio.md b/docs/blog/2026-02-17-the-3-1-ratio.md index 65f365b7a..e4d453897 100644 --- a/docs/blog/2026-02-17-the-3-1-ratio.md +++ b/docs/blog/2026-02-17-the-3-1-ratio.md @@ -7,7 +7,7 @@ title: "The 3:1 Ratio" date: 2026-02-17 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - consolidation @@ -23,7 +23,7 @@ topics: ## Scheduling Consolidation in AI Development -*Jose Alekhinne / February 17, 2026* +*Volkan Özçelik / February 17, 2026* !!! question "How Often Should You Stop Building and Start Cleaning?" Every developer knows technical debt exists. Every developer diff --git a/docs/blog/2026-02-17-when-a-system-starts-explaining-itself.md b/docs/blog/2026-02-17-when-a-system-starts-explaining-itself.md index 4030e9709..7df93ce76 100644 --- a/docs/blog/2026-02-17-when-a-system-starts-explaining-itself.md +++ b/docs/blog/2026-02-17-when-a-system-starts-explaining-itself.md @@ -7,7 +7,7 @@ title: "When a System Starts Explaining Itself" date: 2026-02-17 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - field notes @@ -23,7 +23,7 @@ topics: ## Field Notes from the Moment a Private Workflow Becomes Portable -*Jose Alekhinne / February 17, 2026* +*Volkan Özçelik / February 17, 2026* !!! question "How Do You Know Something Is Working?" **Not** from metrics. **Not** from GitHub stars. **Not** from praise. diff --git a/docs/blog/2026-02-25-the-homework-problem.md b/docs/blog/2026-02-25-the-homework-problem.md index 1c27f4636..a3c4254da 100644 --- a/docs/blog/2026-02-25-the-homework-problem.md +++ b/docs/blog/2026-02-25-the-homework-problem.md @@ -7,7 +7,7 @@ title: "The Dog Ate My Homework: Teaching AI Agents to Read Before They Write" date: 2026-02-25 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - hooks @@ -24,7 +24,7 @@ topics: ## Teaching AI Agents to Read Before They Write -*Jose Alekhinne / February 25, 2026* +*Volkan Özçelik / February 25, 2026* !!! question "Does Your AI Actually Read the Instructions?" You wrote the playbook. You organized the files. You even put diff --git a/docs/blog/2026-02-28-the-last-question.md b/docs/blog/2026-02-28-the-last-question.md index b6c33dd93..d0bf965bb 100644 --- a/docs/blog/2026-02-28-the-last-question.md +++ b/docs/blog/2026-02-28-the-last-question.md @@ -7,7 +7,7 @@ title: "The Last Question" date: 2026-02-28 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - context continuity @@ -23,7 +23,7 @@ topics: ## The System That Never Forgets -*Jose Alekhinne / February 28, 2026* +*Volkan Özçelik / February 28, 2026* !!! quote "The Origin" *"The last question was asked for the first time, half in jest..."* diff --git a/docs/blog/2026-03-04-agent-memory-is-infrastructure.md b/docs/blog/2026-03-04-agent-memory-is-infrastructure.md index d40daa54b..d08fe90aa 100644 --- a/docs/blog/2026-03-04-agent-memory-is-infrastructure.md +++ b/docs/blog/2026-03-04-agent-memory-is-infrastructure.md @@ -7,7 +7,7 @@ title: "Agent Memory Is Infrastructure" date: 2026-03-04 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - context engineering @@ -23,7 +23,7 @@ topics: ## The Problem Isn't Forgetting: It's Not Building Anything That Lasts. -*Jose Alekhinne / March 4, 2026* +*Volkan Özçelik / March 4, 2026* !!! question "A New Developer Joins Your Team Tomorrow and Clones the Repo: What Do They Know?" If the answer depends on which machine they're using, which diff --git a/docs/blog/2026-03-23-we-broke-the-3-1-rule.md b/docs/blog/2026-03-23-we-broke-the-3-1-rule.md index 7363848ca..0b2ee023d 100644 --- a/docs/blog/2026-03-23-we-broke-the-3-1-rule.md +++ b/docs/blog/2026-03-23-we-broke-the-3-1-rule.md @@ -7,7 +7,7 @@ title: "We Broke the 3:1 Rule" date: 2026-03-23 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - consolidation @@ -24,7 +24,7 @@ topics: **The best time to consolidate was after every third session. The second best time is now.** -*Jose Alekhinne / March 23, 2026* +*Volkan Özçelik / March 23, 2026* The rule was simple: three feature sessions, then one consolidation session. diff --git a/docs/blog/2026-04-02-code-structure-as-an-agent-interface.md b/docs/blog/2026-04-02-code-structure-as-an-agent-interface.md index d5ba41776..adacbc17c 100644 --- a/docs/blog/2026-04-02-code-structure-as-an-agent-interface.md +++ b/docs/blog/2026-04-02-code-structure-as-an-agent-interface.md @@ -8,7 +8,7 @@ title: "Code Structure as an Agent Interface: What 19 AST Tests Taught Us About Agent-Readable Code" date: 2026-04-02 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - ast @@ -29,7 +29,7 @@ against the millions of `strings.Split(s, "/")` calls in its training data and coast on statistical inference. It has to actually look up what `token.Slash` is.** -*Jose Alekhinne / April 2, 2026* +*Volkan Özçelik / April 2, 2026* ## How It Began diff --git a/docs/blog/2026-04-06-the-watermelon-rind-anti-pattern.md b/docs/blog/2026-04-06-the-watermelon-rind-anti-pattern.md index df7c36378..6a911019b 100644 --- a/docs/blog/2026-04-06-the-watermelon-rind-anti-pattern.md +++ b/docs/blog/2026-04-06-the-watermelon-rind-anti-pattern.md @@ -8,7 +8,7 @@ title: "The Watermelon-Rind Anti-Pattern: Why Smarter Tools Make Shallower Agents" date: 2026-04-06 -author: Jose Alekhinne +author: Volkan Özçelik reviewed_and_finalized: true topics: - architecture @@ -27,7 +27,7 @@ topics: **Give an agent a graph query tool, and it will tell you everything about your codebase except what actually matters.** -*Jose Alekhinne / April 6, 2026* +*Volkan Özçelik / April 6, 2026* ## A Turkish Proverb Walks into a Codebase From ba2faa548fa0e6578e7b2f6f7a0b5d78f580ed2b Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 16:40:33 -0700 Subject: [PATCH 5/9] refactor(validate): pure BodyFlags, no PreRunE decoration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous shape of validate.RequireBodyFlags(c, flags...) silently wrapped the caller's c.PreRunE, saving the prior hook and chaining to it. That is action-at-a-distance — no other helper in the codebase mutates a passed-in cobra.Command's hooks. The caller had no way to know their PreRunE was being decorated without reading the helper. Refactor: - RequireBodyFlags → BodyFlags. Pure function: validates each named flag's current value, returns an error on the first failure, leaves the command untouched. - decision/cmd/add and learning/cmd/add install their own PreRunE explicitly: c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { return validate.BodyFlags(cobraCmd, ...) } The wiring is visible at the call site. - Tests rewritten to call BodyFlags directly after parsing flags, asserting on pure-function behaviour (acceptance, placeholder rejection, missing-flag rejection via the empty check, first-failure ordering). Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- internal/cli/add/core/validate/doc.go | 38 +++--- internal/cli/add/core/validate/required.go | 47 +++---- .../cli/add/core/validate/required_test.go | 128 ++++++++---------- internal/cli/decision/cmd/add/cmd.go | 14 +- internal/cli/learning/cmd/add/cmd.go | 14 +- 5 files changed, 112 insertions(+), 129 deletions(-) diff --git a/internal/cli/add/core/validate/doc.go b/internal/cli/add/core/validate/doc.go index e10932e8e..0da7ed0b1 100644 --- a/internal/cli/add/core/validate/doc.go +++ b/internal/cli/add/core/validate/doc.go @@ -4,26 +4,26 @@ // \ Copyright 2026-present Context contributors. // SPDX-License-Identifier: Apache-2.0 -// Package validate enforces body-flag contracts on add -// subcommands at PreRunE time. It centralises one policy: +// Package validate is a pure-function policy layer for add +// subcommand body flags. // -// - A closed set of placeholder values (TBD, see chat, -// n/a, etc., plus whitespace-only) is rejected with a -// clear error naming the flag and the offending value. -// Cobra defaults string flags to "", which the same -// empty-value check rejects — so omitting a required -// body flag and supplying a placeholder fail through -// the same code path. Substring matches are not -// treated as placeholders so legitimate prose -// containing the word "TBD" still passes. +// It exports two functions: // -// The package does not call [cobra.Command.MarkFlagRequired]: -// PreRunE is the single enforcement point, and routing -// missing-flag and placeholder-flag through the same path -// avoids both the discarded-error wart on MarkFlagRequired -// and the divergent error messages it would produce. +// - [BodyFlags] reads each named flag from a cobra command and +// returns an error if any value is empty, whitespace-only, +// or matches the closed placeholder set (TBD, see chat, n/a, +// etc.). It does not mutate the command. // -// The package is internal to the add core; noun-level -// subcommands (decision, learning) call [RequireBodyFlags] -// after constructing their command via [build.Cmd]. +// - [RejectPlaceholder] is the per-value primitive used by +// [BodyFlags] and is exported for tests and ad-hoc reuse. +// +// Cobra defaults string flags to "", so the empty-value check +// catches missing flags through the same code path as +// placeholder rejection. Substring matches are not placeholders +// — legitimate prose containing the word "TBD" still passes. +// +// Noun-level add subcommands (decision, learning) invoke +// [BodyFlags] from their own PreRunE so the wiring is visible +// at the call site. The package does not register hooks on the +// caller's command. package validate diff --git a/internal/cli/add/core/validate/required.go b/internal/cli/add/core/validate/required.go index 14755a05c..41f1335ee 100644 --- a/internal/cli/add/core/validate/required.go +++ b/internal/cli/add/core/validate/required.go @@ -15,40 +15,33 @@ import ( errCli "github.com/ActiveMemory/ctx/internal/err/cli" ) -// RequireBodyFlags wraps the command's PreRunE so each named flag -// is read and rejected when its value is empty, whitespace-only, -// or matches the closed placeholder set (TBD, see chat, n/a, -// etc.). Existing PreRunE is preserved and runs after the check. +// BodyFlags reads each named flag from c and returns an error if +// the value is empty, whitespace-only, or matches the closed +// placeholder set (TBD, see chat, n/a, etc.). It is a pure +// function: it does not mutate c, does not register PreRunE, and +// does not call [cobra.Command.MarkFlagRequired]. // -// The check is the single enforcement point: there is no -// [cobra.Command.MarkFlagRequired] call, so help text does not -// gain a "(required)" annotation. Cobra defaults string flags to -// the empty string, which the empty check rejects with a clear -// message — making the marker redundant and the discarded error -// it returns avoidable. +// Callers invoke this directly from their own PreRunE so the +// wiring is visible at the noun-level command constructor. // // Parameters: -// - c: cobra command to mutate +// - c: cobra command whose flags are being checked // - flags: names of body flags to read and policy-check -func RequireBodyFlags(c *cobra.Command, flags ...string) { - prev := c.PreRunE - c.PreRunE = func(cmd *cobra.Command, args []string) error { - for _, name := range flags { - value, getErr := cmd.Flags().GetString(name) - if getErr != nil { - return getErr - } - if rejectErr := RejectPlaceholder( - name, value, - ); rejectErr != nil { - return rejectErr - } +// +// Returns: +// - error: non-nil on the first flag that fails the check; +// nil if every flag passes +func BodyFlags(c *cobra.Command, flags ...string) error { + for _, name := range flags { + value, getErr := c.Flags().GetString(name) + if getErr != nil { + return getErr } - if prev != nil { - return prev(cmd, args) + if rejectErr := RejectPlaceholder(name, value); rejectErr != nil { + return rejectErr } - return nil } + return nil } // RejectPlaceholder returns an error if value is a placeholder diff --git a/internal/cli/add/core/validate/required_test.go b/internal/cli/add/core/validate/required_test.go index f6fef0045..f0ce3e017 100644 --- a/internal/cli/add/core/validate/required_test.go +++ b/internal/cli/add/core/validate/required_test.go @@ -67,100 +67,62 @@ func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { } } -func TestRequireBodyFlagsRejectsPlaceholderViaPreRunE(t *testing.T) { - ran := false +// newCmd builds a minimal cobra command with two string flags and +// a no-op RunE — the standard test fixture for the BodyFlags +// PreRunE pattern that the noun-level constructors use. +func newCmd() *cobra.Command { c := &cobra.Command{ - Use: "test", - RunE: func(_ *cobra.Command, _ []string) error { - ran = true - return nil - }, + Use: "test", + RunE: func(_ *cobra.Command, _ []string) error { return nil }, } c.Flags().String("context", "", "") c.Flags().String("rationale", "", "") - RequireBodyFlags(c, "context", "rationale") - c.SetArgs([]string{ - "--context", "TBD", - "--rationale", "good reason", - }) c.SetOut(&strings.Builder{}) c.SetErr(&strings.Builder{}) - err := c.Execute() - if err == nil { - t.Fatal("expected placeholder rejection") - } - if !strings.Contains(err.Error(), "context") { - t.Errorf("error should name the offending flag: %v", err) - } - if ran { - t.Error("RunE should not have executed after PreRunE rejected input") - } + return c } -func TestRequireBodyFlagsAllowsLegitimateInput(t *testing.T) { - ran := false - c := &cobra.Command{ - Use: "test", - RunE: func(_ *cobra.Command, _ []string) error { - ran = true - return nil - }, - } - c.Flags().String("context", "", "") - c.Flags().String("rationale", "", "") - RequireBodyFlags(c, "context", "rationale") +func TestBodyFlagsAcceptsLegitimateValues(t *testing.T) { + c := newCmd() c.SetArgs([]string{ "--context", "real context", "--rationale", "real rationale", }) - c.SetOut(&strings.Builder{}) - c.SetErr(&strings.Builder{}) - if err := c.Execute(); err != nil { - t.Fatalf("legitimate input rejected: %v", err) + if execErr := c.Execute(); execErr != nil { + t.Fatalf("parse: %v", execErr) } - if !ran { - t.Error("RunE should have executed") + if err := BodyFlags(c, "context", "rationale"); err != nil { + t.Errorf("BodyFlags rejected legitimate input: %v", err) } } -func TestRequireBodyFlagsPreservesExistingPreRunE(t *testing.T) { - preRan := false - c := &cobra.Command{ - Use: "test", - PreRunE: func(_ *cobra.Command, _ []string) error { - preRan = true - return nil - }, - RunE: func(_ *cobra.Command, _ []string) error { return nil }, +func TestBodyFlagsRejectsPlaceholder(t *testing.T) { + c := newCmd() + c.SetArgs([]string{ + "--context", "TBD", + "--rationale", "good reason", + }) + if execErr := c.Execute(); execErr != nil { + t.Fatalf("parse: %v", execErr) } - c.Flags().String("context", "", "") - RequireBodyFlags(c, "context") - c.SetArgs([]string{"--context", "legitimate"}) - c.SetOut(&strings.Builder{}) - c.SetErr(&strings.Builder{}) - if err := c.Execute(); err != nil { - t.Fatalf("unexpected error: %v", err) + err := BodyFlags(c, "context", "rationale") + if err == nil { + t.Fatal("expected placeholder rejection") } - if !preRan { - t.Error("existing PreRunE should still execute") + if !strings.Contains(err.Error(), "context") { + t.Errorf("error should name the offending flag: %v", err) } } -// TestRequireBodyFlagsRejectsMissingFlag verifies that omitting -// the body flag entirely is rejected by PreRunE — cobra's default -// for string flags is "", which trips the empty-value check. No -// separate MarkFlagRequired is needed. -func TestRequireBodyFlagsRejectsMissingFlag(t *testing.T) { - c := &cobra.Command{ - Use: "test", - RunE: func(_ *cobra.Command, _ []string) error { return nil }, - } - c.Flags().String("context", "", "") - RequireBodyFlags(c, "context") - c.SetArgs([]string{}) - c.SetOut(&strings.Builder{}) - c.SetErr(&strings.Builder{}) - err := c.Execute() +func TestBodyFlagsRejectsMissingFlag(t *testing.T) { + // Cobra defaults --context to "" when omitted; the empty-value + // check catches it through the same path as a placeholder. + c := newCmd() + c.SetArgs([]string{"--rationale", "ok"}) + if execErr := c.Execute(); execErr != nil { + t.Fatalf("parse: %v", execErr) + } + err := BodyFlags(c, "context", "rationale") if err == nil { t.Fatal("expected rejection when --context is missing") } @@ -168,3 +130,23 @@ func TestRequireBodyFlagsRejectsMissingFlag(t *testing.T) { t.Errorf("error should name --context: %v", err) } } + +func TestBodyFlagsStopsAtFirstFailure(t *testing.T) { + // Flag order in the call determines which failure is reported + // when multiple flags fail. + c := newCmd() + c.SetArgs([]string{ + "--context", "TBD", + "--rationale", "n/a", + }) + if execErr := c.Execute(); execErr != nil { + t.Fatalf("parse: %v", execErr) + } + err := BodyFlags(c, "rationale", "context") + if err == nil { + t.Fatal("expected rejection") + } + if !strings.Contains(err.Error(), "rationale") { + t.Errorf("expected --rationale to be reported first, got %v", err) + } +} diff --git a/internal/cli/decision/cmd/add/cmd.go b/internal/cli/decision/cmd/add/cmd.go index 2bbfafa68..bd818de7f 100644 --- a/internal/cli/decision/cmd/add/cmd.go +++ b/internal/cli/decision/cmd/add/cmd.go @@ -21,15 +21,19 @@ import ( // Adds a new decision entry to DECISIONS.md with the // required provenance, context, rationale, and consequence // flags. Implementation lives in the shared add core; this -// noun-level constructor additionally enforces that the -// three body flags are present and non-placeholder. +// noun-level constructor installs a PreRunE that calls +// [validate.BodyFlags] to reject empty or placeholder values +// on the three body flags. // // Returns: // - *cobra.Command: Configured decision add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Decision, cmd.DescKeyDecisionAdd, cmd.UseDecisionAdd) - validate.RequireBodyFlags( - c, cFlag.Context, cFlag.Rationale, cFlag.Consequence, - ) + c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { + return validate.BodyFlags( + cobraCmd, + cFlag.Context, cFlag.Rationale, cFlag.Consequence, + ) + } return c } diff --git a/internal/cli/learning/cmd/add/cmd.go b/internal/cli/learning/cmd/add/cmd.go index a57472a15..95b2215bb 100644 --- a/internal/cli/learning/cmd/add/cmd.go +++ b/internal/cli/learning/cmd/add/cmd.go @@ -21,15 +21,19 @@ import ( // Adds a new learning entry to LEARNINGS.md with the // required provenance, context, lesson, and application // flags. Implementation lives in the shared add core; this -// noun-level constructor additionally enforces that the -// three body flags are present and non-placeholder. +// noun-level constructor installs a PreRunE that calls +// [validate.BodyFlags] to reject empty or placeholder values +// on the three body flags. // // Returns: // - *cobra.Command: Configured learning add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Learning, cmd.DescKeyLearningAdd, cmd.UseLearningAdd) - validate.RequireBodyFlags( - c, cFlag.Context, cFlag.Lesson, cFlag.Application, - ) + c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { + return validate.BodyFlags( + cobraCmd, + cFlag.Context, cFlag.Lesson, cFlag.Application, + ) + } return c } From 971bf767f9d4c163fecda130f1645a2069bbcc6c Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 16:52:21 -0700 Subject: [PATCH 6/9] refactor(validate): consolidate body-flag helpers into internal/validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit internal/validate/ already exists and its doc.go explicitly declares itself the anchor for "future non-path validators". A separate internal/cli/add/core/validate/ duplicated that philosophy with a tiny pure-function helper. Fold the helper into internal/validate/ and delete the duplicate. Changes: - internal/validate/bodyflags.go — new file with BodyFlags and RejectPlaceholder (identical to the deleted helpers). - internal/validate/bodyflags_test.go — tests moved over. - internal/validate/doc.go — adds a "CLI Body-Flag Validation" section alongside the existing path validators. - internal/cli/decision/cmd/add/cmd.go and internal/cli/learning/cmd/add/cmd.go — import internal/validate instead of the old path. - internal/cli/add/core/validate/ — deleted. Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- internal/cli/add/core/validate/doc.go | 29 ------------------- internal/cli/decision/cmd/add/cmd.go | 2 +- internal/cli/learning/cmd/add/cmd.go | 2 +- .../required.go => validate/bodyflags.go} | 0 .../bodyflags_test.go} | 16 +++++----- internal/validate/doc.go | 25 +++++++++++----- 6 files changed, 28 insertions(+), 46 deletions(-) delete mode 100644 internal/cli/add/core/validate/doc.go rename internal/{cli/add/core/validate/required.go => validate/bodyflags.go} (100%) rename internal/{cli/add/core/validate/required_test.go => validate/bodyflags_test.go} (91%) diff --git a/internal/cli/add/core/validate/doc.go b/internal/cli/add/core/validate/doc.go deleted file mode 100644 index 0da7ed0b1..000000000 --- a/internal/cli/add/core/validate/doc.go +++ /dev/null @@ -1,29 +0,0 @@ -// / ctx: https://ctx.ist -// ,'`./ do you remember? -// `.,'\ -// \ Copyright 2026-present Context contributors. -// SPDX-License-Identifier: Apache-2.0 - -// Package validate is a pure-function policy layer for add -// subcommand body flags. -// -// It exports two functions: -// -// - [BodyFlags] reads each named flag from a cobra command and -// returns an error if any value is empty, whitespace-only, -// or matches the closed placeholder set (TBD, see chat, n/a, -// etc.). It does not mutate the command. -// -// - [RejectPlaceholder] is the per-value primitive used by -// [BodyFlags] and is exported for tests and ad-hoc reuse. -// -// Cobra defaults string flags to "", so the empty-value check -// catches missing flags through the same code path as -// placeholder rejection. Substring matches are not placeholders -// — legitimate prose containing the word "TBD" still passes. -// -// Noun-level add subcommands (decision, learning) invoke -// [BodyFlags] from their own PreRunE so the wiring is visible -// at the call site. The package does not register hooks on the -// caller's command. -package validate diff --git a/internal/cli/decision/cmd/add/cmd.go b/internal/cli/decision/cmd/add/cmd.go index bd818de7f..af5b50749 100644 --- a/internal/cli/decision/cmd/add/cmd.go +++ b/internal/cli/decision/cmd/add/cmd.go @@ -10,10 +10,10 @@ import ( "github.com/spf13/cobra" "github.com/ActiveMemory/ctx/internal/cli/add/core/build" - "github.com/ActiveMemory/ctx/internal/cli/add/core/validate" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" "github.com/ActiveMemory/ctx/internal/config/entry" cFlag "github.com/ActiveMemory/ctx/internal/config/flag" + "github.com/ActiveMemory/ctx/internal/validate" ) // Cmd returns the "ctx decision add" subcommand. diff --git a/internal/cli/learning/cmd/add/cmd.go b/internal/cli/learning/cmd/add/cmd.go index 95b2215bb..7261cd687 100644 --- a/internal/cli/learning/cmd/add/cmd.go +++ b/internal/cli/learning/cmd/add/cmd.go @@ -10,10 +10,10 @@ import ( "github.com/spf13/cobra" "github.com/ActiveMemory/ctx/internal/cli/add/core/build" - "github.com/ActiveMemory/ctx/internal/cli/add/core/validate" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" "github.com/ActiveMemory/ctx/internal/config/entry" cFlag "github.com/ActiveMemory/ctx/internal/config/flag" + "github.com/ActiveMemory/ctx/internal/validate" ) // Cmd returns the "ctx learning add" subcommand. diff --git a/internal/cli/add/core/validate/required.go b/internal/validate/bodyflags.go similarity index 100% rename from internal/cli/add/core/validate/required.go rename to internal/validate/bodyflags.go diff --git a/internal/cli/add/core/validate/required_test.go b/internal/validate/bodyflags_test.go similarity index 91% rename from internal/cli/add/core/validate/required_test.go rename to internal/validate/bodyflags_test.go index f0ce3e017..2fbdebdcd 100644 --- a/internal/cli/add/core/validate/required_test.go +++ b/internal/validate/bodyflags_test.go @@ -67,10 +67,10 @@ func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { } } -// newCmd builds a minimal cobra command with two string flags and -// a no-op RunE — the standard test fixture for the BodyFlags -// PreRunE pattern that the noun-level constructors use. -func newCmd() *cobra.Command { +// newBodyFlagsCmd builds a minimal cobra command with two string +// flags and a no-op RunE — the standard test fixture for the +// BodyFlags PreRunE pattern that the noun-level constructors use. +func newBodyFlagsCmd() *cobra.Command { c := &cobra.Command{ Use: "test", RunE: func(_ *cobra.Command, _ []string) error { return nil }, @@ -83,7 +83,7 @@ func newCmd() *cobra.Command { } func TestBodyFlagsAcceptsLegitimateValues(t *testing.T) { - c := newCmd() + c := newBodyFlagsCmd() c.SetArgs([]string{ "--context", "real context", "--rationale", "real rationale", @@ -97,7 +97,7 @@ func TestBodyFlagsAcceptsLegitimateValues(t *testing.T) { } func TestBodyFlagsRejectsPlaceholder(t *testing.T) { - c := newCmd() + c := newBodyFlagsCmd() c.SetArgs([]string{ "--context", "TBD", "--rationale", "good reason", @@ -117,7 +117,7 @@ func TestBodyFlagsRejectsPlaceholder(t *testing.T) { func TestBodyFlagsRejectsMissingFlag(t *testing.T) { // Cobra defaults --context to "" when omitted; the empty-value // check catches it through the same path as a placeholder. - c := newCmd() + c := newBodyFlagsCmd() c.SetArgs([]string{"--rationale", "ok"}) if execErr := c.Execute(); execErr != nil { t.Fatalf("parse: %v", execErr) @@ -134,7 +134,7 @@ func TestBodyFlagsRejectsMissingFlag(t *testing.T) { func TestBodyFlagsStopsAtFirstFailure(t *testing.T) { // Flag order in the call determines which failure is reported // when multiple flags fail. - c := newCmd() + c := newBodyFlagsCmd() c.SetArgs([]string{ "--context", "TBD", "--rationale", "n/a", diff --git a/internal/validate/doc.go b/internal/validate/doc.go index 33fcf3b65..b504d09bb 100644 --- a/internal/validate/doc.go +++ b/internal/validate/doc.go @@ -5,7 +5,8 @@ // SPDX-License-Identifier: Apache-2.0 // Package validate provides input-validation helpers -// that ctx uses at filesystem and security boundaries. +// that ctx uses at filesystem, security, and CLI +// boundaries. // // # Path Validation // @@ -23,18 +24,28 @@ // found. Non-existent directories are not an // error (let the caller handle that). // +// # CLI Body-Flag Validation +// +// - [BodyFlags] reads each named flag from a cobra +// command and rejects empty, whitespace-only, or +// placeholder values (TBD, see chat, n/a, etc.). +// Pure function: does not mutate the command. +// Called from a noun-level command's own PreRunE +// so the wiring is visible at the call site. +// - [RejectPlaceholder] is the per-value primitive +// used by [BodyFlags]; exported for tests and +// ad-hoc reuse. Placeholder values live in +// [internal/config/validate]. +// // # Design Philosophy // // Unlike [internal/sanitize] (which transforms bad // input into safe values), this package rejects bad // input outright. Unlike [internal/io] (which guards // against system directory access), this package -// guards against project-boundary escapes and -// symlink-based traversal. -// -// The validate.go file currently contains only the -// package declaration, serving as an anchor for -// future non-path validators. +// guards against project-boundary escapes, +// symlink-based traversal, and missing or +// placeholder CLI body fields. // // # Concurrency // From 0ccc1a83b72dba2a3d2eefa7b62683588d9c7ef6 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 17:54:46 -0700 Subject: [PATCH 7/9] refactor(validate): inline body-flag loop, drop BodyFlags helper Removes internal/validate.BodyFlags entirely. The two callers (decision add, learning add) now loop their body flags themselves in PreRunE and call validate.RejectPlaceholder per (flag, value) pair. The wiring is fully visible at the noun-level constructor and the validate package becomes uniformly string-consuming (matches Symlinks, RejectPlaceholder); the cobra.Command-taking outlier is gone. The RejectPlaceholder helper moves to its own file (rejectplaceholder.go / rejectplaceholder_test.go) per the single-helper-per-file convention used elsewhere in the project (internal/sanitize/truncate.go). The BodyFlags fixture and tests that needed a *cobra.Command go away with it; only the four RejectPlaceholder unit tests remain. Bundled in this commit because they share the same branch arc and the repo is mid-rebuild after a Go toolchain bump: * go.mod bumped 1.26.1 -> 1.26.3 to match the local toolchain (the 1.26.1 cached toolchain in ~/go/pkg/mod was corrupt). * SKILL.md (ctx-spec copilot integration) gains the --brief contract section that was already merged into the canonical /ctx-spec skill in 55acbd81 but missed the embedded asset. * Handover note from the prior session is preserved under .context/handovers/ following the existing precedent. Spec: specs/skill-surface-polish.md Signed-off-by: Jose Alekhinne --- ...7-skill-surface-polish-validate-cleanup.md | 190 ++++++++++++++++++ go.mod | 2 +- .../copilot-cli/skills/ctx-spec/SKILL.md | 41 +++- internal/cli/decision/cmd/add/cmd.go | 27 ++- internal/cli/learning/cmd/add/cmd.go | 27 ++- internal/validate/bodyflags.go | 66 ------ internal/validate/bodyflags_test.go | 152 -------------- internal/validate/doc.go | 17 +- internal/validate/rejectplaceholder.go | 40 ++++ internal/validate/rejectplaceholder_test.go | 66 ++++++ 10 files changed, 385 insertions(+), 243 deletions(-) create mode 100644 .context/handovers/2026-05-11-000847-skill-surface-polish-validate-cleanup.md delete mode 100644 internal/validate/bodyflags.go delete mode 100644 internal/validate/bodyflags_test.go create mode 100644 internal/validate/rejectplaceholder.go create mode 100644 internal/validate/rejectplaceholder_test.go diff --git a/.context/handovers/2026-05-11-000847-skill-surface-polish-validate-cleanup.md b/.context/handovers/2026-05-11-000847-skill-surface-polish-validate-cleanup.md new file mode 100644 index 000000000..e668e50d0 --- /dev/null +++ b/.context/handovers/2026-05-11-000847-skill-surface-polish-validate-cleanup.md @@ -0,0 +1,190 @@ +--- +generated-at: 2026-05-11T00:08:47Z +--- +# Handover [2026-05-11-000847] Phase SK shipped; 4 polish items on `internal/validate.BodyFlags` for next session + +**Provenance:** commit `971bf767` on branch `feat/skill-surface-polish` + +## Summary + +Phase SK (Skill Surface Polish) — all 7 tasks landed across +6 commits on branch `feat/skill-surface-polish` (off `main` at +`a44edfe3`): + +``` +971bf767 refactor(validate): consolidate body-flag helpers into internal/validate +ba2faa54 refactor(validate): pure BodyFlags, no PreRunE decoration +1156e44a docs(blog): align thought-piece bylines (← unrelated, stacked intentionally) +92507039 refactor(validate): single PreRunE enforcement, no panic, no swallowed errors +55acbd81 feat(skills): /ctx-spec --brief, authority boundaries, plan brief offer +f32c8fd9 feat(validate): require body flags on decision/learning add +``` + +All signed off, `make lint` 0 issues, `go test ./...` 0 failures, +working tree clean. Branch is local-only (not pushed). Spec is at +`specs/skill-surface-polish.md`; design ref at +`ideas/002-editorial-pipeline-and-skill-rigor.md` §3. + +The 5-commit churn on the `validate` package (4 commits between +`f32c8fd9` and `971bf767`) reflects iterative correction during +the session — each commit removed a code-smell the previous one +introduced. Functionally correct now, but the API surface still +has 4 specific polish items the user surfaced at session end. +They are non-blocking (the code works, the tests pass) but +should be addressed before PR review. + +## Outstanding polish items (next session) + +All in `internal/validate/` and its two call sites +(`internal/cli/decision/cmd/add/cmd.go`, +`internal/cli/learning/cmd/add/cmd.go`): + +### 1. `RejectPlaceholder` should be unexported + +Currently exported as `validate.RejectPlaceholder`. Only call +site is `BodyFlags` in the same package. Rename to +`rejectPlaceholder`. Update test references. + +### 2. Per-file convention: private helper lives alone + +Convention check confirmed by `internal/sanitize/truncate.go` +(single unexported `truncate` in its own file). After renaming, +move the helper to `internal/validate/rejectplaceholder.go` and +its tests to `internal/validate/rejectplaceholder_test.go`. The +remaining `bodyflags.go` should hold only `BodyFlags`. + +### 3. `BodyFlags` takes too much + +```go +// Current +func BodyFlags(c *cobra.Command, flags ...string) error +``` + +Only uses `c.Flags()`. Change to: + +```go +func BodyFlags(flags *pflag.FlagSet, names ...string) error +``` + +Call site: +```go +c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { + return validate.BodyFlags(cobraCmd.Flags(), cFlag.Context, ...) +} +``` + +Update tests — the fixture no longer needs a full +`cobra.Command`; constructing a `pflag.FlagSet` with two flags +and calling `Parse(args)` is simpler. + +### 4. `Placeholders` map shape is confusing + +`internal/config/validate/placeholder.go` currently defines: + +```go +const ( + PlaceholderTBD = "tbd" + PlaceholderNA = "n/a" + ... +) + +var Placeholders = map[string]struct{}{ + PlaceholderTBD: {}, // reads as if the key were the identifier + PlaceholderNA: {}, + ... +} +``` + +In a map literal, `PlaceholderTBD:` evaluates to its const value +`"tbd"` — the map key stored is the string, not the identifier. +The user surfaced this as a real code smell: the surface reads +"enum-keyed map" but it's actually a string-keyed set. + +**Resolution**: switch to a slice + linear scan (N=9; cost is +negligible): + +```go +var placeholders = []string{ + PlaceholderTBD, PlaceholderNA, PlaceholderNAShort, + PlaceholderNone, PlaceholderSeeChat, PlaceholderSeeAbove, + PlaceholderSeeBelow, PlaceholderPending, PlaceholderToBeDone, +} +``` + +```go +// in rejectPlaceholder +key := strings.ToLower(strings.TrimSpace(value)) +for _, p := range cfgValidate.Placeholders { + if key == p { + return errCli.FlagPlaceholder(flag, value) + } +} +``` + +The slice's contents are the same constants, and the slice name +documents the set's purpose without map-key sleight-of-hand. The +audit's magic-strings check passes because every literal lives +in `const PlaceholderXxx`. + +After the rename, `Placeholders` (capital P) should be +`placeholders` (private) since only `internal/validate` reads it. + +## Gating + +- Each item, when fixed, requires the normal gate: `make lint` + clean, `go test ./...` clean, working tree clean before any + commit. +- Sign every commit (`git commit -s`). +- Do not add a `Co-Authored-By:` line to commits. +- Stay on `feat/skill-surface-polish`; the branch is the + active feature branch and stacking polish on top is intended. +- The 4 items are independent; one focused commit per item is + fine, or fold into a single `refactor(validate): polish surface` + commit if the diff stays small. + +## Session meta — read before resuming + +This session accumulated 5 saved feedback memories in +`~/.claude/projects/-Users-volkan-Desktop-WORKSPACE-ctx/memory/` +(2026-05-10), all from the same root cause: I acted on first +impulse instead of grepping the codebase before scaffolding or +editing. Specifically saved: + +- `feedback_no_coauthored_by.md` — strip Claude line only, never + the whole signoff block +- `feedback_branch_before_commit.md` — branch off main first; + honour "stacking is intentional" +- `feedback_always_signoff.md` — DCO requires `git commit -s` +- `feedback_no_silent_errors_no_panic.md` — propagate errors, no + `_ =` or `panic` outside `Must`-prefixed functions +- `feedback_no_silent_decoration.md` — helpers do not wrap + caller's cobra hooks +- `feedback_grep_before_creating_packages.md` — extend existing + packages by default + +The user observed the cumulative drift mid-session and offered +the handover. Resuming agent: **read those memory files first.** +The 4 polish items above are technically small but every fix +this session has introduced a new issue. Slow down: read the +target file fully, grep for the convention, then edit. + +## Next session + +1. Pull this branch (`feat/skill-surface-polish @ 971bf767`). +2. Read `internal/validate/bodyflags.go` and + `internal/config/validate/placeholder.go` end-to-end. +3. Read `internal/sanitize/truncate.go` for the + single-private-helper-per-file pattern. +4. Apply items 1–4 above. One or four commits, either is fine. +5. After lint+test green and tree clean, hand back for push. + +Optional follow-up after the 4 fixes land: open the PR. PR body +draft already prepared mid-session; see commit `55acbd81` body +for the user-facing scope summary. + +## Open questions + +None. The 4 polish items have unambiguous resolutions above. +The user-visible behaviour (decision/learning reject empty and +placeholder body flags) is correct and tested. Polish is purely +about code shape and API surface. diff --git a/go.mod b/go.mod index 0af61fa74..7b27e1eda 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ActiveMemory/ctx -go 1.26.1 +go 1.26.3 require ( github.com/hashicorp/raft v1.7.3 diff --git a/internal/assets/integrations/copilot-cli/skills/ctx-spec/SKILL.md b/internal/assets/integrations/copilot-cli/skills/ctx-spec/SKILL.md index d5fb8cadc..d56073369 100644 --- a/internal/assets/integrations/copilot-cli/skills/ctx-spec/SKILL.md +++ b/internal/assets/integrations/copilot-cli/skills/ctx-spec/SKILL.md @@ -26,9 +26,48 @@ each section with the user to produce a complete design document. /ctx-spec /ctx-spec (session checkpointing) /ctx-spec (rss feed generation) +/ctx-spec --brief ideas/003-editorial-pipeline-debated-brief.md ``` -## Process +## --brief contract + +When invoked with `--brief `, the skill treats the file at +`` as the authoritative source and skips the fresh-template +Q&A. Two preconditions and an authority order govern the read: + +**Preconditions** + +- The brief file must exist; if it does not, stop and report the + missing path without falling back to the interactive flow. +- The brief file should be the output of a prior `/ctx-plan` + session or a hand-written equivalent. A casual idea note is not + a brief. + +**Authority order** when the brief, recorded decisions, frozen +docs, or your inference disagree: + +1. Frozen contracts in `docs/` (release notes, public CLI docs) +2. Recorded decisions in `.context/DECISIONS.md` +3. The brief at `` +4. Your own inference — only when steps 1–3 are silent, and + labeled `TBD` in the spec so it stands out for review. + +Never invert this order. If the brief contradicts a frozen +contract, surface the contradiction to the user; do not silently +follow the brief. + +**Flow when `--brief` is set** + +1. Read the brief in full. Do not paraphrase it back to the user. +2. Read `specs/tpl/spec-template.md` to get the section list. +3. For each template section, lift content from the brief + verbatim where the brief speaks to it. Light compression for + clarity is allowed; new facts are not. +4. Where the brief is silent, write `TBD` rather than inventing. +5. Write the spec to `specs/{feature-name}.md` and surface the + `TBD` entries for the user to fill in next. + +## Process (interactive, when `--brief` is absent) ### 1. Gather the Feature Name diff --git a/internal/cli/decision/cmd/add/cmd.go b/internal/cli/decision/cmd/add/cmd.go index af5b50749..1d52f50fb 100644 --- a/internal/cli/decision/cmd/add/cmd.go +++ b/internal/cli/decision/cmd/add/cmd.go @@ -21,19 +21,32 @@ import ( // Adds a new decision entry to DECISIONS.md with the // required provenance, context, rationale, and consequence // flags. Implementation lives in the shared add core; this -// noun-level constructor installs a PreRunE that calls -// [validate.BodyFlags] to reject empty or placeholder values -// on the three body flags. +// noun-level constructor installs a PreRunE that reads each +// body flag and calls [validate.RejectPlaceholder] to reject +// empty or placeholder values. // // Returns: // - *cobra.Command: Configured decision add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Decision, cmd.DescKeyDecisionAdd, cmd.UseDecisionAdd) c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { - return validate.BodyFlags( - cobraCmd, - cFlag.Context, cFlag.Rationale, cFlag.Consequence, - ) + flags := cobraCmd.Flags() + names := []string{ + cFlag.Context, + cFlag.Rationale, + cFlag.Consequence, + } + for _, name := range names { + value, getErr := flags.GetString(name) + if getErr != nil { + return getErr + } + rejectErr := validate.RejectPlaceholder(name, value) + if rejectErr != nil { + return rejectErr + } + } + return nil } return c } diff --git a/internal/cli/learning/cmd/add/cmd.go b/internal/cli/learning/cmd/add/cmd.go index 7261cd687..696002b21 100644 --- a/internal/cli/learning/cmd/add/cmd.go +++ b/internal/cli/learning/cmd/add/cmd.go @@ -21,19 +21,32 @@ import ( // Adds a new learning entry to LEARNINGS.md with the // required provenance, context, lesson, and application // flags. Implementation lives in the shared add core; this -// noun-level constructor installs a PreRunE that calls -// [validate.BodyFlags] to reject empty or placeholder values -// on the three body flags. +// noun-level constructor installs a PreRunE that reads each +// body flag and calls [validate.RejectPlaceholder] to reject +// empty or placeholder values. // // Returns: // - *cobra.Command: Configured learning add subcommand func Cmd() *cobra.Command { c := build.Cmd(entry.Learning, cmd.DescKeyLearningAdd, cmd.UseLearningAdd) c.PreRunE = func(cobraCmd *cobra.Command, _ []string) error { - return validate.BodyFlags( - cobraCmd, - cFlag.Context, cFlag.Lesson, cFlag.Application, - ) + flags := cobraCmd.Flags() + names := []string{ + cFlag.Context, + cFlag.Lesson, + cFlag.Application, + } + for _, name := range names { + value, getErr := flags.GetString(name) + if getErr != nil { + return getErr + } + rejectErr := validate.RejectPlaceholder(name, value) + if rejectErr != nil { + return rejectErr + } + } + return nil } return c } diff --git a/internal/validate/bodyflags.go b/internal/validate/bodyflags.go deleted file mode 100644 index 41f1335ee..000000000 --- a/internal/validate/bodyflags.go +++ /dev/null @@ -1,66 +0,0 @@ -// / ctx: https://ctx.ist -// ,'`./ do you remember? -// `.,'\ -// \ Copyright 2026-present Context contributors. -// SPDX-License-Identifier: Apache-2.0 - -package validate - -import ( - "strings" - - "github.com/spf13/cobra" - - cfgValidate "github.com/ActiveMemory/ctx/internal/config/validate" - errCli "github.com/ActiveMemory/ctx/internal/err/cli" -) - -// BodyFlags reads each named flag from c and returns an error if -// the value is empty, whitespace-only, or matches the closed -// placeholder set (TBD, see chat, n/a, etc.). It is a pure -// function: it does not mutate c, does not register PreRunE, and -// does not call [cobra.Command.MarkFlagRequired]. -// -// Callers invoke this directly from their own PreRunE so the -// wiring is visible at the noun-level command constructor. -// -// Parameters: -// - c: cobra command whose flags are being checked -// - flags: names of body flags to read and policy-check -// -// Returns: -// - error: non-nil on the first flag that fails the check; -// nil if every flag passes -func BodyFlags(c *cobra.Command, flags ...string) error { - for _, name := range flags { - value, getErr := c.Flags().GetString(name) - if getErr != nil { - return getErr - } - if rejectErr := RejectPlaceholder(name, value); rejectErr != nil { - return rejectErr - } - } - return nil -} - -// RejectPlaceholder returns an error if value is a placeholder -// (exact case-insensitive match against the closed set, plus -// whitespace-only). Substring matches are not rejected. -// -// Parameters: -// - flag: name of the flag, used in the error message -// - value: raw flag value as received from cobra -// -// Returns: -// - error: non-nil when value is a placeholder; nil otherwise -func RejectPlaceholder(flag, value string) error { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return errCli.FlagEmpty(flag) - } - if _, hit := cfgValidate.Placeholders[strings.ToLower(trimmed)]; hit { - return errCli.FlagPlaceholder(flag, value) - } - return nil -} diff --git a/internal/validate/bodyflags_test.go b/internal/validate/bodyflags_test.go deleted file mode 100644 index 2fbdebdcd..000000000 --- a/internal/validate/bodyflags_test.go +++ /dev/null @@ -1,152 +0,0 @@ -// / ctx: https://ctx.ist -// ,'`./ do you remember? -// `.,'\ -// \ Copyright 2026-present Context contributors. -// SPDX-License-Identifier: Apache-2.0 - -package validate - -import ( - "strings" - "testing" - - "github.com/spf13/cobra" -) - -func TestRejectPlaceholderAcceptsLegitimate(t *testing.T) { - for _, v := range []string{ - "a real rationale", - "because PostgreSQL is well-supported", - "we need TBD-style markers in the body but the field is real", - "see above the line break", - } { - if err := RejectPlaceholder("context", v); err != nil { - t.Errorf("RejectPlaceholder(%q) = %v, want nil", v, err) - } - } -} - -func TestRejectPlaceholderRejectsExactMatches(t *testing.T) { - for _, v := range []string{ - "TBD", "tbd", "Tbd", - "N/A", "n/a", "na", - "see chat", "See Chat", - "see above", "see below", - "pending", "PENDING", - "none", "to be done", - } { - if err := RejectPlaceholder("context", v); err == nil { - t.Errorf("RejectPlaceholder(%q) = nil, want error", v) - } - } -} - -func TestRejectPlaceholderRejectsWhitespace(t *testing.T) { - for _, v := range []string{ - "", - " ", - "\t", - "\n", - " \t \n ", - } { - err := RejectPlaceholder("rationale", v) - if err == nil { - t.Errorf("RejectPlaceholder(%q) = nil, want error", v) - } - if !strings.Contains(err.Error(), "rationale") { - t.Errorf("error should name flag 'rationale': %v", err) - } - } -} - -func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { - // " TBD " is still a placeholder after trim. - err := RejectPlaceholder("consequence", " TBD ") - if err == nil { - t.Error("padded placeholder should still be rejected") - } -} - -// newBodyFlagsCmd builds a minimal cobra command with two string -// flags and a no-op RunE — the standard test fixture for the -// BodyFlags PreRunE pattern that the noun-level constructors use. -func newBodyFlagsCmd() *cobra.Command { - c := &cobra.Command{ - Use: "test", - RunE: func(_ *cobra.Command, _ []string) error { return nil }, - } - c.Flags().String("context", "", "") - c.Flags().String("rationale", "", "") - c.SetOut(&strings.Builder{}) - c.SetErr(&strings.Builder{}) - return c -} - -func TestBodyFlagsAcceptsLegitimateValues(t *testing.T) { - c := newBodyFlagsCmd() - c.SetArgs([]string{ - "--context", "real context", - "--rationale", "real rationale", - }) - if execErr := c.Execute(); execErr != nil { - t.Fatalf("parse: %v", execErr) - } - if err := BodyFlags(c, "context", "rationale"); err != nil { - t.Errorf("BodyFlags rejected legitimate input: %v", err) - } -} - -func TestBodyFlagsRejectsPlaceholder(t *testing.T) { - c := newBodyFlagsCmd() - c.SetArgs([]string{ - "--context", "TBD", - "--rationale", "good reason", - }) - if execErr := c.Execute(); execErr != nil { - t.Fatalf("parse: %v", execErr) - } - err := BodyFlags(c, "context", "rationale") - if err == nil { - t.Fatal("expected placeholder rejection") - } - if !strings.Contains(err.Error(), "context") { - t.Errorf("error should name the offending flag: %v", err) - } -} - -func TestBodyFlagsRejectsMissingFlag(t *testing.T) { - // Cobra defaults --context to "" when omitted; the empty-value - // check catches it through the same path as a placeholder. - c := newBodyFlagsCmd() - c.SetArgs([]string{"--rationale", "ok"}) - if execErr := c.Execute(); execErr != nil { - t.Fatalf("parse: %v", execErr) - } - err := BodyFlags(c, "context", "rationale") - if err == nil { - t.Fatal("expected rejection when --context is missing") - } - if !strings.Contains(err.Error(), "context") { - t.Errorf("error should name --context: %v", err) - } -} - -func TestBodyFlagsStopsAtFirstFailure(t *testing.T) { - // Flag order in the call determines which failure is reported - // when multiple flags fail. - c := newBodyFlagsCmd() - c.SetArgs([]string{ - "--context", "TBD", - "--rationale", "n/a", - }) - if execErr := c.Execute(); execErr != nil { - t.Fatalf("parse: %v", execErr) - } - err := BodyFlags(c, "rationale", "context") - if err == nil { - t.Fatal("expected rejection") - } - if !strings.Contains(err.Error(), "rationale") { - t.Errorf("expected --rationale to be reported first, got %v", err) - } -} diff --git a/internal/validate/doc.go b/internal/validate/doc.go index b504d09bb..bc2274ee0 100644 --- a/internal/validate/doc.go +++ b/internal/validate/doc.go @@ -26,16 +26,15 @@ // // # CLI Body-Flag Validation // -// - [BodyFlags] reads each named flag from a cobra -// command and rejects empty, whitespace-only, or +// - [RejectPlaceholder] checks a single (flag, value) +// pair and rejects empty, whitespace-only, or // placeholder values (TBD, see chat, n/a, etc.). -// Pure function: does not mutate the command. -// Called from a noun-level command's own PreRunE -// so the wiring is visible at the call site. -// - [RejectPlaceholder] is the per-value primitive -// used by [BodyFlags]; exported for tests and -// ad-hoc reuse. Placeholder values live in -// [internal/config/validate]. +// Matching is case-insensitive after trimming; +// substring matches are not rejected. Noun-level +// add commands loop over their body flags in their +// own PreRunE and call this per pair, so the wiring +// is visible at the call site. The placeholder set +// lives in [internal/config/validate]. // // # Design Philosophy // diff --git a/internal/validate/rejectplaceholder.go b/internal/validate/rejectplaceholder.go new file mode 100644 index 000000000..134af25a3 --- /dev/null +++ b/internal/validate/rejectplaceholder.go @@ -0,0 +1,40 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "strings" + + cfgValidate "github.com/ActiveMemory/ctx/internal/config/validate" + errCli "github.com/ActiveMemory/ctx/internal/err/cli" +) + +// RejectPlaceholder returns an error if value is empty, +// whitespace-only, or matches the closed placeholder set +// (TBD, see chat, n/a, etc.). Matching is case-insensitive +// after trimming. Substring matches are not rejected. +// +// Callers loop over their body flags themselves and call +// this per (flag, value) pair so the wiring is visible at +// the noun-level command's PreRunE. +// +// Parameters: +// - flag: name of the flag, used in the error message +// - value: raw flag value as received from cobra +// +// Returns: +// - error: non-nil when value is empty or a placeholder; nil otherwise +func RejectPlaceholder(flag, value string) error { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return errCli.FlagEmpty(flag) + } + if _, hit := cfgValidate.Placeholders[strings.ToLower(trimmed)]; hit { + return errCli.FlagPlaceholder(flag, value) + } + return nil +} diff --git a/internal/validate/rejectplaceholder_test.go b/internal/validate/rejectplaceholder_test.go new file mode 100644 index 000000000..f86e95216 --- /dev/null +++ b/internal/validate/rejectplaceholder_test.go @@ -0,0 +1,66 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "strings" + "testing" +) + +func TestRejectPlaceholderAcceptsLegitimate(t *testing.T) { + for _, v := range []string{ + "a real rationale", + "because PostgreSQL is well-supported", + "we need TBD-style markers in the body but the field is real", + "see above the line break", + } { + if err := RejectPlaceholder("context", v); err != nil { + t.Errorf("RejectPlaceholder(%q) = %v, want nil", v, err) + } + } +} + +func TestRejectPlaceholderRejectsExactMatches(t *testing.T) { + for _, v := range []string{ + "TBD", "tbd", "Tbd", + "N/A", "n/a", "na", + "see chat", "See Chat", + "see above", "see below", + "pending", "PENDING", + "none", "to be done", + } { + if err := RejectPlaceholder("context", v); err == nil { + t.Errorf("RejectPlaceholder(%q) = nil, want error", v) + } + } +} + +func TestRejectPlaceholderRejectsWhitespace(t *testing.T) { + for _, v := range []string{ + "", + " ", + "\t", + "\n", + " \t \n ", + } { + err := RejectPlaceholder("rationale", v) + if err == nil { + t.Errorf("RejectPlaceholder(%q) = nil, want error", v) + } + if !strings.Contains(err.Error(), "rationale") { + t.Errorf("error should name flag 'rationale': %v", err) + } + } +} + +func TestRejectPlaceholderTrimsBeforeMatching(t *testing.T) { + // " TBD " is still a placeholder after trim. + err := RejectPlaceholder("consequence", " TBD ") + if err == nil { + t.Error("padded placeholder should still be rejected") + } +} From 68e5cab7b7e486b433b3fa725e904264576358b9 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 18:11:15 -0700 Subject: [PATCH 8/9] docs(spec): placeholder i18n + .ctxrc override; add Phase 0 task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the localization gap surfaced after 0ccc1a83: the placeholder set used by RejectPlaceholder is hardcoded English constants, has no .ctxrc override hook, and uses locale-naive strings.ToLower that misses Turkish dotted/dotless I and German sharp s once any non-ASCII placeholder enters the set. The spec proposes three coordinated moves in one future commit: 1. Move shipped defaults into an embedded YAML asset (internal/assets/commands/vocab/placeholders.en.yaml — exact path subject to convention audit). 2. Add a .ctxrc placeholders: key with EXTEND semantics, modeled on rc.SessionPrefixes() but combining user list onto defaults rather than replacing — the dominant case in this codebase is "Tarzan Turkish" (EN+TR intermingled), so replace would surprise. 3. Replace strings.ToLower with golang.org/x/text/cases.Fold for proper Unicode case folding at both ingest and compare time. Ship en-only in v1; ctx has no locale-specific assets anywhere yet, so this establishes the structure without committing to a tr.yaml landing in the same change. Phase 0 task added with #prerequisite-for-locale-work tag so the work is identified as gating any future locale-specific phase. Spec: specs/placeholder-i18n.md Signed-off-by: Jose Alekhinne --- .context/TASKS.md | 13 +++ specs/placeholder-i18n.md | 235 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 specs/placeholder-i18n.md diff --git a/.context/TASKS.md b/.context/TASKS.md index 59141cc2c..cc099f48a 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -32,6 +32,19 @@ TASK STATUS LABELS: `--phase` flag too, and we can have a auditor/normalizer for the current task document; or a skill that does a semantic pass, or both too. +- [ ] Localize the placeholder set used by `RejectPlaceholder` + (decision add / learning add and any future body-flag validators). + Move the shipped defaults out of `internal/config/validate/placeholder.go` + Go constants into an embedded YAML asset, add a `.ctxrc placeholders:` + override with EXTEND semantics (user list is appended to defaults, not + replacing them — Tarzan Turkish is the dominant case), and replace the + current `strings.ToLower` with proper Unicode case folding via + `golang.org/x/text/cases` so İ/i, ß/SS, and similar fold correctly. + Ship `en` only in v1; ctx has no locale-specific assets yet, so the + structure is established but no `tr.yaml` lands in this work. + Spec: `specs/placeholder-i18n.md` #priority:high #added:2026-05-11 + #prerequisite-for-locale-work + ### Misc diff --git a/specs/placeholder-i18n.md b/specs/placeholder-i18n.md new file mode 100644 index 000000000..46d01ca22 --- /dev/null +++ b/specs/placeholder-i18n.md @@ -0,0 +1,235 @@ +--- +title: Placeholder set i18n + .ctxrc override +date: 2026-05-11 +status: ready +owner: jose +scope: validate package + new asset bundle + .ctxrc accessor + Unicode-safe folding +design-ref: feedback from session 3c81f71b on commit 0ccc1a83 (validate body-flag refactor) +phase: 0 (prerequisite for any phase that ships locale-specific behaviour) +--- + +# Spec: Placeholder set i18n + .ctxrc override + +Make the placeholder set used by `RejectPlaceholder` localizable and +user-extensible. Today the set is hardcoded English in +`internal/config/validate/placeholder.go`; users writing in any other +language can still slip "por definir", "iptal", or "TBD-yapılacak" +through the validator. + +## Problem + +Three concrete gaps: + +1. **English-only set** baked as Go constants + (`PlaceholderTBD`, `PlaceholderNA`, `PlaceholderSeeChat`, …). A + Spanish, Turkish, German, or Japanese user has no shipped + defence against their language's "to be defined" markers. +2. **No `.ctxrc` override hook**. Every other parser-vocabulary + list in ctx (`session_prefixes`, `classify_rules`, + `spec_signal_words`) is overridable. Placeholders are the + outlier. +3. **Locale-naive case folding**. `RejectPlaceholder` does + `strings.ToLower(trimmed)`. Today the set is pure ASCII so + nothing breaks; the moment a Turkish or German placeholder + lands in the YAML, byte-level `ToLower` will silently miss + values typed with Unicode case differences: + `strings.ToLower("İPTAL") == "i̇ptal"` (combining dot above), + not `"iptal"`. Same trap with German `ß`/`SS`. + +## Approach + +Three coordinated moves, one commit: + +1. **Extract defaults to an embedded YAML asset.** New file + `internal/assets/commands/vocab/placeholders.en.yaml` (path + subject to convention check — see Open Questions). Loaded at + init time the same way `commands/text/*.yaml` is loaded. The + `Placeholder*` Go consts in `internal/config/validate/placeholder.go` + stay as identifier handles for tests and references; only the + string values move to YAML. + +2. **Add a `.ctxrc placeholders:` accessor with EXTEND semantics.** + Modeled on `rc.SessionPrefixes()` but with the combine rule + inverted: + + ```go + // SessionPrefixes — REPLACE: empty user list → use defaults. + // Placeholders — EXTEND: user list is appended to defaults, + // case-folded and de-duplicated. + ``` + + Reason for the difference: a Spanish user setting + `placeholders: ["por definir"]` should still have `tbd` + rejected — bilingual ("Tarzan Turkish": EN+TR intermingled in + the same project) is the dominant case for this codebase, so + replace would surprise. Replace can be reconsidered later via + an opt-in flag if needed; extend is the safe default. + +3. **Replace `strings.ToLower` with proper Unicode case folding.** + Use `golang.org/x/text/cases` + `language.Und` for Unicode-aware + folding that handles İ/I/i correctly across all locales. + Applied at two sites: when loading user overrides from .ctxrc + (normalize on ingest) and when comparing the input value + against the set in `RejectPlaceholder`. + +## Behavior + +### Happy Path + +User in a Turkish-language project adds to `.ctxrc`: + +```yaml +placeholders: + - "iptal" + - "yapılacak" + - "tbd-yapılacak" +``` + +`ctx decision add --rationale "İptal"` → rejected (case-folded +match against user-supplied `iptal`). +`ctx decision add --rationale "TBD"` → still rejected (shipped +default). +`ctx decision add --rationale "iptal edildi çünkü ..."` → +accepted (substring, not exact match — same rule as today). + +### Edge Cases + +| Case | Expected behavior | +|------|-------------------| +| `.ctxrc` has `placeholders:` key but list is empty | Use shipped defaults only (no error) | +| `.ctxrc` user value duplicates a shipped default (`"tbd"`) | De-dupe silently after fold; no error | +| User value differs only by case (`"TBD"` vs `"tbd"`) | Same — de-dupe after fold | +| User value has surrounding whitespace | Trim on ingest; do not store raw | +| Malformed YAML (`placeholders: "foo"` instead of list) | RC loader rejects with typed error; do not silently ignore | +| Turkish dotted/dotless I (`İptal` typed; `iptal` in YAML) | Match (Unicode fold) | +| German sharp s (`STRASSE` typed; `strasse` in YAML) | Match (Unicode fold via `cases.Fold`) | +| Empty user value in list (`placeholders: ["", "tbd"]`) | Skip empty entries silently on ingest | + +### Validation Rules + +- Defaults YAML schema: top-level key `placeholders:`, list of + non-empty strings, lowercase ASCII for the shipped `en` file. +- `.ctxrc` schema: same shape; values may be any Unicode. +- Validator pre-folds and trims both sides before exact-match + comparison; substring matches remain accepted as legitimate + prose. + +### Error Handling + +| Error condition | User-facing message | Recovery | +|-----------------|---------------------|----------| +| Embedded YAML fails to parse at init | Panic with file path (build-time invariant) | Fix the YAML; CI catches via test | +| `.ctxrc placeholders:` is wrong type | `errCli.RCField("placeholders", "expected list of strings")` | User fixes .ctxrc | + +## Interface + +### Configuration + +`.ctxrc` gains: + +```yaml +placeholders: + - "iptal" + - "yapılacak" +``` + +No new CLI flags, no new commands. The validator change is +transparent — same rejection semantics, larger set of triggers. + +## Implementation + +### Files to Create/Modify + +| File | Change | +|------|--------| +| `internal/assets/commands/vocab/placeholders.en.yaml` | NEW: list of shipped defaults | +| `internal/config/validate/placeholder.go` | Keep `Placeholder*` const identifiers; move string values to YAML; delete the `Placeholders` map | +| `internal/assets/read/vocab/vocab.go` | NEW: loader for the vocab namespace, modeled on `read/desc` | +| `internal/rc/rc.go` | NEW: `Placeholders() []string` accessor with EXTEND combine rule | +| `internal/rc/` | Add `Placeholders []string \`yaml:"placeholders"\`` to RC struct | +| `internal/validate/rejectplaceholder.go` | Switch from `cfgValidate.Placeholders` map lookup to `rc.Placeholders()` slice scan; replace `strings.ToLower` with `cases.Fold` | +| `internal/validate/rejectplaceholder_test.go` | Add Unicode-fold cases (İ/i, ß/ss); add .ctxrc-override test | +| `go.mod` / `go.sum` | Add `golang.org/x/text/cases` if not already a transitive | +| `docs/operations/configuration/ctxrc.md` (if exists) | Document the new `placeholders:` key | + +### Key Functions + +```go +// internal/rc/rc.go +func Placeholders() []string { + defaults := vocab.Placeholders() // from embedded YAML + user := RC().Placeholders + return mergeFolded(defaults, user) // de-dupe after Fold +} + +// internal/validate/rejectplaceholder.go +func RejectPlaceholder(flag, value string) error { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return errCli.FlagEmpty(flag) + } + folded := cases.Fold().String(trimmed) + for _, p := range rc.Placeholders() { + if folded == p { // p is pre-folded at load time + return errCli.FlagPlaceholder(flag, value) + } + } + return nil +} +``` + +### Helpers to Reuse + +- `internal/assets/embed.go` for the embed.FS pattern +- `internal/assets/read/desc/desc.go` for the lookup-map pattern +- `internal/rc/rc.go` `SessionPrefixes()` for the accessor shape + (but invert the combine rule) + +## Testing + +- **Unit (`internal/validate/`)**: existing 4 tests stay; add cases + for İ/i, ß/SS, mixed-script Tarzan Turkish, and one negative + case (substring match still accepted). +- **Unit (`internal/rc/`)**: combine rule — user list extends + defaults; user duplicates fold to the same key; empty user list + → defaults only. +- **Integration**: write a temp `.ctxrc` with custom + `placeholders:`, run `ctx decision add` with a value matching + the user-supplied placeholder, assert non-zero exit and the + flag-name in stderr. +- **Audit**: `desckey_namespace_test`-style audit that every YAML + entry parses and every `Placeholder*` const has a corresponding + YAML value in the `en` file. + +## Non-Goals + +- **Full error-message i18n.** That's the `internal/config/embed/text` + package's job and is out of scope. Only the placeholder + vocabulary moves; the `errCli.FlagEmpty` and + `errCli.FlagPlaceholder` messages stay in their current form. +- **Per-flag placeholder sets.** One vocabulary applies to all + body flags (decision/learning add today, more later). Per-flag + customization can be added if a real need appears. +- **Shipping `tr` (or any other locale) defaults in this spec.** + ctx has not shipped any locale-specific behaviour yet (no `tr` + files anywhere). Ship `en` only; the structure makes adding + `tr.yaml`, `es.yaml`, etc. a copy-edit later. +- **Fuzzy / Levenshtein / stem matching.** Exact case-folded + match after trim, same as today. Substring matches still + accepted. +- **Replace semantics for `.ctxrc` overrides.** Extend only in + v1. Reconsider via an explicit `placeholders_replace: true` + toggle if a real need appears. + +## Open Questions + +- **Asset path.** Two reasonable homes: `internal/assets/commands/vocab/` + (sibling to `commands/text/`) or a new + `internal/assets/vocab/`. Pick whichever the existing convention + audits prefer; if the audit is silent, default to + `commands/vocab/` for consistency with the existing + `commands/text/` and `commands/cmd/` neighbors. +- **Loader namespace.** Mirror `internal/assets/read/desc` as + `internal/assets/read/vocab` or fold into `desc`? Separate + package keeps the responsibility line clean (display text vs + parser vocabulary). From 91b18c5fe2db1ae3ec0bed4426bc9b33c570acfe Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Sun, 10 May 2026 18:14:59 -0700 Subject: [PATCH 9/9] docs(context): capture EXTEND semantics decision + Go toolchain learning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two captures from session 3c81f71b that future contributors need: * DECISIONS.md: rc.Placeholders() will use EXTEND semantics (user list appended to defaults, case-folded de-duplicated), diverging from rc.SessionPrefixes() which uses REPLACE. The divergence is intentional — Tarzan Turkish (EN+TR intermingled) is the dominant case, so REPLACE would force users to re-list every English default to add one Turkish term and silently regress baseline coverage. * LEARNINGS.md: the "compile vs go tool version mismatch" error is caused by a corrupt cached toolchain in ~/go/pkg/mod/golang.org/ toolchain@v0.0.1-go./, NOT by the system Go install. Reinstalling Go does not fix it; deleting the cached dir or bumping the go.mod pin does. Spec: specs/placeholder-i18n.md Signed-off-by: Jose Alekhinne --- .context/DECISIONS.md | 15 +++++++++++++++ .context/LEARNINGS.md | 11 +++++++++++ 2 files changed, 26 insertions(+) diff --git a/.context/DECISIONS.md b/.context/DECISIONS.md index 126e2978a..17f8e7569 100644 --- a/.context/DECISIONS.md +++ b/.context/DECISIONS.md @@ -3,6 +3,7 @@ | Date | Decision | |----|--------| +| 2026-05-10 | Placeholder overrides use EXTEND not REPLACE semantics | | 2026-05-10 | Editorial constitution at .context/ingest/KB-RULES.md, not CONSTITUTION.md | | 2026-05-10 | Phase KB ships handover plus editorial paired, not split | | 2026-05-10 | KB ontology is pipeline-only-writer; no /ctx-kb-decide parallel skill | @@ -134,6 +135,20 @@ For significant decisions: --> +## [2026-05-10-181404] Placeholder overrides use EXTEND not REPLACE semantics + +**Status**: Accepted + +**Context**: When localizing the placeholder set used by validate.RejectPlaceholder, .ctxrc gains a placeholders: list. The existing precedent (rc.SessionPrefixes) uses REPLACE semantics: any non-empty user list completely replaces the shipped defaults. Placeholders need a different rule. + +**Decision**: Placeholder overrides use EXTEND not REPLACE semantics + +**Rationale**: The dominant case in this codebase is Tarzan Turkish — bilingual EN+TR projects where users need both English (TBD, n/a, see chat) and Turkish (iptal, yapılacak, görüşülecek) placeholders rejected simultaneously. REPLACE would force users to re-list every English default just to add one Turkish term, which they would skip and silently lose half the validator's coverage. EXTEND appends user list onto the shipped defaults so partial overrides do not regress baseline protection. + +**Consequence**: rc.Placeholders() must combine defaults + user list with case-folded de-duplication, diverging from the SessionPrefixes pattern. A future maintainer reading both accessors side-by-side will notice the inconsistency; the divergence is intentional and Spec: specs/placeholder-i18n.md captures why. If REPLACE is later wanted, add an opt-in placeholders_replace: true toggle rather than flipping the default. + +--- + ## [2026-05-10-001857] Editorial constitution at .context/ingest/KB-RULES.md, not CONSTITUTION.md **Status**: Accepted diff --git a/.context/LEARNINGS.md b/.context/LEARNINGS.md index 2ff79f1fe..87b7da092 100644 --- a/.context/LEARNINGS.md +++ b/.context/LEARNINGS.md @@ -17,6 +17,7 @@ DO NOT UPDATE FOR: | Date | Learning | |----|--------| +| 2026-05-10 | Go compile/tool version mismatch comes from the cached toolchain, not the system Go | | 2026-05-10 | An ongoing user's concrete workaround tax is the strongest validation evidence | | 2026-05-10 | Lift renames alongside features when borrowing from battle-tested external designs | | 2026-05-10 | KB epistemology: in a KB you do not decide, you increase confidence | @@ -129,6 +130,16 @@ DO NOT UPDATE FOR: --- +## [2026-05-10-181418] Go compile/tool version mismatch comes from the cached toolchain, not the system Go + +**Context**: Hit 'compile: version "go1.26.1" does not match go tool version "go1.26.2"' on every go build / go test / make lint, even with my changes stashed out. System Go was 1.26.2 (healthy); go.mod pinned 1.26.1, so Go's auto-toolchain feature had downloaded 1.26.1 to ~/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.26.1.darwin-arm64/. That cached toolchain was internally inconsistent: its compile binary and stdlib export data disagreed on version. + +**Lesson**: When the compile-vs-tool version error appears, the bug is the cached toolchain dir, not the installed Go. Reinstalling Go (brew, installer, etc.) does NOT touch the cached download, so the error persists after reinstall. Three real fixes: (1) rm -rf ~/go/pkg/mod/golang.org/toolchain@v0.0.1-go./ to force a clean re-download (~30s); (2) bump go.mod to match the system Go so the cached one is bypassed; (3) GOTOOLCHAIN=go to override the pin per-invocation. go clean -cache and GOTOOLCHAIN=local do not help. + +**Application**: First diagnostic on this error: check go env GOROOT — if it points to ~/go/pkg/mod/golang.org/toolchain@... the cached toolchain is in play. Then either delete the cached dir (most surgical) or bump go.mod (one-line diff, but lands in a commit). Do not waste time reinstalling Go. + +--- + ## [2026-05-10-001859] An ongoing user's concrete workaround tax is the strongest validation evidence **Context**: When extracting the editorial pipeline, the user pointed at things-wtf-disaster-recovery as a project where they were already running the editorial pattern manually — but at concrete cost: CLAUDE.md disabling half of ctx code-dev skills (/ctx-commit, /ctx-implement, /ctx-spec, /ctx-architecture, /ctx-brainstorm, /ctx-wrap-up), 10-CONSTITUTION.md at repo root colliding with .context/CONSTITUTION.md, hand-typed 8-item closeouts, hand-managed 20-INBOX.md, dedicated reference/vcf/external-grounding.md for ground-mode. The workaround was visible and the pain was specific.