diff --git a/.context/ARCHITECTURE.md b/.context/ARCHITECTURE.md index 26bc2b5ca..367576c14 100644 --- a/.context/ARCHITECTURE.md +++ b/.context/ARCHITECTURE.md @@ -284,9 +284,10 @@ Six defense layers (innermost to outermost): ### Layered Package Taxonomy Every CLI package follows `cmd/root + core/` taxonomy (Decision -2026-03-06). `cmd/root/cmd.go` defines the Cobra command; -`cmd/root/run.go` implements the handler. Shared logic lives in -`core/`. Grouping commands use `internal/cli/parent.Cmd()` factory. +2026-03-06). Each feature's `cmd/root/cmd.go` defines the Cobra +command; `cmd/root/run.go` implements the handler. Shared logic +lives in `core/`. Grouping commands use `internal/cli/parent.Cmd()` +factory. ### Config Explosion diff --git a/.context/DECISIONS.md b/.context/DECISIONS.md index 59cc4667b..dbf97c4b9 100644 --- a/.context/DECISIONS.md +++ b/.context/DECISIONS.md @@ -3,8 +3,14 @@ | Date | Decision | |----|--------| +| 2026-04-04 | TestNoMagicStrings and TestNoMagicValues no longer exempt const/var definitions outside config/ | +| 2026-04-04 | String-typed enums belong in config/, not domain packages | +| 2026-04-03 | Output functions belong in write/ (consolidated) | +| 2026-04-03 | YAML text externalization pipeline (consolidated) | +| 2026-04-03 | Package taxonomy and code placement (consolidated) | +| 2026-04-03 | Eager init over lazy loading (consolidated) | +| 2026-04-03 | Pure logic separation of concerns (consolidated) | | 2026-04-03 | config/ explosion is correct — fix is documentation, not restructuring | -| 2026-04-03 | YAML text externalization justification is agent legibility, not i18n | | 2026-04-01 | IRC to Discord as primary community channel | | 2026-04-01 | AST audit tests live in internal/audit/, one file per check | | 2026-04-01 | Split assets/hooks/ into assets/integrations/ + assets/hooks/messages/ | @@ -20,37 +26,23 @@ | 2026-03-25 | Prompt templates removed — skills are the single agent instruction mechanism | | 2026-03-24 | Write-once baseline with explicit end-consolidation for consolidation lifecycle | | 2026-03-23 | Pre/pre HTML tags promoted to shared constants in config/marker | -| 2026-03-23 | Pure-data param structs in entity — replace function pointers with text keys | -| 2026-03-22 | No runtime pluralization — use singular/plural text key pairs | | 2026-03-22 | Output functions belong in write/, never in core/ or cmd/ | -| 2026-03-21 | Output functions belong in write/, logic and types in core/ | | 2026-03-20 | Shared formatting utilities belong in internal/format | | 2026-03-20 | Go-YAML linkage check added to lint-drift as check 5 | -| 2026-03-18 | Eager Init() for static embedded data instead of per-accessor sync.Once | | 2026-03-18 | Singular command names for all CLI entities | | 2026-03-17 | Pre-compute-then-print for write package output blocks | -| 2026-03-16 | Server methods only handle dispatch and I/O, not struct construction | -| 2026-03-16 | Explicit Init over package-level init() for resource lookup | | 2026-03-16 | Resource name constants in config/mcp/resource, mapping in server/resource | | 2026-03-16 | Rename --consequences flag to --consequence for singular consistency | -| 2026-03-15 | Pure-logic CompactContext with no I/O — callers own file writes and reporting | -| 2026-03-15 | TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values | -| 2026-03-15 | Split text.yaml into 6 domain files loaded via loadYAMLDir | | 2026-03-14 | Error package taxonomy: 22 domain files replace monolithic errors.go | | 2026-03-14 | Session prefixes are parser vocabulary, not i18n text | | 2026-03-14 | System path deny-list as safety net, not security boundary | | 2026-03-14 | Config-driven freshness check with per-file review URLs | | 2026-03-13 | Delete ctx-context-monitor skill — hook output is self-sufficient | | 2026-03-13 | build target depends on sync-why to prevent embedded doc drift | -| 2026-03-13 | Templates and user-facing text live in assets, structural constants stay in config | | 2026-03-12 | Recommend companion RAGs as peer MCP servers not bridge through ctx | -| 2026-03-12 | Split commands.yaml into 4 domain files | | 2026-03-12 | Rename ctx-map skill to ctx-architecture | | 2026-03-07 | Use composite directory path constants for multi-segment paths | | 2026-03-06 | Drop fatih/color dependency — Unicode symbols are sufficient for terminal output, color was redundant | -| 2026-03-06 | Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() | -| 2026-03-06 | cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd//{cmd.go,run.go}, shared helpers in core/ | -| 2026-03-06 | Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage | | 2026-03-06 | PR #27 (MCP server) meets v0.1 spec requirements — merge-ready pending 3 compliance fixes | | 2026-03-06 | Skills stay CLI-based; MCP Prompts are the protocol equivalent | | 2026-03-06 | Peer MCP model for external tool integration | @@ -124,31 +116,115 @@ For significant decisions: --> -## [2026-04-03-133244] config/ explosion is correct — fix is documentation, not restructuring +## [2026-04-04-025755] TestNoMagicStrings and TestNoMagicValues no longer exempt const/var definitions outside config/ **Status**: Accepted -**Context**: Architecture analysis flagged 60+ config sub-packages as a bottleneck. Evaluation showed the alternative (8-10 domain packages) trades granular imports for fat dependency units. Current structure gives zero internal dependencies, surgical dependency tracking, and minimal recompile scope. +**Context**: The isConstDef/isVarDef blanket exemption masked 156+ string and 7 numeric constants in the wrong package -**Decision**: config/ explosion is correct — fix is documentation, not restructuring +**Decision**: TestNoMagicStrings and TestNoMagicValues no longer exempt const/var definitions outside config/ -**Rationale**: Go's compilation unit is the package. Granular packages mean precise dependency tracking. The developer experience cost (IDE noise, package discovery) is real but solvable with a README decision tree, not restructuring. Restructuring would be massive mechanical churn for cosmetic benefit. +**Rationale**: Const definitions outside config/ are magic values in the wrong place — naming them does not fix the structural problem -**Consequence**: config/README.md written with organizational guide and decision tree. No restructuring planned. embed/text/ file count will shrink naturally when tpl/ migrates to text/template. +**Consequence**: All new code with string/numeric constants outside config/ fails these tests immediately + +--- + +## [2026-04-04-025746] String-typed enums belong in config/, not domain packages + +**Status**: Accepted + +**Context**: Debated whether type IssueType string with const values belongs in domain or config. The string value is the same regardless of type annotation. + +**Decision**: String-typed enums belong in config/, not domain packages + +**Rationale**: Types without behavior belong in config. Promote to entity/ only when methods/interfaces appear. + +**Consequence**: All type Foo string + const blocks outside config/ are now caught by TestNoMagicStrings. + +--- + +## [2026-04-03-180000] Output functions belong in write/ (consolidated) + +**Status**: Accepted + +**Consolidated from**: 2 entries (2026-03-21 to 2026-03-22) + +**Decision**: Output functions belong in write/, logic and types in core/, orchestration in cmd/ + +**Rationale**: The write/ taxonomy is flat by domain — each CLI feature gets its own write/ package. core/ owns domain logic and types. cmd/ owns Cobra orchestration. Functions that call cmd.Print/Println/Printf belong in write/. core/ never imports cobra for output purposes. + +**Consequence**: All new CLI output must go through a write/ package. No cmd.Print* calls in internal/cli/ outside of internal/write/. + +--- + +## [2026-04-03-180000] YAML text externalization pipeline (consolidated) + +**Status**: Accepted + +**Consolidated from**: 5 entries (2026-03-06 to 2026-04-03) + +**Decision**: All user-facing text externalized to embedded YAML domain files, justified by agent legibility and drift prevention — not i18n + +**Rationale**: The real justification is agent legibility (named DescKey constants as traversable graphs) and drift prevention (TestDescKeyYAMLLinkage catches orphans mechanically). i18n is a free downstream consequence. The exhaustive test verifies all constants resolve to non-empty YAML values — new keys are automatically covered. + +**Consequence**: commands.yaml split into 4 domain files (commands, flags, text, examples) loaded via dedicated loaders. text.yaml split into 6 domain files loaded via loadYAMLDir. The 3-file ceremony (DescKey + YAML + write/err function) is the cost of agent-legible, drift-proof output. + +--- + +## [2026-04-03-180000] Package taxonomy and code placement (consolidated) + +**Status**: Accepted + +**Consolidated from**: 3 entries (2026-03-06 to 2026-03-13) + +**Decision**: Three-zone taxonomy: cmd/ for Cobra wiring (cmd.go + run.go), core/ for logic and types, assets/ for templates and user-facing text. config/ for structural constants only. + +**Rationale**: Taxonomical symmetry makes navigation instant and agent-friendly. Domain types that multiple packages consume belong in domain packages (internal/entry), not CLI subpackages. Templates and user-facing text live in assets/ for i18n readiness; structural constants (paths, limits, regexes) stay in config/. + +**Consequence**: Every CLI package has the same predictable shape. Shared entry types live in internal/entry. Template files (tpl_*.go) moved from config/ to assets/. 474 files changed in initial restructuring. + +--- + +## [2026-04-03-180000] Eager init over lazy loading (consolidated) + +**Status**: Accepted + +**Consolidated from**: 2 entries (2026-03-16 to 2026-03-18) + +**Decision**: Explicit Init() called eagerly at startup for static embedded data and resource lookups, instead of per-accessor sync.Once or package-level init() + +**Rationale**: Static embedded data is required at startup — sync.Once per accessor is cargo cult. Package-level init() hides startup dependencies and makes ordering unclear. Explicit Init() called from main.go / NewServer makes the dependency visible and testable. + +**Consequence**: Maps unexported, accessors are plain lookups. Tests call Init() in TestMain. res.Init() called from NewServer before ToList(). No package-level side effects, zero sync.Once in the lookup pipeline. + +--- + +## [2026-04-03-180000] Pure logic separation of concerns (consolidated) + +**Status**: Accepted + +**Consolidated from**: 3 entries (2026-03-15 to 2026-03-23) + +**Decision**: Pure-logic functions return data structs; callers own I/O, file writes, and reporting. Function pointers in param structs replaced with text keys. + +**Rationale**: Pure logic with no I/O lets both MCP (JSON-RPC) and CLI (cobra) callers control output independently. Methods that don't access receiver state hide their true dependencies — make them free functions. If all callers of a callback vary only by a string key, the callback is data in disguise. + +**Consequence**: CompactContext returns CompactResult; callers iterate FileUpdates. Server response helpers in server/out, prompt builders in server/prompt. All cross-cutting param structs in entity are function-pointer-free. --- -## [2026-04-03-133236] YAML text externalization justification is agent legibility, not i18n +## [2026-04-03-133244] config/ explosion is correct — fix is documentation, not restructuring **Status**: Accepted -**Context**: Principal analysis initially framed 879-key YAML externalization as a bet on i18n. Blog post review (v0.8.0) revealed the real justification: agent legibility (named DescKey constants as traversable graphs), drift prevention (TestDescKeyYAMLLinkage catches orphans mechanically), and completing the archaeology of finding all 879 scattered strings. +**Context**: Architecture analysis flagged 60+ config sub-packages as a bottleneck. Evaluation showed the alternative (8-10 domain packages) trades granular imports for fat dependency units. Current structure gives zero internal dependencies, surgical dependency tracking, and minimal recompile scope. -**Decision**: YAML text externalization justification is agent legibility, not i18n +**Decision**: config/ explosion is correct — fix is documentation, not restructuring -**Rationale**: The v0.8.0 blog makes it explicit: finding strings is the hard part, translation is mechanical. The externalization already pays for itself through agent legibility and mechanical verification. i18n is a free downstream consequence, not the justification. +**Rationale**: Go's compilation unit is the package. Granular packages mean precise dependency tracking. The developer experience cost (IDE noise, package discovery) is real but solvable with a README decision tree, not restructuring. Restructuring would be massive mechanical churn for cosmetic benefit. -**Consequence**: Future architecture analysis should frame externalization as already-justified investment. The 3-file ceremony (DescKey + YAML + write/err function) is the cost of agent-legible, drift-proof output — not speculative i18n prep. +**Consequence**: config/README.md written with organizational guide and decision tree. No restructuring planned. embed/text/ file count will shrink naturally when tpl/ migrates to text/template. --- @@ -362,34 +438,6 @@ For significant decisions: --- -## [2026-03-23-003346] Pure-data param structs in entity — replace function pointers with text keys - -**Status**: Accepted - -**Context**: MergeParams had UpdateFn callback, DeployParams had ListErr/ReadErr function pointers — both smuggled side effects into data structs - -**Decision**: Pure-data param structs in entity — replace function pointers with text keys - -**Rationale**: Text keys are pure data, keep entity dependency-free, and the consuming function can do the dispatch itself - -**Consequence**: All cross-cutting param structs in entity must be function-pointer-free; I/O functions passed as direct parameters - ---- - -## [2026-03-22-084509] No runtime pluralization — use singular/plural text key pairs - -**Status**: Accepted - -**Context**: Hardcoded English plural rules (+ s, y → ies) were scattered across format.Pluralize, padPluralize, and inline code — all i18n dead-ends - -**Decision**: No runtime pluralization — use singular/plural text key pairs - -**Rationale**: Different languages have vastly different plural rules. Complete sentence templates with embedded counts (time.minute-count '1 minute', time.minutes-count '%d minutes') let each locale define its own plural forms. - -**Consequence**: format.Pluralize and format.PluralWord are deleted. All plural output uses paired text keys with the count embedded in the template. - ---- - ## [2026-03-22-084316] Output functions belong in write/, never in core/ or cmd/ **Status**: Accepted @@ -404,20 +452,6 @@ For significant decisions: --- -## [2026-03-21-084020] Output functions belong in write/, logic and types in core/ - -**Status**: Accepted - -**Context**: PrintFeedReport was initially placed in cli/site/core/ but it calls cmd.Println — that's output formatting, not business logic - -**Decision**: Output functions belong in write/, logic and types in core/ - -**Rationale**: The project taxonomy separates concerns: core/ owns domain logic, types, and helpers; write/ owns CLI output formatting that takes *cobra.Command for Println. Mixing them blurs the boundary and makes testing harder. - -**Consequence**: All functions that call cmd.Print/Println/Printf belong in the write/ package tree. core/ never imports cobra for output purposes. - ---- - ## [2026-03-20-232506] Shared formatting utilities belong in internal/format **Status**: Accepted @@ -446,20 +480,6 @@ For significant decisions: --- -## [2026-03-18-193631] Eager Init() for static embedded data instead of per-accessor sync.Once - -**Status**: Accepted - -**Context**: 4 sync.Once guards + 4 exported maps + 4 Load functions + a wrapper package for YAML that never mutates. - -**Decision**: Eager Init() for static embedded data instead of per-accessor sync.Once - -**Rationale**: Data is static and required at startup. sync.Once per accessor is cargo cult. One Init() in main.go is sufficient. Tests call Init() in TestMain. - -**Consequence**: Maps unexported, accessors are plain lookups, permissions and stopwords also loaded eagerly. Zero sync.Once remains in the lookup pipeline. - ---- - ## [2026-03-18-193623] Singular command names for all CLI entities **Status**: Accepted @@ -488,34 +508,6 @@ For significant decisions: --- -## [2026-03-16-122033] Server methods only handle dispatch and I/O, not struct construction - -**Status**: Accepted - -**Context**: MCP server had ok/error/writeError as methods plus prompt builders that didn't use Server state — they just constructed response structs - -**Decision**: Server methods only handle dispatch and I/O, not struct construction - -**Rationale**: Methods that don't access receiver state hide their true dependencies and inflate the Server interface. Free functions make the dependency graph explicit and are independently testable. - -**Consequence**: New response helpers go in server/out, prompt builders in server/prompt. Server methods are limited to dispatch (handlePromptsGet) and I/O (writeJSON, emitNotification). Same principle applies to future tool/resource builders. - ---- - -## [2026-03-16-104143] Explicit Init over package-level init() for resource lookup - -**Status**: Accepted - -**Context**: server/resource package used init() to silently build the URI lookup map - -**Decision**: Explicit Init over package-level init() for resource lookup - -**Rationale**: Implicit init hides startup dependencies, makes ordering unclear, and is harder to test. Explicit Init called from NewServer makes the dependency visible. - -**Consequence**: res.Init() called explicitly from NewServer before ToList(); no package-level side effects - ---- - ## [2026-03-16-104142] Resource name constants in config/mcp/resource, mapping in server/resource **Status**: Accepted @@ -544,48 +536,6 @@ For significant decisions: --- -## [2026-03-15-230640] Pure-logic CompactContext with no I/O — callers own file writes and reporting - -**Status**: Accepted - -**Context**: MCP server and CLI compact command both implemented task compaction independently, with the MCP handler using a local WriteContextFile wrapper - -**Decision**: Pure-logic CompactContext with no I/O — callers own file writes and reporting - -**Rationale**: Separating pure logic from I/O lets both MCP (JSON-RPC responses) and CLI (cobra cmd.Println) callers control output and file writes. Eliminates duplication and the unnecessary mcp/server/fs package - -**Consequence**: tidy.CompactContext returns a CompactResult struct; callers iterate FileUpdates and write them. Archive logic stays in callers since MCP and CLI have different archive policies - ---- - -## [2026-03-15-101336] TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values - -**Status**: Accepted - -**Context**: PR #42 merged with ~80 new MCP text keys but no test coverage for key-to-YAML mapping - -**Decision**: TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values - -**Rationale**: A single table-driven test parsing embed.go source catches typos and missing YAML entries at test time — no manual key list to maintain - -**Consequence**: New TextDescKey constants are automatically covered; orphaned keys fail CI - ---- - -## [2026-03-15-040638] Split text.yaml into 6 domain files loaded via loadYAMLDir - -**Status**: Accepted - -**Context**: text.yaml grew to 1812 lines covering write, errors, mcp, doctor, hooks, and ui domains - -**Decision**: Split text.yaml into 6 domain files loaded via loadYAMLDir - -**Rationale**: Matches existing split pattern (commands.yaml, flags.yaml, examples.yaml); loadYAMLDir merges all files in commands/text/ transparently so TextDesc() API stays unchanged - -**Consequence**: New domain files must go into commands/text/; loadYAMLDir reads all .yaml in the directory at init time - ---- - ## [2026-03-14-180905] Error package taxonomy: 22 domain files replace monolithic errors.go **Status**: Accepted @@ -670,20 +620,6 @@ For significant decisions: --- -## [2026-03-13-151954] Templates and user-facing text live in assets, structural constants stay in config - -**Status**: Accepted - -**Context**: Ongoing refactoring session moving Tpl* constants out of config/ - -**Decision**: Templates and user-facing text live in assets, structural constants stay in config - -**Rationale**: config/ is for structural constants (paths, limits, regexes); assets/ is for templates, labels, and text that would need i18n. Clean separation of concerns - -**Consequence**: All tpl_entry.go, tpl_journal.go, tpl_loop.go, tpl_recall.go moved to assets/ - ---- - ## [2026-03-12-133007] Recommend companion RAGs as peer MCP servers not bridge through ctx **Status**: Accepted @@ -698,20 +634,6 @@ For significant decisions: --- -## [2026-03-12-133007] Split commands.yaml into 4 domain files - -**Status**: Accepted - -**Context**: Single 2373-line YAML mixed commands, flags, text, and examples with inconsistent quoting - -**Decision**: Split commands.yaml into 4 domain files - -**Rationale**: Context is for humans — localization files should be human-readable block scalars. Separate files eliminate the underscore prefix namespace hack - -**Consequence**: 4 files (commands.yaml, flags.yaml, text.yaml, examples.yaml) with dedicated loaders in embed.go - ---- - ## [2026-03-12-133007] Rename ctx-map skill to ctx-architecture **Status**: Accepted @@ -754,48 +676,6 @@ For significant decisions: --- -## [2026-03-06-200257] Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() - -**Status**: Accepted - -**Context**: Command descriptions were inline strings scattered across 105 cobra.Command definitions - -**Decision**: Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() - -**Rationale**: Centralizing user-facing text in a single translatable file prepares for i18n without runtime cost (embedded at compile time) - -**Consequence**: System's 30 hidden hook subcommands excluded (not user-facing); flag descriptions use _flags.scope.name convention - ---- - -## [2026-03-06-200247] cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd//{cmd.go,run.go}, shared helpers in core/ - -**Status**: Accepted - -**Context**: 35 CLI packages had inconsistent flat structures mixing Cmd(), run logic, helpers, and types in the same directory - -**Decision**: cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd//{cmd.go,run.go}, shared helpers in core/ - -**Rationale**: Taxonomical symmetry: every package has the same predictable shape, making navigation instant and agent-friendly - -**Consequence**: cmd/ contains only cmd.go + run.go; helpers go to core/; 474 files changed in initial restructuring - ---- - -## [2026-03-06-200227] Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage - -**Status**: Accepted - -**Context**: External consumers were importing cli/add for EntryParams/ValidateEntry/WriteEntry, creating a leaky abstraction - -**Decision**: Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage - -**Rationale**: Domain types in CLI packages force consumers to depend on CLI internals; internal/entry provides a clean boundary - -**Consequence**: entry aliases Params from add/core to avoid import cycle (entry imports add/core for insert logic); future work may move insert logic to entry to eliminate the cycle - ---- - ## [2026-03-06-141507] PR #27 (MCP server) meets v0.1 spec requirements — merge-ready pending 3 compliance fixes **Status**: Accepted @@ -1161,5 +1041,6 @@ See: `specs/injection-oversize-nudge.md`. --- + *Module-specific, already-shipped, and historical decisions: [decisions-reference.md](decisions-reference.md)* diff --git a/.context/LEARNINGS.md b/.context/LEARNINGS.md index 19e6fc2a1..8363dec6a 100644 --- a/.context/LEARNINGS.md +++ b/.context/LEARNINGS.md @@ -17,6 +17,15 @@ DO NOT UPDATE FOR: | Date | Learning | |----|--------| +| 2026-04-04 | Format-verb strings are localizable text, not exempt from magic string checks | +| 2026-04-04 | Agents add allowlist entries to make tests pass — guard every exemption | +| 2026-04-03 | Subagent scope creep and cleanup (consolidated) | +| 2026-04-03 | Bulk rename and replace_all hazards (consolidated) | +| 2026-04-03 | Import cycles and package splits (consolidated) | +| 2026-04-03 | Lint suppression and gosec patterns (consolidated) | +| 2026-04-03 | Skill lifecycle and promotion (consolidated) | +| 2026-04-03 | Cross-cutting change ripple (consolidated) | +| 2026-04-03 | Dead code detection (consolidated) | | 2026-04-03 | desc.Text() is the single highest-connectivity symbol in the codebase | | 2026-04-01 | Raw I/O migration unlocks downstream checks for free | | 2026-04-01 | go/packages respects build tags — darwin-only violations invisible on Linux | @@ -29,73 +38,47 @@ DO NOT UPDATE FOR: | 2026-03-31 | JSON Schema default fields cause linter errors with some validators | | 2026-03-30 | Architecture diagrams drift silently during feature additions | | 2026-03-30 | Python-generated doc.go files need gofmt — formatter strips bare // padding lines | -| 2026-03-30 | internal/cli/recall/ was dead code — never registered in bootstrap | | 2026-03-30 | lint-docstrings.sh greedy sed hid all return-type violations | | 2026-03-25 | Machine-generated CLAUDE.md content consumes per-turn budget without proportional value | -| 2026-03-25 | Dead files accumulate when nothing consumes them | | 2026-03-25 | Template improvements don't propagate to existing projects | | 2026-03-24 | lint-drift false positives from conflating constant namespaces | | 2026-03-24 | git describe --tags follows ancestry, not global tag list | | 2026-03-23 | Typography detection script needs exclusion lists for intentional uses | -| 2026-03-23 | Subagents rename functions and restructure code beyond their scope | | 2026-03-23 | Splitting core/ into subpackages reveals hidden structure | | 2026-03-23 | Higher-order callbacks in param structs are a code smell | -| 2026-03-22 | Types in god-object files create circular dependencies | -| 2026-03-20 | replace_all on short tokens like function names will also replace their definitions | | 2026-03-20 | Commit messages containing script paths trigger PreToolUse hooks | -| 2026-03-19 | Rename constants to avoid gosec G101 false positives | -| 2026-03-18 | Tests in package X cannot import X/sub packages that import X back | -| 2026-03-18 | Bulk sed on imports displaces aliased imports | | 2026-03-18 | Lazy sync.Once per-accessor is a code smell for static embedded data | | 2026-03-17 | Write package output census: 69 trivial/simple, 38 consolidation candidates, 18 complex | | 2026-03-16 | Docstring tasks require reading CONVENTIONS.md Documentation section first | | 2026-03-16 | Convention enforcement needs mechanical verification, not behavioral repetition | | 2026-03-16 | One-liner method wrappers hide dependencies without adding value | | 2026-03-16 | Agents reliably introduce gofmt issues during bulk renames | -| 2026-03-15 | replace_all on short tokens like core. mangles aliased imports | -| 2026-03-15 | Delete legacy code instead of maintaining it — MigrateKeyFile had 5 callers and test coverage but zero users | | 2026-03-15 | Contributor PRs need post-merge follow-up commits for convention alignment | | 2026-03-15 | Grep for callers must cover entire working tree before deleting functions | | 2026-03-14 | Stderr error messages are user-facing text that belongs in assets | -| 2026-03-14 | Subagents rename packages and modify unrelated files without being asked | | 2026-03-14 | Hardcoded _alt suffixes create implicit language favoritism | -| 2026-03-14 | Subagents reorganize file structure without being asked | -| 2026-03-14 | Internal skill rename requires updates across 6+ layers | -| 2026-03-13 | Skills without a trigger mechanism are dead code | -| 2026-03-13 | Variable shadowing causes cascading test failures after package splits | | 2026-03-13 | sync-why mechanism existed but was not wired to build | -| 2026-03-13 | Linter reverts import-only edits when references still use old package | | 2026-03-12 | Project-root files vs context files are distinct categories | | 2026-03-12 | Constants belong in their domain package not in god objects | | 2026-03-07 | Always search for existing constants before adding new ones | | 2026-03-07 | SafeReadFile requires split base+filename paths | -| 2026-03-06 | Spawned agents reliably create new files but consistently fail to delete old ones — always audit for stale files, duplicate function definitions, and orphaned imports after agent-driven refactoring | -| 2026-03-06 | Import cycle avoidance: when package A imports package B for logic, B must own shared types — A aliases them. entry imports add/core for insert logic, so add/core owns EntryParams and entry aliases it as entry.Params | | 2026-03-06 | Stale directory inodes cause invisible files over SSH | | 2026-03-06 | Stats sort uses string comparison on RFC3339 timestamps with mixed timezones | | 2026-03-06 | Claude Code supports PreCompact and SessionStart hooks that ctx does not use | -| 2026-03-06 | nolint:goconst for trivial values normalizes magic strings | | 2026-03-06 | Package-local err.go files invite broken windows from future agents | | 2026-03-05 | State directory accumulates silently without auto-prune | | 2026-03-05 | Global tombstones suppress hooks across all sessions | | 2026-03-05 | Claude Code has two separate memory systems behind feature flags | | 2026-03-05 | Blog post editorial feedback is higher-leverage than drafting | | 2026-03-04 | CONSTITUTION hook compliance is non-negotiable — don't work around it | -| 2026-03-04 | nolint:errcheck in tests normalizes unchecked errors for agents | -| 2026-03-04 | golangci-lint v2 ignores inline nolint directives for some linters | | 2026-03-02 | Hook message registry test enforces exhaustive coverage of embedded templates | | 2026-03-02 | Existing Projects is ambiguous framing for migration notes | | 2026-03-02 | Claude Code JSONL model ID does not distinguish 200k from 1M context | | 2026-03-01 | Gosec G306 flags test file WriteFile with 0644 permissions | | 2026-03-01 | Converting PersistentPreRun to PersistentPreRunE changes exit behavior | -| 2026-03-01 | Key path changes ripple across 15+ doc files and 2 skills | | 2026-03-01 | Test HOME isolation is required for user-level path functions | -| 2026-03-01 | Skill enhancement is a documentation-heavy operation across 10+ files | | 2026-03-01 | Task descriptions can be stale in reverse — implementation done but task not marked complete | -| 2026-03-01 | Elevating private skills requires synchronized updates across 6 layers | | 2026-03-01 | Model-to-window mapping requires ordered prefix matching | -| 2026-03-01 | Removing embedded asset directories requires synchronized cleanup across 5+ layers | -| 2026-03-01 | Absorbing shell scripts into Go commands creates a discoverability gap | | 2026-03-01 | TASKS.md template checkbox syntax inside HTML comments is parsed by RegExTaskMultiline | | 2026-03-01 | Hook logs had no rotation; event log already did | | 2026-02-28 | ctx pad import, ctx pad export, and ctx system resources make three hack scripts redundant | @@ -120,12 +103,106 @@ DO NOT UPDATE FOR: | 2026-02-22 | Linting and static analysis (consolidated) | | 2026-02-22 | Permission and settings drift (consolidated) | | 2026-02-22 | Gitignore and filesystem hygiene (consolidated) | -| 2026-02-19 | Feature can be code-complete but invisible to users | | 2026-01-28 | IDE is already the UI | --- +## [2026-04-04-025813] Format-verb strings are localizable text, not exempt from magic string checks + +**Context**: Strings like '%d entries checked' were passing TestNoMagicStrings because the format-verb exemption was too broad + +**Lesson**: Any string containing English words alongside format directives is user-facing text that belongs in YAML assets + +**Application**: Removed format-verb, URL-scheme, HTML-entity, and err/ exemptions from TestNoMagicStrings + +--- + +## [2026-04-04-025805] Agents add allowlist entries to make tests pass — guard every exemption + +**Context**: Found that every exemption map/allowlist in audit tests is a tempting shortcut for agents + +**Lesson**: Added DO NOT widen guard comments to all 10 exemption data structures across 7 test files + +**Application**: Every new audit test with an exemption must include the guard comment. Review PRs for drive-by allowlist additions. + +--- + +## [2026-04-03-180000] Subagent scope creep and cleanup (consolidated) + +**Consolidated from**: 4 entries (2026-03-06 to 2026-03-23) + +- Subagents reliably rename functions, restructure files, change import aliases, and modify function signatures beyond their stated scope — even narrowly scoped tasks like fixing em-dashes in comments +- Subagents create new files during refactors but consistently fail to delete the originals — always audit for stale files, duplicate definitions, and orphaned imports afterward +- After any agent-driven refactor: run `git diff --stat` and `git diff --name-only HEAD`, revert anything outside the intended scope, and check for stale package declarations before building + +--- + +## [2026-04-03-180000] Bulk rename and replace_all hazards (consolidated) + +**Consolidated from**: 3 entries (2026-03-15 to 2026-03-20) + +- `replace_all` on short tokens (e.g. `core.`, function names) matches inside longer identifiers and function definitions — `remindcore.` becomes `remindtidy.`, `func HumanAgo` becomes `func format.DurationAgo` (invalid Go) +- `sed` insert-before-first-match does not understand Go import aliases — the alias attaches to whatever line sed inserts, not the original target +- For function renames: delete the old definition separately rather than using replace_all. For bulk import additions: check for aliased imports first and handle them separately, or use goimports + +--- + +## [2026-04-03-180000] Import cycles and package splits (consolidated) + +**Consolidated from**: 5 entries (2026-03-06 to 2026-03-22) + +- Types in god-object files (e.g. hook/types.go with 15+ types from 8 domains) create circular dependencies — move types to their owning domain package +- Tests in parent package X cannot import X/sub packages that import X back — move tests to the sub-package they exercise +- Variable shadowing causes cascading failures after splits: `dir`, `file`, `entry` are common Go variable names that collide with new sub-package names — run `go test ./...` before committing splits +- When moving constants between packages, change imports and all references in a single atomic write so the linter never sees an inconsistent state +- Import cycle rule: the package providing implementation logic must own the shared types; the facade package aliases them (e.g. `entry.Params` aliases `add/core.EntryParams`) + +--- + +## [2026-04-03-180000] Lint suppression and gosec patterns (consolidated) + +**Consolidated from**: 4 entries (2026-03-04 to 2026-03-19) + +- Rename constants to avoid gosec G101 false positives (Tokens->Usage, Passed->OK) instead of adding nolint/nosec/path exclusions — exclusions break on file reorganization +- `nolint:goconst` for trivial values normalizes magic strings — use config constants instead of suppressing the linter +- `nolint:errcheck` in tests teaches agents to spread the pattern to production code — use `t.Fatal(err)` for setup, `defer func() { _ = f.Close() }()` for cleanup +- golangci-lint v2 ignores inline nolint directives for some linters — use config-level `exclusions.rules` for gosec patterns, fix the code instead of suppressing errcheck + +--- + +## [2026-04-03-180000] Skill lifecycle and promotion (consolidated) + +**Consolidated from**: 4 entries (2026-03-01 to 2026-03-14) + +- Internal skill renames and promotions require synchronized updates across 6+ layers: SKILL.md frontmatter, internal cross-references, external docs, embed_test.go expected list, recipe/reference docs, and plugin cache rebuild + session restart +- Skill behavior changes ripple through hook messages, fallback strings in Go code, doc descriptions, and Makefile hints — grep for the skill name across the entire repo +- Skills without a trigger mechanism (no user invocation, no hook loading) are dead code — audit skills for reachability +- After promoting skills: grep -r for the old name across the whole tree, run plugin-reload.sh, restart session to verify autocomplete, and clean stale Skill() entries from settings.local.json + +--- + +## [2026-04-03-180000] Cross-cutting change ripple (consolidated) + +**Consolidated from**: 4 entries (2026-02-19 to 2026-03-01) + +- Path changes (e.g. key file location) ripple across 15+ doc files and 2 skills — grep broadly (not just code) and budget for 15+ file touches +- Removing embedded asset directories requires synchronized cleanup across 5+ layers: embed directive, accessor functions, callers, tests, config constants, build targets, documentation — work outward from the embed +- Absorbing shell scripts into Go commands creates a discoverability gap — update contributing.md, common-workflows.md, and CLI index as part of the absorption checklist +- A feature without docs is invisible to users: always check feature page, cli-reference.md, relevant recipes, and zensical.toml nav after implementing a new CLI subcommand + +--- + +## [2026-04-03-180000] Dead code detection (consolidated) + +**Consolidated from**: 3 entries (2026-03-15 to 2026-03-30) + +- Dead packages can build and test green while being completely unreachable — detection requires checking bootstrap registration, not just build success (e.g. internal/cli/recall/ existed with tests but was never wired into the command tree) +- Files created by `ctx init` that no agent, hook, or skill ever reads are dead on arrival — verify there is at least one consumer before adding to init scaffolding +- When touching legacy compat code, first ask whether the legacy path has real users — if not, delete it entirely rather than improving it (MigrateKeyFile had 5 callers and test coverage but zero users) + +--- + ## [2026-04-03-133244] desc.Text() is the single highest-connectivity symbol in the codebase **Context**: GitNexus enrichment during architecture analysis revealed desc.Text() (internal/assets/read/desc/desc.go:75) has 30+ direct callers spanning every architectural layer (MCP handler, format, index, tidy, trace, memory, sysinfo, io) and participates in 53 execution flows. @@ -246,16 +323,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-30-003720] internal/cli/recall/ was dead code — never registered in bootstrap - -**Context**: The entire recall CLI package existed with tests but was never wired into the command tree. Journal consumed it but nobody deleted the ghost - -**Lesson**: Dead package detection requires checking bootstrap registration, not just build success. A package can build and test green while being completely unreachable - -**Application**: Add a compliance test that verifies all cli/ packages are registered in bootstrap - ---- - ## [2026-03-30-003707] lint-docstrings.sh greedy sed hid all return-type violations **Context**: sed 's/.*) //' consumed return type parens, leaving { — functions with return types were invisible to the script for months @@ -276,16 +343,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-25-173339] Dead files accumulate when nothing consumes them - -**Context**: IMPLEMENTATION_PLAN.md and PROMPT.md were created by ctx init but no agent, hook, or skill ever read them - -**Lesson**: Before adding a file to init scaffolding, verify there is at least one consumer. Periodically audit what init creates vs what the system reads. - -**Application**: The prompt deprecation spec documents the reasoning as a papertrail for future removals. - ---- - ## [2026-03-25-173338] Template improvements don't propagate to existing projects **Context**: 5 of 8 context files in the ctx project itself had stale/missing comment headers — templates evolved but non-destructive init never re-synced them @@ -326,16 +383,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-23-165610] Subagents rename functions and restructure code beyond their scope - -**Context**: Agents tasked with fixing em-dashes in comments also renamed exported functions, changed import aliases, and modified function signatures - -**Lesson**: Always diff-audit agent output for structural changes before accepting edits, even when the task is narrowly scoped - -**Application**: After any agent batch edit, run git diff --stat and scan for non-comment changes before staging - ---- - ## [2026-03-23-003544] Splitting core/ into subpackages reveals hidden structure **Context**: init core/ was a flat bag of domain objects — splitting into backup/, claude/, entry/, merge/, plan/, plugin/, project/, prompt/, tpl/, validate/ exposed duplicated logic, misplaced types, and function-pointer smuggling that were invisible in the flat layout @@ -356,26 +403,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-22-220846] Types in god-object files create circular dependencies - -**Context**: hook/types.go had 15+ types from 8 domains; session importing hook for SessionTokenInfo created a cycle - -**Lesson**: Moving types to their owning domain package breaks import cycles - -**Application**: When a type is only used by one domain package, move it there. Check with grep before assuming a type is cross-cutting. - ---- - -## [2026-03-20-232518] replace_all on short tokens like function names will also replace their definitions - -**Context**: Using replace_all to rename HumanAgo to format.DurationAgo also changed the func declaration line to func format.DurationAgo which is invalid Go - -**Lesson**: replace_all matches everywhere in the file including function definitions, not just call sites - -**Application**: When renaming functions, delete the old definition separately rather than using replace_all. Or use a more specific match pattern that excludes func declarations. - ---- - ## [2026-03-20-160112] Commit messages containing script paths trigger PreToolUse hooks **Context**: Git commit message body contained a path to a shell script under the hack directory which matched a hook pattern that blocks direct script invocation @@ -386,36 +413,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-19-194942] Rename constants to avoid gosec G101 false positives - -**Context**: Constants named ColTokens, DriftPassed, StatusTokensFormat triggered gosec G101 credential detection. Suppressing with nolint or broadening .golangci.yml exclusion paths is fragile — paths change when files split. - -**Lesson**: Rename the constant to avoid the trigger word instead of suppressing the lint. Tokens→Usage, Passed→OK convey the same semantics without false positives. This is cleaner than nolint annotations or path-based exclusions that break on file reorganization. - -**Application**: When gosec flags a constant name, ask what the value semantically represents and rename to that. Do not add nolint, nosec, or path exclusions. - ---- - -## [2026-03-18-193616] Tests in package X cannot import X/sub packages that import X back - -**Context**: embed_test.go in package assets kept importing read/ sub-packages that import assets.FS, creating cycles. Recurred 4 times this session. - -**Lesson**: Tests that exercise domain accessor packages must live in those packages, not in the parent. The parent test file can only use the shared resource (FS) directly. - -**Application**: When splitting functions from a parent package into sub-packages, move the corresponding tests too. Do not leave them in the parent. - ---- - -## [2026-03-18-193602] Bulk sed on imports displaces aliased imports - -**Context**: Used sed to add desc import to 278 files. Files with aliased imports like ctxCfg config/ctx got the alias stolen by the new import line inserted before it. - -**Lesson**: sed insert-before-first-match does not understand Go import aliases. The alias attaches to whatever import line sed inserts, not the original target. - -**Application**: When bulk-adding imports, check for aliased imports first and handle them separately. Or use goimports if available. - ---- - ## [2026-03-18-133457] Lazy sync.Once per-accessor is a code smell for static embedded data **Context**: assets package had 4 sync.Once guards, 4 exported maps, 4 Load*() functions, and a wrapper desc package — all to lazily load YAML from embed.FS that never mutates. Every accessor call went through sync.Once + global map + wrapper indirection. @@ -476,26 +473,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-15-230643] replace_all on short tokens like core. mangles aliased imports - -**Context**: Used Edit tool replace_all to change core. to tidy. across handle_tool.go - -**Lesson**: Short replacement tokens match inside longer identifiers — remindcore. became remindtidy. silently - -**Application**: When doing bulk renames, prefer targeted replacements or grep for collateral damage immediately after. Avoid replace_all on tokens shorter than the full identifier - ---- - -## [2026-03-15-101346] Delete legacy code instead of maintaining it — MigrateKeyFile had 5 callers and test coverage but zero users - -**Context**: Started by adding constants for legacy key names, then realized nobody uses legacy keys - -**Lesson**: When touching legacy compat code, first ask whether the legacy path has real users. If not, delete it entirely rather than improving it - -**Application**: Apply to any backward-compat shim: check actual usage before investing in maintenance - ---- - ## [2026-03-15-101342] Contributor PRs need post-merge follow-up commits for convention alignment **Context**: PR #42 (MCP v0.2) addressed bulk of review feedback but left ~12 inline strings, no embed_test coverage, and substring matching in containsOverlap @@ -526,16 +503,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-14-180902] Subagents rename packages and modify unrelated files without being asked - -**Context**: ET agent renamed internal/eventlog/ to internal/log/ and modified 5+ caller files outside the internal/err/ scope - -**Lesson**: Always diff agent output against HEAD to catch scope creep before building; revert unrelated changes immediately - -**Application**: After any agent-driven refactor, run git diff --name-only HEAD and revert anything outside the intended scope before testing - ---- - ## [2026-03-14-131202] Hardcoded _alt suffixes create implicit language favoritism **Context**: Session parser had session_prefix_alt hardcoding Turkish as a special case alongside English default @@ -546,46 +513,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-14-110750] Subagents reorganize file structure without being asked - -**Context**: Asked subagent to replace os.ReadFile callsites with validation wrappers. It also moved functions to new files, renamed them (ReadUserFile to SafeReadUserFile), and created a new internal/io package. - -**Lesson**: Subagents optimize for clean results and will restructure beyond stated scope — moved, renamed, and split files without being asked. - -**Application**: After subagent-driven refactors, always verify file organization matches intent. Audit for moved, renamed, and split files, not just the requested callsite changes. - ---- - -## [2026-03-14-093757] Internal skill rename requires updates across 6+ layers - -**Context**: Renamed ctx-alignment-audit to _ctx-alignment-audit. The allow list test in embed_test.go failed because it iterates all bundled skills and expects each in the allow list. - -**Lesson**: The allow list test needed a strings.HasPrefix(_) skip for internal skills. This was not obvious until tests ran. - -**Application**: When converting public to internal skills, audit: allow.txt, embed_test.go allow list test, reference/skills.md, all recipe docs referencing the skill, contributing.md dev-only skills table, and permissions docs. - ---- - -## [2026-03-13-223110] Skills without a trigger mechanism are dead code - -**Context**: ctx-context-monitor was a skill documenting how to respond to hook output, but no hook or agent ever loaded it. The hook output already contained sufficient instructions. - -**Lesson**: A skill only enters the agent context when explicitly invoked via /skill-name. If the description says not user-invocable and no mechanism loads it automatically, it is unreachable. - -**Application**: Audit skills for reachability. If nothing triggers the skill, either add a trigger or delete it. - ---- - -## [2026-03-13-223108] Variable shadowing causes cascading test failures after package splits - -**Context**: Large refactoring moved constants from monolithic config package to sub-packages (dir, entry, file). Test files had local variables named dir, entry, file that shadowed the new package imports. - -**Lesson**: When splitting a package, audit test files for local variable names that collide with new package names. dir, file, entry are especially common Go variable names. - -**Application**: Before committing a package split, run go test ./... and check for undefined errors caused by variable shadowing. - ---- - ## [2026-03-13-151952] sync-why mechanism existed but was not wired to build **Context**: assets/why/ had drifted from docs/ — the sync targets existed in the Makefile but build did not depend on sync-why @@ -596,16 +523,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-13-151951] Linter reverts import-only edits when references still use old package - -**Context**: Moving tpl_entry.go from config/entry to assets — linter kept reverting the import change - -**Lesson**: When moving constants between packages, change imports and all references in a single atomic write (use Write not incremental Edit), so the linter never sees an inconsistent state - -**Application**: For future package migrations, use full file rewrites when a linter is active - ---- - ## [2026-03-12-133008] Project-root files vs context files are distinct categories **Context**: Tried moving ImplementationPlan constant to config/ctx assuming it was a context file. (Note: IMPLEMENTATION_PLAN.md was removed in 2026-03-25 as a dead file — no agent consumer.) @@ -646,26 +563,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-06-200319] Spawned agents reliably create new files but consistently fail to delete old ones — always audit for stale files, duplicate function definitions, and orphaned imports after agent-driven refactoring - -**Context**: Multiple agent batches across cmd/ restructuring, color removal, and flag externalization left stale files, duplicate run.go, and unupdated parent imports - -**Lesson**: Agent cleanup is a known gap — budget 5-10 minutes for post-agent audit per batch - -**Application**: After every agent batch: grep for stale package declarations, check parent imports point to cmd/root not cmd/, verify old files are deleted - ---- - -## [2026-03-06-200237] Import cycle avoidance: when package A imports package B for logic, B must own shared types — A aliases them. entry imports add/core for insert logic, so add/core owns EntryParams and entry aliases it as entry.Params - -**Context**: Extracting entry.Params as a standalone struct in internal/entry created a cycle because entry/write.go imports add/core for AppendEntry - -**Lesson**: The package that provides implementation logic must own the types; the facade package aliases them - -**Application**: When extracting shared types from implementation packages, check the import direction first — the type lives where the logic lives - ---- - ## [2026-03-06-141506] Stale directory inodes cause invisible files over SSH **Context**: Files created by Claude Code hooks were visible inside the VM but not from the SSH terminal @@ -696,16 +593,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-06-050126] nolint:goconst for trivial values normalizes magic strings - -**Context**: Found 5 callsites suppressing goconst with nolint:goconst trivial user input check for y/yes comparisons - -**Lesson**: Suppressing a linter with a trivial justification sets precedent for other agents to do the same. The fix (two constants) costs less than the accumulated tech debt. - -**Application**: Use config.ConfirmShort/config.ConfirmLong instead of suppressing goconst. Prefer constants over nolint directives. - ---- - ## [2026-03-06-050125] Package-local err.go files invite broken windows from future agents **Context**: Found err.go files in 5 CLI packages with heavily duplicated error constructors (errFileWrite, errMkdir, errZensicalNotFound repeated across packages) @@ -766,26 +653,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-04-040211] nolint:errcheck in tests normalizes unchecked errors for agents - -**Context**: User flagged that suppressing errcheck in tests teaches the agent to spread the pattern to production code - -**Lesson**: Broken-window theory applies to lint suppressions. Agents learn from test code patterns. Use _ = f.Close() in a closure or check errors with t.Fatal — never suppress with nolint. - -**Application**: Handle all errors in test code the same as production: t.Fatal(err) for setup, defer func() { _ = f.Close() }() for best-effort cleanup. - ---- - -## [2026-03-04-040209] golangci-lint v2 ignores inline nolint directives for some linters - -**Context**: nolint:errcheck and nolint:gosec comments were present but golangci-lint v2 still reported violations - -**Lesson**: In golangci-lint v2, use config-level exclusions.rules for gosec patterns (G204, G301, G306) rather than relying on inline nolint directives. For errcheck, fix the code instead of suppressing. - -**Application**: When adding new lint suppressions, prefer config-level rules for gosec false positives on safe paths/args; never suppress errcheck — handle the error. - ---- - ## [2026-03-02-165039] Hook message registry test enforces exhaustive coverage of embedded templates **Context**: Adding billing.txt to embedded assets without a registry entry caused TestRegistryCoversAllEmbeddedFiles to fail immediately @@ -836,16 +703,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-01-194147] Key path changes ripple across 15+ doc files and 2 skills - -**Context**: Updating docs for the .context/.ctx.key → ~/.local/ctx/keys/ → ~/.ctx/.ctx.key migrations - -**Lesson**: Key path changes have a long documentation tail — recipes, references, getting-started, operations, CLI docs, and skills all carry path references. The worktree behavior flip (limitation to works automatically) was the highest-value change per line edited. Simplifying from per-project slugs to a single global key eliminated more code and docs than the original migration added. - -**Application**: When moving a file path that appears in user-facing docs, grep broadly (not just code) and budget for 15+ file touches - ---- - ## [2026-03-01-161459] Test HOME isolation is required for user-level path functions **Context**: After adding ~/.ctx/.ctx.key as global key location, test suites wrote real files to the developer home directory @@ -856,16 +713,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-01-144544] Skill enhancement is a documentation-heavy operation across 10+ files - -**Context**: Enhancing /ctx-journal-enrich-all to handle export-if-needed touched the skill, hook messages, fallback strings, 5 doc files, 2 Makefiles, and TASKS.md - -**Lesson**: Skill behavior changes ripple through hook messages, fallback strings in Go code, doc descriptions, and Makefile hints — all must stay synchronized - -**Application**: When modifying a skill's scope, grep for its name across the entire repo and update every description, not just the skill file itself - ---- - ## [2026-03-01-133014] Task descriptions can be stale in reverse — implementation done but task not marked complete **Context**: ctx recall sync task said 'command is not registered in Cobra' but the code was fully wired and all tests passed. The task description was stale. @@ -876,16 +723,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-01-125807] Elevating private skills requires synchronized updates across 6 layers - -**Context**: Promoted 6 _ctx-* skills to bundled ctx-* plugin skills - -**Lesson**: Moving a skill from .claude/skills/ to internal/assets/claude/skills/ touches: (1) SKILL.md frontmatter name field, (2) internal cross-references between skills (slash command paths), (3) external cross-references in other skills and docs, (4) embed_test.go expected skill list, (5) recipe and reference docs that mention the old name, (6) plugin cache rebuild (`hack/plugin-reload.sh`) + session restart — Claude Code snapshots skills from `~/.claude/plugins/cache/` at startup, so new skills are invisible until the cache is refreshed. Also clean stale underscore-prefixed `Skill(_ctx-*)` entries from `.claude/settings.local.json`. - -**Application**: When promoting future skills, use grep -r /_ctx-{name} across the whole tree before declaring done. After code changes, run plugin-reload.sh and restart the session to verify the skill appears in autocomplete. - ---- - ## [2026-03-01-124921] Model-to-window mapping requires ordered prefix matching **Context**: Implementing modelContextWindow() for the three-tier context window fallback. Claude model IDs use nested prefixes (claude-sonnet-4-5 vs claude-sonnet-4-20250514). @@ -896,26 +733,6 @@ DO NOT UPDATE FOR: --- -## [2026-03-01-112538] Removing embedded asset directories requires synchronized cleanup across 5+ layers - -**Context**: Deleting .context/tools/ deployment touched embed directive, asset functions, init logic, tests, config constants, Makefile targets, and docs — missing any one layer leaves dead code or build failures. - -**Lesson**: Embedded asset removal is a cross-cutting concern: embed directive → accessor functions → callers → tests → config constants → build targets → documentation. Work outward from the embed. - -**Application**: When removing an embedded asset category, use the grep-first approach (search for all references to the accessor functions and constants) before deleting anything. - ---- - -## [2026-03-01-102232] Absorbing shell scripts into Go commands creates a discoverability gap - -**Context**: Deleted make backup/backup-global/backup-all and make rc-dev/rc-base/rc-status targets when absorbing into ctx system backup and ctx config switch. The Makefile served as self-documenting discovery (make help). - -**Lesson**: When eliminating Makefile targets, the CLI reference page alone is not sufficient — contributor-facing docs (contributing.md) and command catalogs (common-workflows.md) must gain explicit entries to compensate. - -**Application**: For future hack/ absorptions (e.g. pad-import-ideas.sh, context-watch.sh), audit contributing.md, common-workflows.md CLI-Only table, and the CLI index page as part of the absorption checklist. - ---- - ## [2026-03-01-095709] TASKS.md template checkbox syntax inside HTML comments is parsed by RegExTaskMultiline **Context**: Template had example checkboxes (- [x], - [ ]) in HTML comments that the line-based regex matched as real tasks, causing TestArchiveCommand_NoCompletedTasks to fail @@ -1180,16 +997,6 @@ DO NOT UPDATE FOR: --- -## [2026-02-19-215200] Feature can be code-complete but invisible to users - -**Context**: ctx pad merge was fully implemented with 19 passing tests and binary support, but had zero coverage in user-facing docs (scratchpad.md, cli-reference.md, scratchpad-sync recipe). Only discoverable via --help. - -**Lesson**: Implementation completeness \!= user-facing completeness. A feature without docs is invisible to users who don't explore CLI help. - -**Application**: After implementing a new CLI subcommand, always check: feature page, cli-reference.md, relevant recipes, and zensical.toml nav (if new page). - ---- - ## [2026-01-28-051426] IDE is already the UI **Context**: Considering whether to build custom UI for .context/ files @@ -1202,5 +1009,6 @@ git integration, extensions - all free. --- + *Module-specific, niche, and historical learnings: [learnings-reference.md](learnings-reference.md)* diff --git a/.context/TASKS.md b/.context/TASKS.md index 31100f257..77df2400d 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -25,6 +25,15 @@ TASK STATUS LABELS: `#in-progress`: currently being worked on (add inline, don't move task) --> +### Misc + +- [ ] gitnexus analyze --embeddings --skill : we need a make target, but also this + butchers AGENTS.md and CLAUDE.md so those files need to be reverted and + GITNEXUS.md should be updated accordingly. The make target should remind + the user about it. + +- [ ] SMB mount path support: add `CTX_BACKUP_SMB_MOUNT_PATH` env var so `ctx backup` can use fstab/systemd automounts instead of requiring GVFS. Spec: specs/smb-mount-path-support.md #priority:medium #added:2026-04-04-010000 + ### Architecture Docs - [ ] Publish architecture docs to docs/: copy ARCHITECTURE.md, DETAILED_DESIGN domain files, and CHEAT-SHEETS.md to docs/reference/. Sanitize intervention points into docs/contributing/. Exclude DANGER-ZONES.md and ARCHITECTURE-PRINCIPAL.md (internal only). Spec: specs/publish-architecture-docs.md #priority:medium #added:2026-04-03-150000 @@ -33,7 +42,6 @@ TASK STATUS LABELS: ### Code Cleanup Findings -- [x] Add TestTypeFileConvention audit check: type definitions must live in types.go, not mixed into function files. Scan all non-test .go files for ast.TypeSpec declarations; flag any that appear in files not named types.go. Migrate violations. #priority:medium #added:2026-04-03-030033 #done:2026-04-03 - [ ] Extend flagbind helpers (IntFlag, DurationFlag, DurationFlagP, StringP, BoolP) and migrate ~50 call sites to unblock TestNoFlagBindOutsideFlagbind #added:2026-04-01-233250 @@ -45,21 +53,12 @@ TASK STATUS LABELS: and types from cmd/ directories to core/. See grandfathered map in compliance_test.go for the full list. #priority:medium #added:2026-03-31-005115 -- [x] Collect all exec.Commands under internal/exec. See - Phase EXEC below for breakdown. — done, exec/{git,dep,gio,zensical} - exist, no exec.Command calls remain outside internal/exec - #done:2026-03-31 - [ ] PD.4.5: Update AGENT_PLAYBOOK.md — add generic "check available skills" instruction #priority:medium #added:2026-03-25-203340 **PD.5 — Validate:** -- [x] PD.5.2: Run `ctx init` on a clean directory — verify no - `.context/prompts/` created. loop.md and skills checks are stale: - loop.md was never a ctx init artifact (ctx loop generates on demand), - skills deploy via plugin install, not ctx init. - #priority:high #added:2026-03-25-203340 #done:2026-03-31 ### Phase -3: DevEx @@ -92,15 +91,6 @@ TASK STATUS LABELS: .ctxrc or settings.json. Related: consolidation nudge hook spec. #added:2026-03-23-223500 -- [x] Bug: check-version hook missing throttle touch on plugin - version read error (run.go:70). When claude.PluginVersion() - fails, the hook returns without touching the daily throttle - marker, causing repeated checks on days when plugin.json is - missing or corrupted. Fix: add - internalIo.TouchFile(markerFile) before the early return. - See docs/recipes/hook-sequence-diagrams.md check-version - diagram which documents the expected behavior. - #added:2026-03-23-162802 #done:2026-03-31 - [ ] Design UserPromptSubmit hook that runs go build and surfaces compilation errors before the agent acts on stale @@ -305,9 +295,6 @@ P0.4.10 task. one file; types need to go to types.go per convention etc etc) * Human: split err package into sub packages. -- [x] Add Use* constants for all system subcommands — all 30 system - subcommands already use cmd.UseSystem* constants - #added:2026-03-21-092550 #done:2026-03-31 - [ ] Refactor site/cmd/feed: extract helpers and types to core/, make Run public #added:2026-03-21-074859 @@ -453,15 +440,6 @@ Many call sites use `_ =` or `_, _ =` to discard errors without any feedback. Some are legitimate (best-effort cleanup), most are lazy escapes that hide failures. -- [x] EH.0: Create central warning sink — `internal/log/warn/warn.go` with - `var sink io.Writer = os.Stderr` and `func Warn(format string, - args ...any)`. - All stderr warnings (`fmt.Fprintf(os.Stderr, ...)`) route through this - function. The `fmt.Fprintf` return error is handled once, centrally. - The sink is swappable (tests use `io.Discard`, future: syslog, file). - EH.2–EH.4 should use `log.Warn()` instead of raw `fmt.Fprintf`. - DoD: `grep -rn 'fmt.Fprintf(os.Stderr' internal/` returns zero hits - #priority:high #added:2026-03-15 - [ ] EH.1: Catalogue all silent error discards — recursive walk of `internal/` for patterns: `_ = `, `_, _ = `, `//nolint:errcheck`, bare `return` after @@ -596,34 +574,11 @@ Taxonomy (from prefix analysis): state.go to types.go — file and type no longer exist, refactored away #priority:low #added:2026-03-07-220825 -- [x] Cleanup internal/cli/system/core/wrapup.go: line 18 constant should go to - config; make WrappedUpExpiry configurable via ctxrc — already done, - wrap.ExpiryHours and wrap.Marker exist in config/wrap - #priority:low #added:2026-03-07-220825 #done:2026-03-31 -- [x] Cleanup internal/cli/system/core/version.go: line 81 newline should come - from config — already done, uses token.NewlineLF - #priority:low #added:2026-03-07-220819 #done:2026-03-31 -- [x] Add taxonomy to internal/cli/system/core/ — currently an unstructured bag - of files; group by domain (backup, hooks, session, knowledge, etc.) - — already done, 20 domain subdirectories exist - #priority:medium #added:2026-03-07-220819 #done:2026-03-31 -- [x] Cleanup internal/cli/system/core/version_drift.go: line 53 string - formatting should use assets — file moved to core/drift/, now uses - desc.Text and assets throughout - #priority:medium #added:2026-03-07-220819 #done:2026-03-31 -- [x] Cleanup internal/cli/system/core/state.go: magic permissions (0o750), - magic strings ('Context: ' prefix, etc.) — file moved to core/state/, - magic values extracted to config - #priority:medium #added:2026-03-07-220819 #done:2026-03-31 -- [x] Cleanup internal/cli/system/core/smb.go: errors should come from - internal/err; lines 101, 116, 111 need assets text — file moved to - core/archive/, errors routed through err package - #priority:medium #added:2026-03-07-220819 #done:2026-03-31 - [ ] Make AutoPruneStaleDays configurable via ctxrc. Currently hardcoded to 7 days in config.AutoPruneStaleDays; add a ctxrc key (e.g., auto_prune_days) and @@ -714,19 +669,11 @@ Taxonomy (from prefix analysis): tables) #added:2026-03-06-190225 -- [x] Remove FlagNoColor and fatih/color dependency — replaced with plain - output, dependency removed from go.mod - #added:2026-03-06-182831 #done:2026-03-31 - [ ] Validate .ctxrc against ctxrc.schema.json at load time — schema is embedded but never enforced, doctor does field-level checks without using it #added:2026-03-06-174851 -- [x] Fix 3 CI compliance issues from PR #27 after merge: missing copyright - header on internal/mcp/server_test.go, missing doc.go for internal/cli/mcp/, - literal newlines in internal/mcp/resources.go and - tools.go — all fixed, files moved to mcp/server/ with copyright - #added:2026-03-06-141508 #done:2026-03-31 - [ ] Add PostToolUse session event capture. Append lightweight event records (tool name, files touched, timestamp) to .context/state/session-events.jsonl diff --git a/.context/archive/decisions-consolidated-2026-04-03.md b/.context/archive/decisions-consolidated-2026-04-03.md new file mode 100644 index 000000000..0f89a5087 --- /dev/null +++ b/.context/archive/decisions-consolidated-2026-04-03.md @@ -0,0 +1,229 @@ +# Archived Decisions (consolidated 2026-04-03) + +Originals replaced by consolidated entries in DECISIONS.md. + +## Group: Output/write location + +## [2026-03-22-084509] No runtime pluralization — use singular/plural text key pairs + +**Status**: Accepted + +**Context**: Hardcoded English plural rules (+ s, y → ies) were scattered across format.Pluralize, padPluralize, and inline code — all i18n dead-ends + +**Decision**: No runtime pluralization — use singular/plural text key pairs + +**Rationale**: Different languages have vastly different plural rules. Complete sentence templates with embedded counts (time.minute-count '1 minute', time.minutes-count '%d minutes') let each locale define its own plural forms. + +**Consequence**: format.Pluralize and format.PluralWord are deleted. All plural output uses paired text keys with the count embedded in the template. + +--- + +## [2026-03-21-084020] Output functions belong in write/, logic and types in core/ + +**Status**: Accepted + +**Context**: PrintFeedReport was initially placed in cli/site/core/ but it calls cmd.Println — that's output formatting, not business logic + +**Decision**: Output functions belong in write/, logic and types in core/ + +**Rationale**: The project taxonomy separates concerns: core/ owns domain logic, types, and helpers; write/ owns CLI output formatting that takes *cobra.Command for Println. Mixing them blurs the boundary and makes testing harder. + +**Consequence**: All functions that call cmd.Print/Println/Printf belong in the write/ package tree. core/ never imports cobra for output purposes. + +--- + + +## Group: YAML text externalization + +## [2026-04-03-133236] YAML text externalization justification is agent legibility, not i18n + +**Status**: Accepted + +**Context**: Principal analysis initially framed 879-key YAML externalization as a bet on i18n. Blog post review (v0.8.0) revealed the real justification: agent legibility (named DescKey constants as traversable graphs), drift prevention (TestDescKeyYAMLLinkage catches orphans mechanically), and completing the archaeology of finding all 879 scattered strings. + +**Decision**: YAML text externalization justification is agent legibility, not i18n + +**Rationale**: The v0.8.0 blog makes it explicit: finding strings is the hard part, translation is mechanical. The externalization already pays for itself through agent legibility and mechanical verification. i18n is a free downstream consequence, not the justification. + +**Consequence**: Future architecture analysis should frame externalization as already-justified investment. The 3-file ceremony (DescKey + YAML + write/err function) is the cost of agent-legible, drift-proof output — not speculative i18n prep. + +--- + +## [2026-03-15-101336] TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values + +**Status**: Accepted + +**Context**: PR #42 merged with ~80 new MCP text keys but no test coverage for key-to-YAML mapping + +**Decision**: TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values + +**Rationale**: A single table-driven test parsing embed.go source catches typos and missing YAML entries at test time — no manual key list to maintain + +**Consequence**: New TextDescKey constants are automatically covered; orphaned keys fail CI + +--- + +## [2026-03-15-040638] Split text.yaml into 6 domain files loaded via loadYAMLDir + +**Status**: Accepted + +**Context**: text.yaml grew to 1812 lines covering write, errors, mcp, doctor, hooks, and ui domains + +**Decision**: Split text.yaml into 6 domain files loaded via loadYAMLDir + +**Rationale**: Matches existing split pattern (commands.yaml, flags.yaml, examples.yaml); loadYAMLDir merges all files in commands/text/ transparently so TextDesc() API stays unchanged + +**Consequence**: New domain files must go into commands/text/; loadYAMLDir reads all .yaml in the directory at init time + +--- + +## [2026-03-12-133007] Split commands.yaml into 4 domain files + +**Status**: Accepted + +**Context**: Single 2373-line YAML mixed commands, flags, text, and examples with inconsistent quoting + +**Decision**: Split commands.yaml into 4 domain files + +**Rationale**: Context is for humans — localization files should be human-readable block scalars. Separate files eliminate the underscore prefix namespace hack + +**Consequence**: 4 files (commands.yaml, flags.yaml, text.yaml, examples.yaml) with dedicated loaders in embed.go + +--- + +## [2026-03-06-200257] Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() + +**Status**: Accepted + +**Context**: Command descriptions were inline strings scattered across 105 cobra.Command definitions + +**Decision**: Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() + +**Rationale**: Centralizing user-facing text in a single translatable file prepares for i18n without runtime cost (embedded at compile time) + +**Consequence**: System's 30 hidden hook subcommands excluded (not user-facing); flag descriptions use _flags.scope.name convention + +--- + + +## Group: Package taxonomy and code placement + +## [2026-03-06-200247] cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd//{cmd.go,run.go}, shared helpers in core/ + +**Status**: Accepted + +**Context**: 35 CLI packages had inconsistent flat structures mixing Cmd(), run logic, helpers, and types in the same directory + +**Decision**: cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd//{cmd.go,run.go}, shared helpers in core/ + +**Rationale**: Taxonomical symmetry: every package has the same predictable shape, making navigation instant and agent-friendly + +**Consequence**: cmd/ contains only cmd.go + run.go; helpers go to core/; 474 files changed in initial restructuring + +--- + +## [2026-03-06-200227] Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage + +**Status**: Accepted + +**Context**: External consumers were importing cli/add for EntryParams/ValidateEntry/WriteEntry, creating a leaky abstraction + +**Decision**: Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage + +**Rationale**: Domain types in CLI packages force consumers to depend on CLI internals; internal/entry provides a clean boundary + +**Consequence**: entry aliases Params from add/core to avoid import cycle (entry imports add/core for insert logic); future work may move insert logic to entry to eliminate the cycle + +--- + +## [2026-03-13-151954] Templates and user-facing text live in assets, structural constants stay in config + +**Status**: Accepted + +**Context**: Ongoing refactoring session moving Tpl* constants out of config/ + +**Decision**: Templates and user-facing text live in assets, structural constants stay in config + +**Rationale**: config/ is for structural constants (paths, limits, regexes); assets/ is for templates, labels, and text that would need i18n. Clean separation of concerns + +**Consequence**: All tpl_entry.go, tpl_journal.go, tpl_loop.go, tpl_recall.go moved to assets/ + +--- + + +## Group: Eager init over lazy loading + +## [2026-03-18-193631] Eager Init() for static embedded data instead of per-accessor sync.Once + +**Status**: Accepted + +**Context**: 4 sync.Once guards + 4 exported maps + 4 Load functions + a wrapper package for YAML that never mutates. + +**Decision**: Eager Init() for static embedded data instead of per-accessor sync.Once + +**Rationale**: Data is static and required at startup. sync.Once per accessor is cargo cult. One Init() in main.go is sufficient. Tests call Init() in TestMain. + +**Consequence**: Maps unexported, accessors are plain lookups, permissions and stopwords also loaded eagerly. Zero sync.Once remains in the lookup pipeline. + +--- + +## [2026-03-16-104143] Explicit Init over package-level init() for resource lookup + +**Status**: Accepted + +**Context**: server/resource package used init() to silently build the URI lookup map + +**Decision**: Explicit Init over package-level init() for resource lookup + +**Rationale**: Implicit init hides startup dependencies, makes ordering unclear, and is harder to test. Explicit Init called from NewServer makes the dependency visible. + +**Consequence**: res.Init() called explicitly from NewServer before ToList(); no package-level side effects + +--- + + +## Group: Pure logic separation of concerns + +## [2026-03-15-230640] Pure-logic CompactContext with no I/O — callers own file writes and reporting + +**Status**: Accepted + +**Context**: MCP server and CLI compact command both implemented task compaction independently, with the MCP handler using a local WriteContextFile wrapper + +**Decision**: Pure-logic CompactContext with no I/O — callers own file writes and reporting + +**Rationale**: Separating pure logic from I/O lets both MCP (JSON-RPC responses) and CLI (cobra cmd.Println) callers control output and file writes. Eliminates duplication and the unnecessary mcp/server/fs package + +**Consequence**: tidy.CompactContext returns a CompactResult struct; callers iterate FileUpdates and write them. Archive logic stays in callers since MCP and CLI have different archive policies + +--- + +## [2026-03-16-122033] Server methods only handle dispatch and I/O, not struct construction + +**Status**: Accepted + +**Context**: MCP server had ok/error/writeError as methods plus prompt builders that didn't use Server state — they just constructed response structs + +**Decision**: Server methods only handle dispatch and I/O, not struct construction + +**Rationale**: Methods that don't access receiver state hide their true dependencies and inflate the Server interface. Free functions make the dependency graph explicit and are independently testable. + +**Consequence**: New response helpers go in server/out, prompt builders in server/prompt. Server methods are limited to dispatch (handlePromptsGet) and I/O (writeJSON, emitNotification). Same principle applies to future tool/resource builders. + +--- + +## [2026-03-23-003346] Pure-data param structs in entity — replace function pointers with text keys + +**Status**: Accepted + +**Context**: MergeParams had UpdateFn callback, DeployParams had ListErr/ReadErr function pointers — both smuggled side effects into data structs + +**Decision**: Pure-data param structs in entity — replace function pointers with text keys + +**Rationale**: Text keys are pure data, keep entity dependency-free, and the consuming function can do the dispatch itself + +**Consequence**: All cross-cutting param structs in entity must be function-pointer-free; I/O functions passed as direct parameters + +--- + + diff --git a/.context/archive/learnings-consolidated-2026-04-03.md b/.context/archive/learnings-consolidated-2026-04-03.md new file mode 100644 index 000000000..851ed7f74 --- /dev/null +++ b/.context/archive/learnings-consolidated-2026-04-03.md @@ -0,0 +1,295 @@ +# Archived Learnings (consolidated 2026-04-03) + +Originals replaced by consolidated entries in LEARNINGS.md. + +## Group: Subagent scope creep and cleanup + +## [2026-03-23-165610] Subagents rename functions and restructure code beyond their scope + +**Context**: Agents tasked with fixing em-dashes in comments also renamed exported functions, changed import aliases, and modified function signatures + +**Lesson**: Always diff-audit agent output for structural changes before accepting edits, even when the task is narrowly scoped + +**Application**: After any agent batch edit, run git diff --stat and scan for non-comment changes before staging + +--- + +## [2026-03-14-180902] Subagents rename packages and modify unrelated files without being asked + +**Context**: ET agent renamed internal/eventlog/ to internal/log/ and modified 5+ caller files outside the internal/err/ scope + +**Lesson**: Always diff agent output against HEAD to catch scope creep before building; revert unrelated changes immediately + +**Application**: After any agent-driven refactor, run git diff --name-only HEAD and revert anything outside the intended scope before testing + +--- + +## [2026-03-14-110750] Subagents reorganize file structure without being asked + +**Context**: Asked subagent to replace os.ReadFile callsites with validation wrappers. It also moved functions to new files, renamed them (ReadUserFile to SafeReadUserFile), and created a new internal/io package. + +**Lesson**: Subagents optimize for clean results and will restructure beyond stated scope — moved, renamed, and split files without being asked. + +**Application**: After subagent-driven refactors, always verify file organization matches intent. Audit for moved, renamed, and split files, not just the requested callsite changes. + +--- + +## [2026-03-06-200319] Spawned agents reliably create new files but consistently fail to delete old ones — always audit for stale files, duplicate function definitions, and orphaned imports after agent-driven refactoring + +**Context**: Multiple agent batches across cmd/ restructuring, color removal, and flag externalization left stale files, duplicate run.go, and unupdated parent imports + +**Lesson**: Agent cleanup is a known gap — budget 5-10 minutes for post-agent audit per batch + +**Application**: After every agent batch: grep for stale package declarations, check parent imports point to cmd/root not cmd/, verify old files are deleted + +--- + + +## Group: Bulk rename and replace_all hazards + +## [2026-03-20-232518] replace_all on short tokens like function names will also replace their definitions + +**Context**: Using replace_all to rename HumanAgo to format.DurationAgo also changed the func declaration line to func format.DurationAgo which is invalid Go + +**Lesson**: replace_all matches everywhere in the file including function definitions, not just call sites + +**Application**: When renaming functions, delete the old definition separately rather than using replace_all. Or use a more specific match pattern that excludes func declarations. + +--- + +## [2026-03-15-230643] replace_all on short tokens like core. mangles aliased imports + +**Context**: Used Edit tool replace_all to change core. to tidy. across handle_tool.go + +**Lesson**: Short replacement tokens match inside longer identifiers — remindcore. became remindtidy. silently + +**Application**: When doing bulk renames, prefer targeted replacements or grep for collateral damage immediately after. Avoid replace_all on tokens shorter than the full identifier + +--- + +## [2026-03-18-193602] Bulk sed on imports displaces aliased imports + +**Context**: Used sed to add desc import to 278 files. Files with aliased imports like ctxCfg config/ctx got the alias stolen by the new import line inserted before it. + +**Lesson**: sed insert-before-first-match does not understand Go import aliases. The alias attaches to whatever import line sed inserts, not the original target. + +**Application**: When bulk-adding imports, check for aliased imports first and handle them separately. Or use goimports if available. + +--- + + +## Group: Import cycles and package splits + +## [2026-03-22-220846] Types in god-object files create circular dependencies + +**Context**: hook/types.go had 15+ types from 8 domains; session importing hook for SessionTokenInfo created a cycle + +**Lesson**: Moving types to their owning domain package breaks import cycles + +**Application**: When a type is only used by one domain package, move it there. Check with grep before assuming a type is cross-cutting. + +--- + +## [2026-03-18-193616] Tests in package X cannot import X/sub packages that import X back + +**Context**: embed_test.go in package assets kept importing read/ sub-packages that import assets.FS, creating cycles. Recurred 4 times this session. + +**Lesson**: Tests that exercise domain accessor packages must live in those packages, not in the parent. The parent test file can only use the shared resource (FS) directly. + +**Application**: When splitting functions from a parent package into sub-packages, move the corresponding tests too. Do not leave them in the parent. + +--- + +## [2026-03-13-223108] Variable shadowing causes cascading test failures after package splits + +**Context**: Large refactoring moved constants from monolithic config package to sub-packages (dir, entry, file). Test files had local variables named dir, entry, file that shadowed the new package imports. + +**Lesson**: When splitting a package, audit test files for local variable names that collide with new package names. dir, file, entry are especially common Go variable names. + +**Application**: Before committing a package split, run go test ./... and check for undefined errors caused by variable shadowing. + +--- + +## [2026-03-13-151951] Linter reverts import-only edits when references still use old package + +**Context**: Moving tpl_entry.go from config/entry to assets — linter kept reverting the import change + +**Lesson**: When moving constants between packages, change imports and all references in a single atomic write (use Write not incremental Edit), so the linter never sees an inconsistent state + +**Application**: For future package migrations, use full file rewrites when a linter is active + +--- + +## [2026-03-06-200237] Import cycle avoidance: when package A imports package B for logic, B must own shared types — A aliases them. entry imports add/core for insert logic, so add/core owns EntryParams and entry aliases it as entry.Params + +**Context**: Extracting entry.Params as a standalone struct in internal/entry created a cycle because entry/write.go imports add/core for AppendEntry + +**Lesson**: The package that provides implementation logic must own the types; the facade package aliases them + +**Application**: When extracting shared types from implementation packages, check the import direction first — the type lives where the logic lives + +--- + + +## Group: Lint suppression and gosec patterns + +## [2026-03-19-194942] Rename constants to avoid gosec G101 false positives + +**Context**: Constants named ColTokens, DriftPassed, StatusTokensFormat triggered gosec G101 credential detection. Suppressing with nolint or broadening .golangci.yml exclusion paths is fragile — paths change when files split. + +**Lesson**: Rename the constant to avoid the trigger word instead of suppressing the lint. Tokens→Usage, Passed→OK convey the same semantics without false positives. This is cleaner than nolint annotations or path-based exclusions that break on file reorganization. + +**Application**: When gosec flags a constant name, ask what the value semantically represents and rename to that. Do not add nolint, nosec, or path exclusions. + +--- + +## [2026-03-06-050126] nolint:goconst for trivial values normalizes magic strings + +**Context**: Found 5 callsites suppressing goconst with nolint:goconst trivial user input check for y/yes comparisons + +**Lesson**: Suppressing a linter with a trivial justification sets precedent for other agents to do the same. The fix (two constants) costs less than the accumulated tech debt. + +**Application**: Use config.ConfirmShort/config.ConfirmLong instead of suppressing goconst. Prefer constants over nolint directives. + +--- + +## [2026-03-04-040211] nolint:errcheck in tests normalizes unchecked errors for agents + +**Context**: User flagged that suppressing errcheck in tests teaches the agent to spread the pattern to production code + +**Lesson**: Broken-window theory applies to lint suppressions. Agents learn from test code patterns. Use _ = f.Close() in a closure or check errors with t.Fatal — never suppress with nolint. + +**Application**: Handle all errors in test code the same as production: t.Fatal(err) for setup, defer func() { _ = f.Close() }() for best-effort cleanup. + +--- + +## [2026-03-04-040209] golangci-lint v2 ignores inline nolint directives for some linters + +**Context**: nolint:errcheck and nolint:gosec comments were present but golangci-lint v2 still reported violations + +**Lesson**: In golangci-lint v2, use config-level exclusions.rules for gosec patterns (G204, G301, G306) rather than relying on inline nolint directives. For errcheck, fix the code instead of suppressing. + +**Application**: When adding new lint suppressions, prefer config-level rules for gosec false positives on safe paths/args; never suppress errcheck — handle the error. + +--- + + +## Group: Skill lifecycle and promotion + +## [2026-03-14-093757] Internal skill rename requires updates across 6+ layers + +**Context**: Renamed ctx-alignment-audit to _ctx-alignment-audit. The allow list test in embed_test.go failed because it iterates all bundled skills and expects each in the allow list. + +**Lesson**: The allow list test needed a strings.HasPrefix(_) skip for internal skills. This was not obvious until tests ran. + +**Application**: When converting public to internal skills, audit: allow.txt, embed_test.go allow list test, reference/skills.md, all recipe docs referencing the skill, contributing.md dev-only skills table, and permissions docs. + +--- + +## [2026-03-13-223110] Skills without a trigger mechanism are dead code + +**Context**: ctx-context-monitor was a skill documenting how to respond to hook output, but no hook or agent ever loaded it. The hook output already contained sufficient instructions. + +**Lesson**: A skill only enters the agent context when explicitly invoked via /skill-name. If the description says not user-invocable and no mechanism loads it automatically, it is unreachable. + +**Application**: Audit skills for reachability. If nothing triggers the skill, either add a trigger or delete it. + +--- + +## [2026-03-01-125807] Elevating private skills requires synchronized updates across 6 layers + +**Context**: Promoted 6 _ctx-* skills to bundled ctx-* plugin skills + +**Lesson**: Moving a skill from .claude/skills/ to internal/assets/claude/skills/ touches: (1) SKILL.md frontmatter name field, (2) internal cross-references between skills (slash command paths), (3) external cross-references in other skills and docs, (4) embed_test.go expected skill list, (5) recipe and reference docs that mention the old name, (6) plugin cache rebuild (`hack/plugin-reload.sh`) + session restart — Claude Code snapshots skills from `~/.claude/plugins/cache/` at startup, so new skills are invisible until the cache is refreshed. Also clean stale underscore-prefixed `Skill(_ctx-*)` entries from `.claude/settings.local.json`. + +**Application**: When promoting future skills, use grep -r /_ctx-{name} across the whole tree before declaring done. After code changes, run plugin-reload.sh and restart the session to verify the skill appears in autocomplete. + +--- + +## [2026-03-01-144544] Skill enhancement is a documentation-heavy operation across 10+ files + +**Context**: Enhancing /ctx-journal-enrich-all to handle export-if-needed touched the skill, hook messages, fallback strings, 5 doc files, 2 Makefiles, and TASKS.md + +**Lesson**: Skill behavior changes ripple through hook messages, fallback strings in Go code, doc descriptions, and Makefile hints — all must stay synchronized + +**Application**: When modifying a skill's scope, grep for its name across the entire repo and update every description, not just the skill file itself + +--- + + +## Group: Cross-cutting change ripple + +## [2026-03-01-194147] Key path changes ripple across 15+ doc files and 2 skills + +**Context**: Updating docs for the .context/.ctx.key → ~/.local/ctx/keys/ → ~/.ctx/.ctx.key migrations + +**Lesson**: Key path changes have a long documentation tail — recipes, references, getting-started, operations, CLI docs, and skills all carry path references. The worktree behavior flip (limitation to works automatically) was the highest-value change per line edited. Simplifying from per-project slugs to a single global key eliminated more code and docs than the original migration added. + +**Application**: When moving a file path that appears in user-facing docs, grep broadly (not just code) and budget for 15+ file touches + +--- + +## [2026-03-01-112538] Removing embedded asset directories requires synchronized cleanup across 5+ layers + +**Context**: Deleting .context/tools/ deployment touched embed directive, asset functions, init logic, tests, config constants, Makefile targets, and docs — missing any one layer leaves dead code or build failures. + +**Lesson**: Embedded asset removal is a cross-cutting concern: embed directive → accessor functions → callers → tests → config constants → build targets → documentation. Work outward from the embed. + +**Application**: When removing an embedded asset category, use the grep-first approach (search for all references to the accessor functions and constants) before deleting anything. + +--- + +## [2026-03-01-102232] Absorbing shell scripts into Go commands creates a discoverability gap + +**Context**: Deleted make backup/backup-global/backup-all and make rc-dev/rc-base/rc-status targets when absorbing into ctx system backup and ctx config switch. The Makefile served as self-documenting discovery (make help). + +**Lesson**: When eliminating Makefile targets, the CLI reference page alone is not sufficient — contributor-facing docs (contributing.md) and command catalogs (common-workflows.md) must gain explicit entries to compensate. + +**Application**: For future hack/ absorptions (e.g. pad-import-ideas.sh, context-watch.sh), audit contributing.md, common-workflows.md CLI-Only table, and the CLI index page as part of the absorption checklist. + +--- + +## [2026-02-19-215200] Feature can be code-complete but invisible to users + +**Context**: ctx pad merge was fully implemented with 19 passing tests and binary support, but had zero coverage in user-facing docs (scratchpad.md, cli-reference.md, scratchpad-sync recipe). Only discoverable via --help. + +**Lesson**: Implementation completeness \!= user-facing completeness. A feature without docs is invisible to users who don't explore CLI help. + +**Application**: After implementing a new CLI subcommand, always check: feature page, cli-reference.md, relevant recipes, and zensical.toml nav (if new page). + +--- + + +## Group: Dead code detection + +## [2026-03-30-003720] internal/cli/recall/ was dead code — never registered in bootstrap + +**Context**: The entire recall CLI package existed with tests but was never wired into the command tree. Journal consumed it but nobody deleted the ghost + +**Lesson**: Dead package detection requires checking bootstrap registration, not just build success. A package can build and test green while being completely unreachable + +**Application**: Add a compliance test that verifies all cli/ packages are registered in bootstrap + +--- + +## [2026-03-25-173339] Dead files accumulate when nothing consumes them + +**Context**: IMPLEMENTATION_PLAN.md and PROMPT.md were created by ctx init but no agent, hook, or skill ever read them + +**Lesson**: Before adding a file to init scaffolding, verify there is at least one consumer. Periodically audit what init creates vs what the system reads. + +**Application**: The prompt deprecation spec documents the reasoning as a papertrail for future removals. + +--- + +## [2026-03-15-101346] Delete legacy code instead of maintaining it — MigrateKeyFile had 5 callers and test coverage but zero users + +**Context**: Started by adding constants for legacy key names, then realized nobody uses legacy keys + +**Lesson**: When touching legacy compat code, first ask whether the legacy path has real users. If not, delete it entirely rather than improving it + +**Application**: Apply to any backward-compat shim: check actual usage before investing in maintenance + +--- + + diff --git a/.context/archive/tasks-2026-04-03.md b/.context/archive/tasks-2026-04-03.md new file mode 100644 index 000000000..5210b4d81 --- /dev/null +++ b/.context/archive/tasks-2026-04-03.md @@ -0,0 +1,64 @@ +# Archived Tasks - 2026-04-03 + +- [x] Add TestTypeFileConvention audit check: type definitions must live in types.go, not mixed into function files. Scan all non-test .go files for ast.TypeSpec declarations; flag any that appear in files not named types.go. Migrate violations. #priority:medium #added:2026-04-03-030033 #done:2026-04-03 +- [x] Collect all exec.Commands under internal/exec. See + Phase EXEC below for breakdown. — done, exec/{git,dep,gio,zensical} + exist, no exec.Command calls remain outside internal/exec + #done:2026-03-31 +- [x] PD.5.2: Run `ctx init` on a clean directory — verify no + `.context/prompts/` created. loop.md and skills checks are stale: + loop.md was never a ctx init artifact (ctx loop generates on demand), + skills deploy via plugin install, not ctx init. + #priority:high #added:2026-03-25-203340 #done:2026-03-31 +- [x] Bug: check-version hook missing throttle touch on plugin + version read error (run.go:70). When claude.PluginVersion() + fails, the hook returns without touching the daily throttle + marker, causing repeated checks on days when plugin.json is + missing or corrupted. Fix: add + internalIo.TouchFile(markerFile) before the early return. + See docs/recipes/hook-sequence-diagrams.md check-version + diagram which documents the expected behavior. + #added:2026-03-23-162802 #done:2026-03-31 +- [x] Add Use* constants for all system subcommands — all 30 system + subcommands already use cmd.UseSystem* constants + #added:2026-03-21-092550 #done:2026-03-31 +- [x] EH.0: Create central warning sink — `internal/log/warn/warn.go` with + `var sink io.Writer = os.Stderr` and `func Warn(format string, + args ...any)`. + All stderr warnings (`fmt.Fprintf(os.Stderr, ...)`) route through this + function. The `fmt.Fprintf` return error is handled once, centrally. + The sink is swappable (tests use `io.Discard`, future: syslog, file). + EH.2–EH.4 should use `log.Warn()` instead of raw `fmt.Fprintf`. + DoD: `grep -rn 'fmt.Fprintf(os.Stderr' internal/` returns zero hits + #priority:high #added:2026-03-15 +- [x] Cleanup internal/cli/system/core/wrapup.go: line 18 constant should go to + config; make WrappedUpExpiry configurable via ctxrc — already done, + wrap.ExpiryHours and wrap.Marker exist in config/wrap + #priority:low #added:2026-03-07-220825 #done:2026-03-31 +- [x] Cleanup internal/cli/system/core/version.go: line 81 newline should come + from config — already done, uses token.NewlineLF + #priority:low #added:2026-03-07-220819 #done:2026-03-31 +- [x] Add taxonomy to internal/cli/system/core/ — currently an unstructured bag + of files; group by domain (backup, hooks, session, knowledge, etc.) + — already done, 20 domain subdirectories exist + #priority:medium #added:2026-03-07-220819 #done:2026-03-31 +- [x] Cleanup internal/cli/system/core/version_drift.go: line 53 string + formatting should use assets — file moved to core/drift/, now uses + desc.Text and assets throughout + #priority:medium #added:2026-03-07-220819 #done:2026-03-31 +- [x] Cleanup internal/cli/system/core/state.go: magic permissions (0o750), + magic strings ('Context: ' prefix, etc.) — file moved to core/state/, + magic values extracted to config + #priority:medium #added:2026-03-07-220819 #done:2026-03-31 +- [x] Cleanup internal/cli/system/core/smb.go: errors should come from + internal/err; lines 101, 116, 111 need assets text — file moved to + core/archive/, errors routed through err package + #priority:medium #added:2026-03-07-220819 #done:2026-03-31 +- [x] Remove FlagNoColor and fatih/color dependency — replaced with plain + output, dependency removed from go.mod + #added:2026-03-06-182831 #done:2026-03-31 +- [x] Fix 3 CI compliance issues from PR #27 after merge: missing copyright + header on internal/mcp/server_test.go, missing doc.go for internal/cli/mcp/, + literal newlines in internal/mcp/resources.go and + tools.go — all fixed, files moved to mcp/server/ with copyright + #added:2026-03-06-141508 #done:2026-03-31 diff --git a/docs/reference/audit-conventions.md b/docs/reference/audit-conventions.md new file mode 100644 index 000000000..a591a6b91 --- /dev/null +++ b/docs/reference/audit-conventions.md @@ -0,0 +1,874 @@ +# Audit Conventions: Common Patterns and Fixes + +This guide documents the code conventions enforced by `internal/audit/` +AST tests. Each section shows the violation pattern, the fix, and the +rationale. When a test fails, find the matching section below. + +All tests skip `_test.go` files. The patterns apply only to production +code under `internal/`. + +--- + +## Variable Shadowing (bare `err :=` reuse) + +**Test:** `TestNoVariableShadowing` + +When a function has multiple `:=` assignments to `err`, each shadows +the previous one. This makes it impossible to tell which error a later +`if err != nil` is checking. + +**Before:** + +```go +func Run(cmd *cobra.Command) error { + data, err := os.ReadFile(path) + if err != nil { + return err + } + + result, err := json.Unmarshal(data) // shadows first err + if err != nil { + return err + } + + err = validate(result) // shadows again + return err +} +``` + +**After:** + +```go +func Run(cmd *cobra.Command) error { + data, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + + result, parseErr := json.Unmarshal(data) + if parseErr != nil { + return parseErr + } + + validateErr := validate(result) + return validateErr +} +``` + +**Rule:** Use descriptive error names (`readErr`, `writeErr`, `parseErr`, +`walkErr`, `absErr`, `relErr`) so each error site is independently +identifiable. + +--- + +## Import Name Shadowing + +**Test:** `TestNoImportNameShadowing` + +When a local variable has the same name as an imported package, the +import becomes inaccessible in that scope. + +**Before:** + +```go +import "github.com/ActiveMemory/ctx/internal/session" + +func process(session *entity.Session) { // param shadows import + // session package is now unreachable here +} +``` + +**After:** + +```go +import "github.com/ActiveMemory/ctx/internal/session" + +func process(sess *entity.Session) { + // session package still accessible +} +``` + +**Rule:** Parameters, variables, and return values must not reuse +imported package names. Common renames: `session` -> `sess`, +`token` -> `tok`, `config` -> `cfg`, `entry` -> `ent`. + +--- + +## Magic Strings + +**Test:** `TestNoMagicStrings` + +String literals in function bodies are invisible to refactoring tools +and cause silent breakage when the value changes in one place but not +another. + +**Before (string literals):** + +```go +func loadContext() { + data := filepath.Join(dir, "TASKS.md") + if strings.HasSuffix(name, ".yaml") { + // ... + } +} +``` + +**After:** + +```go +func loadContext() { + data := filepath.Join(dir, config.FilenameTask) + if strings.HasSuffix(name, config.ExtYAML) { + // ... + } +} +``` + +**Before (format verbs — also caught):** + +```go +func EntryHash(text string) string { + h := sha256.Sum256([]byte(text)) + return fmt.Sprintf("%x", h[:8]) +} +``` + +**After:** + +```go +func EntryHash(text string) string { + h := sha256.Sum256([]byte(text)) + return hex.EncodeToString(h[:cfgFmt.HashPrefixLen]) +} +``` + +**Before (URL schemes — also caught):** + +```go +if strings.HasPrefix(target, "https://") || + strings.HasPrefix(target, "http://") { + return target +} +``` + +**After:** + +```go +if strings.HasPrefix(target, cfgHTTP.PrefixHTTPS) || + strings.HasPrefix(target, cfgHTTP.PrefixHTTP) { + return target +} +``` + +**Exempt from this check:** + +- Empty string `""`, single space `" "`, indentation strings +- Regex capture references (`$1`, `${name}`) +- `const` and `var` definition sites (that's where constants live) +- Struct tags +- Import paths +- Packages under `internal/config/`, `internal/assets/tpl/` + +**Rule:** If a string is used for comparison, path construction, or +appears in 3+ files, it belongs in `internal/config/` as a constant. +Format strings belong in `internal/config/` as named constants +(e.g., `cfgGit.FlagLastN`, `cfgTrace.RefFormat`). User-facing prose +belongs in `internal/assets/` YAML files accessed via `desc.Text()`. + +**Common fix for `fmt.Sprintf` with format verbs:** + +| Pattern | Fix | +|---------|-----| +| `fmt.Sprintf("%d", n)` | `strconv.Itoa(n)` | +| `fmt.Sprintf("%d", int64Val)` | `strconv.FormatInt(int64Val, 10)` | +| `fmt.Sprintf("%x", bytes)` | `hex.EncodeToString(bytes)` | +| `fmt.Sprintf("%q", s)` | `strconv.Quote(s)` | +| `fmt.Sscanf(s, "%d", &n)` | `strconv.Atoi(s)` | +| `fmt.Sprintf("-%d", n)` | `fmt.Sprintf(cfgGit.FlagLastN, n)` | +| `"https://"` | `cfgHTTP.PrefixHTTPS` | +| `"<"` | config constant in `config/html/` | + +--- + +## Direct Printf Calls + +**Test:** `TestNoPrintfCalls` + +`cmd.Printf` and `cmd.PrintErrf` bypass the write-package formatting +pipeline and scatter user-facing text across the codebase. + +**Before:** + +```go +func Run(cmd *cobra.Command, args []string) { + cmd.Printf("Found %d tasks\n", count) +} +``` + +**After:** + +```go +func Run(cmd *cobra.Command, args []string) { + write.TaskCount(cmd, count) +} +``` + +**Rule:** All formatted output goes through `internal/write/` which +uses `cmd.Print`/`cmd.Println` with pre-formatted strings from +`desc.Text()`. + +--- + +## Raw Time Format Strings + +**Test:** `TestNoRawTimeFormats` + +Inline time format strings (`"2006-01-02"`, `"15:04:05"`) drift when +one call site is updated but others are missed. + +**Before:** + +```go +func formatDate(t time.Time) string { + return t.Format("2006-01-02") +} +``` + +**After:** + +```go +func formatDate(t time.Time) string { + return t.Format(cfgTime.DateFormat) +} +``` + +**Rule:** All time format strings must use constants from +`internal/config/time/`. + +--- + +## Direct Flag Registration + +**Test:** `TestNoFlagBindOutsideFlagbind` + +Direct cobra flag calls (`.Flags().StringVar()`, etc.) scatter flag +wiring across dozens of `cmd.go` files. Centralizing through +`internal/flagbind/` gives one place to audit flag names, defaults, +and description key lookups. + +**Before:** + +```go +func Cmd() *cobra.Command { + var output string + c := &cobra.Command{Use: cmd.UseStatus} + c.Flags().StringVarP(&output, "output", "o", "", + "output format") + return c +} +``` + +**After:** + +```go +func Cmd() *cobra.Command { + var output string + c := &cobra.Command{Use: cmd.UseStatus} + flagbind.StringFlagShort(c, &output, flag.Output, + flag.OutputShort, cmd.DescKeyOutput) + return c +} +``` + +**Rule:** All flag registration goes through `internal/flagbind/`. +If the helper you need doesn't exist, add it to `flagbind/flag.go` +before using it. + +--- + +## TODO Comments + +**Test:** `TestNoTODOComments` + +TODO, FIXME, HACK, and XXX comments in production code are invisible +to project tracking. They accumulate silently and never get addressed. + +**Before:** + +```go +// TODO: handle pagination +func listEntries() []Entry { +``` + +**After:** + +Remove the comment and add a task to `.context/TASKS.md`: + +``` +- [ ] Handle pagination in listEntries (internal/task/task.go) +``` + +**Rule:** Deferred work lives in TASKS.md, not in source comments. + +--- + +## Dead Exports + +**Test:** `TestNoDeadExports` + +Exported symbols with zero references outside their definition file +are dead weight. They increase API surface, confuse contributors, and +cost maintenance. + +**Fix:** Either delete the export (preferred) or demote it to +unexported if it's still used within the file. + +If the symbol existed for historical reasons and might be needed again, +move it to `quarantine/deadcode/` with a `.dead` extension. This +preserves the code in git without polluting the live codebase: + +``` +quarantine/deadcode/internal/config/flag/flag.go.dead +``` + +Each `.dead` file includes a header: + +```go +// Dead exports quarantined from internal/config/flag/flag.go +// Quarantined: 2026-04-02 +// Restore from git history if needed. +``` + +**Rule:** If a test-only allowlist entry is needed (the export exists +only for test use), add the fully qualified symbol to `testOnlyExports` +in `dead_exports_test.go`. Keep this list small — prefer eliminating +the export. + +--- + +## Core Package Structure + +**Test:** `TestCoreStructure` + +`core/` directories under `internal/cli/` must contain only `doc.go` +and test files at the top level. All domain logic lives in subpackages. +This prevents `core/` from becoming a god package. + +**Before:** + +``` +internal/cli/dep/core/ + go.go # violation — logic at core/ level + python.go # violation + node.go # violation + types.go # violation +``` + +**After:** + +``` +internal/cli/dep/core/ + doc.go # package doc only + golang/ + golang.go + golang_test.go + doc.go + python/ + python.go + python_test.go + doc.go + node/ + node.go + node_test.go + doc.go +``` + +**Rule:** Extract each logical unit into its own subpackage under +`core/`. Each subpackage gets a `doc.go`. The subpackage name should +match the domain concept (`golang`, `check`, `fix`, `store`), not a +generic label (`util`, `helper`). + +--- + +## Cross-Package Types + +**Test:** `TestCrossPackageTypes` + +When a type defined in one package is used from a different module +(e.g., `cli/doctor` importing a type from `cli/notify`), the type +has crossed its module boundary. Cross-cutting types belong in +`internal/entity/` for discoverability. + +**Before:** + +```go +// internal/cli/notify/core/types.go +type NotifyPayload struct { ... } + +// internal/cli/doctor/core/check/check.go +import "github.com/ActiveMemory/ctx/internal/cli/notify/core" +func check(p core.NotifyPayload) { ... } +``` + +**After:** + +```go +// internal/entity/notify.go +type NotifyPayload struct { ... } + +// internal/cli/doctor/core/check/check.go +import "github.com/ActiveMemory/ctx/internal/entity" +func check(p entity.NotifyPayload) { ... } +``` + +**Exempt:** Types inside `entity/`, `proto/`, `core/` subpackages, +and `config/` packages. Same-module usage (e.g., `cli/doctor/cmd/` +using `cli/doctor/core/`) is not flagged. + +--- + +## Type File Convention + +**Test:** `TestTypeFileConvention`, `TestTypeFileConventionReport` + +Exported types in `core/` subpackages should live in `types.go` (the +convention from CONVENTIONS.md), not scattered across implementation +files. This makes type definitions discoverable. +`TestTypeFileConventionReport` generates a diagnostic summary of all +type placements for triage. + +**Exception:** `entity/` organizes by domain (`task.go`, `session.go`), +`proto/` uses `schema.go`, and `err/` packages colocate error types +with their domain context. + +--- + +## DescKey / YAML Linkage + +**Test:** `TestDescKeyYAMLLinkage` + +Every DescKey constant must have a corresponding key in the YAML asset +files, and every YAML key must have a corresponding DescKey constant. +Orphans in either direction mean dead text or runtime panics. + +**Fix for orphan YAML key:** Delete the YAML entry, or add the +corresponding `DescKey` constant in `config/embed/{text,cmd,flag}/`. + +**Fix for orphan DescKey:** Delete the constant, or add the +corresponding entry in the YAML file under +`internal/assets/commands/text/`, `cmd/`, or `flag/`. + +If the orphan YAML entry was once valid but the feature was removed, +move the YAML entry to a `.dead` file in `quarantine/deadcode/`. + +--- + +## Package Doc Quality + +**Test:** `TestPackageDocQuality` + +Every package under `internal/` must have a `doc.go` with a meaningful +package doc comment (at least 8 lines of real content). One-liners and +file-list patterns (`// - foo.go`, `// Source files:`) are flagged +because they drift as files change. + +**Template:** + +```go +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Package mypackage does X. +// +// It handles Y by doing Z. The main entry point is [FunctionName] +// which accepts A and returns B. +// +// Configuration is read from [config.SomeConstant]. Output is +// written through [write.SomeHelper]. +// +// This package is used by [parentpackage] during the W lifecycle +// phase. +package mypackage +``` + +--- + +## Inline Regex Compilation + +**Test:** `TestNoInlineRegexpCompile` + +`regexp.MustCompile` and `regexp.Compile` inside function bodies +recompile the pattern on every call. Compiled patterns belong at +package level. + +**Before:** + +```go +func parse(s string) bool { + re := regexp.MustCompile(`\d{4}-\d{2}-\d{2}`) + return re.MatchString(s) +} +``` + +**After:** + +```go +// In internal/config/regex/regex.go: +// DatePattern matches ISO date format (YYYY-MM-DD). +var DatePattern = regexp.MustCompile(`\d{4}-\d{2}-\d{2}`) + +// In calling package: +func parse(s string) bool { + return regex.DatePattern.MatchString(s) +} +``` + +**Rule:** All compiled regexes live in `internal/config/regex/` as +package-level `var` declarations. Two tests enforce this: +`TestNoInlineRegexpCompile` catches function-body compilation, and +`TestNoRegexpOutsideRegexPkg` catches package-level compilation +outside `config/regex/`. + +--- + +## Doc Comments + +**Test:** `TestDocComments` + +All functions (exported and unexported), structs, and package-level +variables must have a doc comment. Config packages allow group doc +comments for `const` blocks. + +**Before:** + +```go +func buildIndex(entries []Entry) map[string]int { +``` + +**After:** + +```go +// buildIndex maps entry names to their position in the +// ordered slice for O(1) lookup during reconciliation. +// +// Parameters: +// - entries: ordered slice of entries to index +// +// Returns: +// - map[string]int: name-to-position mapping +func buildIndex(entries []Entry) map[string]int { +``` + +**Rule:** Every function, struct, and package-level `var` gets a doc +comment in godoc format. Functions include `Parameters:` and +`Returns:` sections. Structs with 2+ fields document every field. +See CONVENTIONS.md for the full template. + +--- + +## Line Length + +**Test:** `TestLineLength` + +Lines in non-test Go files must not exceed 80 characters. This is a +hard check, not a suggestion. + +**Before:** + +```go +_ = trace.Record(fmt.Sprintf(cfgTrace.RefFormat, cfgTrace.RefTypeTask, matchedNum), state.Dir()) +``` + +**After:** + +```go +ref := fmt.Sprintf( + cfgTrace.RefFormat, cfgTrace.RefTypeTask, matchedNum, +) +_ = trace.Record(ref, state.Dir()) +``` + +**Rule:** Break at natural points: function arguments, struct fields, +chained calls. Long strings (URLs, struct tags) are the rare +acceptable exception. + +--- + +## Literal Whitespace + +**Test:** `TestNoLiteralWhitespace` + +Bare whitespace string and byte literals (`"\n"`, `"\r\n"`, `"\t"`) +must not appear outside `internal/config/token/`. All other packages +use the token constants. + +**Before:** + +```go +output := strings.Join(lines, "\n") +``` + +**After:** + +```go +output := strings.Join(lines, token.Newline) +``` + +**Rule:** Whitespace literals are defined once in +`internal/config/token/`. Use `token.Newline`, `token.Tab`, +`token.CRLF`, etc. + +--- + +## Magic Numeric Values + +**Test:** `TestNoMagicValues` + +Numeric literals in function bodies need constants, with narrow +exceptions. + +**Before:** + +```go +if len(entries) > 100 { + entries = entries[:100] +} +``` + +**After:** + +```go +if len(entries) > config.MaxEntries { + entries = entries[:config.MaxEntries] +} +``` + +**Exempt:** `0`, `1`, `-1`, `2`–`10`, strconv radix/bitsize args +(`10`, `32`, `64` in `strconv.Parse*`/`Format*`), octal permissions +(caught separately by `TestNoRawPermissions`), and `const`/`var` +definition sites. + +--- + +## Inline Separators + +**Test:** `TestNoInlineSeparators` + +`strings.Join` calls must use token constants for their separator +argument, not string literals. + +**Before:** + +```go +result := strings.Join(parts, ", ") +``` + +**After:** + +```go +result := strings.Join(parts, token.CommaSep) +``` + +**Rule:** Separator strings live in `internal/config/token/`. + +--- + +## Stuttery Function Names + +**Test:** `TestNoStutteryFunctions` + +Function names must not redundantly include their package name as a +PascalCase word boundary. Go callers already write `pkg.Function`, +so `pkg.PkgFunction` stutters. + +**Before:** + +```go +// In package write +func WriteJournal(cmd *cobra.Command, ...) { +``` + +**After:** + +```go +// In package write +func Journal(cmd *cobra.Command, ...) { +``` + +**Exempt:** Identity functions like `write.Write` / `write.write`. + +--- + +## Predicate Naming (no `Is`/`Has`/`Can` prefix) + +**Test:** None (manual review convention) + +Exported methods that return `bool` must not use `Is`, `Has`, or +`Can` prefixes. The predicate reads more naturally without them, +especially at call sites where the package name provides context. + +**Before:** + +```go +func IsCompleted(t *Task) bool { ... } +func HasChildren(n *Node) bool { ... } +func IsExemptPackage(path string) bool { ... } +``` + +**After:** + +```go +func Completed(t *Task) bool { ... } +func Children(n *Node) bool { ... } // or: ChildCount > 0 +func ExemptPackage(path string) bool { ... } +``` + +**Rule:** Drop the prefix. Private helpers may use prefixes when it +reads more naturally (`isValid` in a local context is fine). This +convention applies to exported methods and package-level functions. +See CONVENTIONS.md "Predicates" section. + +This is not yet enforced by an AST test — it requires semantic +understanding of return types and naming intent that makes automated +detection fragile. Apply during code review. + +--- + +## Mixed Visibility + +**Test:** `TestNoMixedVisibility` + +Files with exported functions must not also contain unexported +functions. Public API and private helpers live in separate files. + +**Before:** + +``` +load.go + func Load() { ... } // exported + func parseHeader() { ... } // unexported — violation +``` + +**After:** + +``` +load.go + func Load() { ... } // exported only +parse.go + func parseHeader() { ... } // private helper +``` + +**Exempt:** Files with exactly one function, `doc.go`, test files. + +--- + +## Stray err.go Files + +**Test:** `TestNoStrayErrFiles` + +`err.go` files must only exist under `internal/err/`. Error +constructors anywhere else create a broken-window pattern where +contributors add local error definitions when they see a local +`err.go`. + +**Fix:** Move the error constructor to `internal/err//`. + +--- + +## CLI Cmd Structure + +**Test:** `TestCLICmdStructure` + +Each `cmd/$sub/` directory under `internal/cli/` may contain only +`cmd.go`, `run.go`, `doc.go`, and test files. Extra `.go` files +(helpers, output formatters, types) belong in the corresponding +`core/` subpackage. + +**Before:** + +``` +internal/cli/doctor/cmd/root/ + cmd.go + run.go + format.go # violation — helper in cmd dir +``` + +**After:** + +``` +internal/cli/doctor/cmd/root/ + cmd.go + run.go +internal/cli/doctor/core/format/ + format.go + doc.go +``` + +--- + +## DescKey Namespace + +**Test:** `TestUseConstantsOnlyInCobraUse`, +`TestDescKeyOnlyInLookupCalls`, +`TestNoWrongNamespaceLookup` + +Three tests enforce DescKey/Use constant discipline: + +1. `Use*` constants appear only in cobra `Use:` struct field + assignments — never as arguments to `desc.Text()` or elsewhere. +2. `DescKey*` constants are passed only to `assets.CommandDesc()`, + `assets.FlagDesc()`, or `desc.Text()` — never to cobra `Use:`. +3. No cross-namespace lookups — `TextDescKey` must not be passed to + `CommandDesc()`, `FlagDescKey` must not be passed to `Text()`, etc. + +--- + +## YAML Examples / Registry Linkage + +**Test:** `TestExamplesYAMLLinkage`, `TestRegistryYAMLLinkage` + +Every key in `examples.yaml` and `registry.yaml` must match a known +entry type constant. Prevents orphan entries that are never rendered. + +**Fix:** Delete the orphan YAML entry, or add the corresponding +constant in `config/entry/`. + +--- + +## Other Enforced Patterns + +These tests follow the same fix approach — extract the operation to +its designated package: + +| Test | Violation | Fix | +|------|-----------|-----| +| `TestNoNakedErrors` | `fmt.Errorf`/`errors.New` outside `internal/err/` | Add error constructor to `internal/err//` | +| `TestNoRawFileIO` | Direct `os.ReadFile`, `os.Create`, etc. | Use `io.SafeReadFile`, `io.SafeWriteFile`, etc. | +| `TestNoRawLogging` | Direct `fmt.Fprintf(os.Stderr, ...)` | Use `log/warn.Warn()` or `log/event.Append()` | +| `TestNoExecOutsideExecPkg` | `exec.Command` outside `internal/exec/` | Add command to `internal/exec//` | +| `TestNoCmdPrintOutsideWrite` | `cmd.Print*` outside `internal/write/` | Add output helper to `internal/write//` | +| `TestNoRawPermissions` | Octal literals (`0644`, `0755`) | Use `config/fs.PermFile`, `config/fs.PermExec`, etc. | +| `TestNoErrorsAs` | `errors.As()` | Use `errors.AsType()` (generic, Go 1.23+) | +| `TestNoStringConcatPaths` | `dir + "/" + file` | Use `filepath.Join(dir, file)` | + +--- + +## General Fix Workflow + +When an audit test fails: + +1. **Read the error message.** It includes `file:line` and a + description of the violation. +2. **Find the matching section above.** The test name maps directly + to a section. +3. **Apply the pattern.** Most fixes are mechanical: extract to the + right package, rename a variable, or replace a literal with a + constant. +4. **Run `make test` before committing.** Audit tests run as part of + `go test ./internal/audit/`. +5. **Don't add allowlist entries as a first resort.** Fix the code. + Allowlists exist only for genuinely unfixable cases (test-only + exports, config packages that are definitionally exempt). diff --git a/internal/assets/commands/text/errors.yaml b/internal/assets/commands/text/errors.yaml index 9d19b79ab..0b1941e06 100644 --- a/internal/assets/commands/text/errors.yaml +++ b/internal/assets/commands/text/errors.yaml @@ -446,12 +446,8 @@ err.skill.invalid-yaml: short: 'skill: %s: invalid YAML frontmatter: %w' err.skill.load: short: 'skill: %s: %w' -err.skill.missing-closing-delimiter: - short: missing closing frontmatter delimiter (---) err.skill.missing-name: short: 'skill: %s is missing required ''name'' field' -err.skill.missing-opening-delimiter: - short: missing opening frontmatter delimiter (---) err.skill.not-found: short: 'skill %q not found' err.skill.not-valid-dir: @@ -478,10 +474,6 @@ err.steering.file-exists: short: 'steering file already exists: %s' err.steering.invalid-yaml: short: 'steering: %s: invalid YAML frontmatter: %w' -err.steering.missing-closing-delimiter: - short: missing closing frontmatter delimiter (---) -err.steering.missing-opening-delimiter: - short: missing opening frontmatter delimiter (---) err.steering.no-tool: short: 'no tool specified: use --tool , --all, or set the tool field in .ctxrc' err.steering.output-escapes-root: @@ -572,3 +564,7 @@ err.validation.parse-file: short: 'failed to parse %s: %w' err.validation.working-directory: short: 'failed to get working directory: %w' +err.parser.missing-open-delim: + short: missing opening frontmatter delimiter (---) +err.parser.missing-close-delim: + short: missing closing frontmatter delimiter (---) diff --git a/internal/assets/commands/text/ui.yaml b/internal/assets/commands/text/ui.yaml index 8d411ffde..9bb61ec7f 100644 --- a/internal/assets/commands/text/ui.yaml +++ b/internal/assets/commands/text/ui.yaml @@ -526,3 +526,148 @@ write.why-label-invariants: short: "Design Invariants" help.community-footer: short: "\nHave a question, bug, or feature request? Join us on Discord:\n https://ctx.ist/discord\n" + +write.steering-created: + short: 'Created %s' +write.steering-skipped: + short: 'Skipped %s (already exists)' +write.steering-init-summary: + short: "\n%d created, %d skipped" +write.steering-file-entry: + short: '%-20s inclusion=%-7s priority=%-3d tools=%s' +write.steering-file-count: + short: "\n%d steering file(s)" +write.steering-preview-head: + short: 'Steering files matching prompt %q:' +write.steering-preview-entry: + short: ' %-20s inclusion=%-7s priority=%-3d tools=%s' +write.steering-preview-count: + short: "\n%d file(s) would be included" +write.steering-sync-written: + short: 'Written: %s' +write.steering-sync-skipped: + short: 'Skipped: %s' +write.steering-sync-error: + short: 'Error: %s' +write.steering-sync-summary: + short: "\n%d written, %d skipped, %d errors" + +write.skill-installed: + short: 'Installed %s → %s' +write.skill-entry-desc: + short: ' %-20s %s' +write.skill-entry: + short: ' %s' +write.skill-count: + short: "\n%d skill(s)" +write.skill-removed: + short: 'Removed %s' + +write.setup-deploy-complete: + short: '%s setup complete.' +write.setup-deploy-mcp: + short: ' MCP server: %s' +write.setup-deploy-steering: + short: ' Steering: %s' +write.setup-deploy-exists: + short: "\u2713 %s already exists (skipped)" +write.setup-deploy-created: + short: "\u2713 Created %s" +write.setup-deploy-synced: + short: "\u2713 Synced steering: %s" +write.setup-deploy-skip-steer: + short: ' Skipped steering: %s (unchanged)' + +mcp.steering-section: + short: "## %s\n\n%s\n\n" +mcp.search-hit-line: + short: "%s:%d: %s\n" +mcp.search-no-match: + short: 'No matches for %q in %s.' + +trigger.warn: + short: 'hook %s: %v' +trigger.error-item: + short: '%s: %s' +trigger.skip-warn: + short: 'hook skip %s: %v' + +drift.tool-suffix: + short: '%s (tool: %s)' + +write.trigger-created: + short: 'Created %s' +write.trigger-disabled: + short: 'Disabled %s (%s)' +write.trigger-enabled: + short: 'Enabled %s (%s)' +write.trigger-type-hdr: + short: '[%s]' +write.trigger-entry: + short: ' %-20s %-8s %s' +write.trigger-count: + short: '%d hook(s)' +write.trigger-test-hdr: + short: 'Testing %s hooks...' +write.trigger-test-input: + short: "Input:\n%s" +write.trigger-cancelled: + short: 'Cancelled: %s' +write.trigger-context: + short: "Context:\n%s" +write.trigger-err-line: + short: ' %s' + +write.setup-cursor-head: + short: 'Cursor integration:' +write.setup-cursor-run: + short: ' Run: ctx setup cursor --write' +write.setup-cursor-mcp: + short: ' Creates: .cursor/mcp.json (MCP server config)' +write.setup-cursor-sync: + short: ' Syncs: .cursor/rules/ (steering files)' +write.setup-kiro-head: + short: 'Kiro integration:' +write.setup-kiro-run: + short: ' Run: ctx setup kiro --write' +write.setup-kiro-mcp: + short: ' Creates: .kiro/settings/mcp.json (MCP server config)' +write.setup-kiro-sync: + short: ' Syncs: .kiro/steering/ (steering files)' +write.setup-cline-head: + short: 'Cline integration:' +write.setup-cline-run: + short: ' Run: ctx setup cline --write' +write.setup-cline-mcp: + short: ' Creates: .vscode/mcp.json (MCP server config)' +write.setup-cline-sync: + short: ' Syncs: .clinerules/ (steering files)' +write.setup-no-steering-to-sync: + short: ' No steering files to sync (run ctx steering init first)' + +write.skill-no-skills: + short: 'No skills installed.' + +write.steering-no-files: + short: 'No steering files found.' +write.steering-no-match: + short: 'No steering files match the given prompt.' + +write.trigger-no-hooks: + short: 'No hooks found.' +write.trigger-errors-hdr: + short: 'Errors:' +write.trigger-no-output: + short: 'No output from hooks.' + +mcp.hooks-disabled: + short: 'Hooks disabled.' +mcp.session-start-ok: + short: 'Session start hooks executed. No additional context.' +mcp.session-end-ok: + short: 'Session end hooks executed.' + +mcp.steering-no-files: + short: 'No steering files found.' +mcp.steering-no-match: + short: 'No matching steering files.' diff --git a/internal/assets/hooks/messages/doc.go b/internal/assets/hooks/messages/doc.go index 21d4e5ac5..4c681ad55 100644 --- a/internal/assets/hooks/messages/doc.go +++ b/internal/assets/hooks/messages/doc.go @@ -9,6 +9,6 @@ // The embedded registry.yaml maps each hook+variant pair to a // category and description. [Registry] returns all entries, // [Lookup] finds a specific one, and [Variants] enumerates -// the available names. [CategoryCtxSpecific] marks entries -// internal to ctx. +// the available names. Category constants live in +// config/hook (CategoryCustomizable, CategoryCtxSpecific). package messages diff --git a/internal/assets/hooks/messages/hooks.go b/internal/assets/hooks/messages/hooks.go new file mode 100644 index 000000000..26a857b3e --- /dev/null +++ b/internal/assets/hooks/messages/hooks.go @@ -0,0 +1,34 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package messages + +// registryError returns any error encountered while +// parsing the embedded registry.yaml. Nil on success. +// +// Returns: +// - error: Parse error, or nil on success +func registryError() error { + Registry() // ensure sync.Once has run + return registryErr +} + +// hooks returns a deduplicated list of hook names in +// the registry. +// +// Returns: +// - []string: Hook names in alphabetical order +func hooks() []string { + seen := make(map[string]bool) + var result []string + for _, info := range Registry() { + if !seen[info.Hook] { + seen[info.Hook] = true + result = append(result, info.Hook) + } + } + return result +} diff --git a/internal/assets/hooks/messages/registry.go b/internal/assets/hooks/messages/registry.go index 2d0ca0e71..aacef54b5 100644 --- a/internal/assets/hooks/messages/registry.go +++ b/internal/assets/hooks/messages/registry.go @@ -17,14 +17,6 @@ import ( errParser "github.com/ActiveMemory/ctx/internal/err/parser" ) -// CategoryCustomizable marks messages intended for -// project-specific customization. -const CategoryCustomizable = "customizable" - -// CategoryCtxSpecific marks messages specific to ctx's -// own development workflow. -const CategoryCtxSpecific = "ctx-specific" - // registryOnce, registryData, and registryErr cache the parsed hook // message registry loaded once via sync.Once. var ( @@ -61,16 +53,6 @@ func Registry() []HookMessageInfo { return registryData } -// RegistryError returns any error encountered while parsing the -// embedded registry.yaml. Nil on success. -// -// Returns: -// - error: Parse error from registry.yaml, or nil on success -func RegistryError() error { - Registry() // ensure sync.Once has run - return registryErr -} - // Lookup returns the HookMessageInfo for the given hook and variant, // or nil if not found. // @@ -89,22 +71,6 @@ func Lookup(hookName, variant string) *HookMessageInfo { return nil } -// Hooks returns a deduplicated list of hook names in the registry. -// -// Returns: -// - []string: Hook names in alphabetical order -func Hooks() []string { - seen := make(map[string]bool) - var hooks []string - for _, info := range Registry() { - if !seen[info.Hook] { - seen[info.Hook] = true - hooks = append(hooks, info.Hook) - } - } - return hooks -} - // Variants returns the variant names for a given hook. // // Parameters: diff --git a/internal/assets/hooks/messages/registry_test.go b/internal/assets/hooks/messages/registry_test.go index 6ab0f3777..5c387f7b4 100644 --- a/internal/assets/hooks/messages/registry_test.go +++ b/internal/assets/hooks/messages/registry_test.go @@ -6,7 +6,11 @@ package messages -import "testing" +import ( + "testing" + + cfgHook "github.com/ActiveMemory/ctx/internal/config/hook" +) func TestRegistryCount(t *testing.T) { entries := Registry() @@ -19,8 +23,8 @@ func TestRegistryCount(t *testing.T) { } func TestRegistryYAMLParses(t *testing.T) { - if parseErr := RegistryError(); parseErr != nil { - t.Fatalf("RegistryError() = %v, want nil", parseErr) + if parseErr := registryError(); parseErr != nil { + t.Fatalf("registryError() = %v, want nil", parseErr) } for i, entry := range Registry() { @@ -30,8 +34,8 @@ func TestRegistryYAMLParses(t *testing.T) { if entry.Variant == "" { t.Errorf("entry %d: empty variant", i) } - validCategory := entry.Category == CategoryCustomizable || - entry.Category == CategoryCtxSpecific + validCategory := entry.Category == cfgHook.CategoryCustomizable || + entry.Category == cfgHook.CategoryCtxSpecific if !validCategory { t.Errorf("entry %d (%s/%s): invalid category %q", i, entry.Hook, entry.Variant, entry.Category) @@ -48,8 +52,8 @@ func TestLookupKnownEntry(t *testing.T) { if info == nil { t.Fatal("Lookup(check-persistence, nudge) = nil, want non-nil") } - if info.Category != CategoryCustomizable { - t.Errorf("category = %q, want %q", info.Category, CategoryCustomizable) + if info.Category != cfgHook.CategoryCustomizable { + t.Errorf("category = %q, want %q", info.Category, cfgHook.CategoryCustomizable) } if info.Description != "Context persistence nudge" { t.Errorf("description = %q, want %q", @@ -69,7 +73,7 @@ func TestLookupUnknown(t *testing.T) { } func TestHooksReturnsUniqueNames(t *testing.T) { - hooks := Hooks() + hooks := hooks() if len(hooks) == 0 { t.Fatal("Hooks() returned empty list") } diff --git a/internal/assets/tpl/tpl_steering.go b/internal/assets/tpl/tpl_steering.go new file mode 100644 index 000000000..62e38628f --- /dev/null +++ b/internal/assets/tpl/tpl_steering.go @@ -0,0 +1,55 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package tpl + +// Foundation steering file names. +const ( + // SteeringNameProduct is the name for the product context file. + SteeringNameProduct = "product" + // SteeringNameTech is the name for the technology stack file. + SteeringNameTech = "tech" + // SteeringNameStructure is the name for the project structure file. + SteeringNameStructure = "structure" + // SteeringNameWorkflow is the name for the development workflow file. + SteeringNameWorkflow = "workflow" +) + +// Foundation steering file descriptions. +const ( + // SteeringDescProduct describes the product context file. + SteeringDescProduct = "Product context, goals, and target users" + // SteeringDescTech describes the technology stack file. + SteeringDescTech = "Technology stack, constraints, " + + "and dependencies" + // SteeringDescStructure describes the project structure file. + SteeringDescStructure = "Project structure and " + + "directory conventions" + // SteeringDescWorkflow describes the development workflow file. + SteeringDescWorkflow = "Development workflow and process rules" +) + +// Foundation steering file body templates. +const ( + // SteeringBodyProduct is the body for the product context file. + SteeringBodyProduct = "# Product Context\n\n" + + "Describe the product, its goals, " + + "and target users.\n" + // SteeringBodyTech is the body for the technology stack file. + SteeringBodyTech = "# Technology Stack\n\n" + + "Describe the technology stack, " + + "constraints, and key dependencies.\n" + // SteeringBodyStructure is the body for the project structure + // file. + SteeringBodyStructure = "# Project Structure\n\n" + + "Describe the project layout " + + "and directory conventions.\n" + // SteeringBodyWorkflow is the body for the development workflow + // file. + SteeringBodyWorkflow = "# Development Workflow\n\n" + + "Describe the development workflow, " + + "branching strategy, and process rules.\n" +) diff --git a/internal/assets/tpl/tpl_trigger.go b/internal/assets/tpl/tpl_trigger.go new file mode 100644 index 000000000..969017bbd --- /dev/null +++ b/internal/assets/tpl/tpl_trigger.go @@ -0,0 +1,32 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package tpl + +// Shell script template for new hook scripts created by +// ctx hook add. +const ( + // TriggerScript is the bash hook script template. + // Args: name, hookType. + TriggerScript = `#!/usr/bin/env bash +# Hook: %s +# Type: %s +# Created by: ctx hook add + +set -euo pipefail + +INPUT=$(cat) + +# Parse input fields +HOOK_TYPE=$(echo "$INPUT" | jq -r '.hookType') +TOOL=$(echo "$INPUT" | jq -r '.tool // empty') + +# Your hook logic here + +# Return output +echo '{"cancel": false, "context": "", "message": ""}' +` +) diff --git a/internal/audit/cli_cmd_structure_test.go b/internal/audit/cli_cmd_structure_test.go index 996d2161e..1efbede93 100644 --- a/internal/audit/cli_cmd_structure_test.go +++ b/internal/audit/cli_cmd_structure_test.go @@ -21,6 +21,10 @@ var allowedCmdFiles = map[string]bool{ "doc.go": true, } +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // cmdSubdirAllowlist lists cmd/ subdirectories with // stray files that cannot be moved to core/. This // should be empty. diff --git a/internal/audit/cross_package_types_test.go b/internal/audit/cross_package_types_test.go index 8dc426eb0..56cbd79f9 100644 --- a/internal/audit/cross_package_types_test.go +++ b/internal/audit/cross_package_types_test.go @@ -14,6 +14,10 @@ import ( "golang.org/x/tools/go/packages" ) +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // typeExemptPackages lists packages where exported // types are expected to be used cross-package by // design (entity, config, proto, etc.). @@ -201,10 +205,10 @@ func sameModule(a, b string) bool { } // cli/* consuming any domain module is the // standard consumer layer pattern. - if isConsumerLayer(ma) && !isConsumerLayer(mb) { + if consumerLayer(ma) && !consumerLayer(mb) { return true } - if isConsumerLayer(mb) && !isConsumerLayer(ma) { + if consumerLayer(mb) && !consumerLayer(ma) { return true } // err/ consumed from cli/ or . @@ -274,8 +278,8 @@ func moduleRoot(pkgPath string) string { return parts[0] } -// isConsumerLayer returns true if the module root is a +// consumerLayer returns true if the module root is a // consumer layer that naturally imports domain types. -func isConsumerLayer(mod string) bool { +func consumerLayer(mod string) bool { return strings.HasPrefix(mod, "cli/") } diff --git a/internal/audit/dead_exports_test.go b/internal/audit/dead_exports_test.go index ec6878dd7..856d69b13 100644 --- a/internal/audit/dead_exports_test.go +++ b/internal/audit/dead_exports_test.go @@ -26,31 +26,10 @@ import ( // internal and may be used via reflection or are // genuinely file-scoped helpers. -// testOnlyExports lists exported symbols that exist -// solely for test usage. The dead-export scanner skips -// test files, so these would otherwise be false -// positives. Keep this list small: prefer eliminating -// the export over adding it here. -var testOnlyExports = map[string]bool{ - "github.com/ActiveMemory/ctx/internal/assets/hooks/messages.CategoryCustomizable": true, - "github.com/ActiveMemory/ctx/internal/assets/hooks/messages.Hooks": true, - "github.com/ActiveMemory/ctx/internal/assets/hooks/messages.RegistryError": true, - "github.com/ActiveMemory/ctx/internal/cli/initialize/core/vscode.CreateVSCodeArtifacts": true, - "github.com/ActiveMemory/ctx/internal/cli/journal/core/lock.LockedFrontmatterLine": true, - "github.com/ActiveMemory/ctx/internal/cli/pad/core/store.EnsureGitignore": true, - "github.com/ActiveMemory/ctx/internal/cli/system/core/state.SetDirForTest": true, - "github.com/ActiveMemory/ctx/internal/config/asset.DirReferences": true, - "github.com/ActiveMemory/ctx/internal/config/regex.Phase": true, - "github.com/ActiveMemory/ctx/internal/inspect.StartsWithCtxMarker": true, - "github.com/ActiveMemory/ctx/internal/journal/parser.Parser": true, - "github.com/ActiveMemory/ctx/internal/journal/parser.RegisteredTools": true, - "github.com/ActiveMemory/ctx/internal/mcp/proto.ErrCodeInvalidReq": true, - "github.com/ActiveMemory/ctx/internal/mcp/proto.InitializeParams": true, - "github.com/ActiveMemory/ctx/internal/mcp/proto.UnsubscribeParams": true, - "github.com/ActiveMemory/ctx/internal/rc.Reset": true, - "github.com/ActiveMemory/ctx/internal/task.MatchFull": true, -} - +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // linuxOnlyExports lists exported symbols used only from // _linux.go source files. These appear dead on non-Linux // builds because go/packages loads only the current @@ -146,9 +125,30 @@ func TestNoDeadExports(t *testing.T) { } } - // Phase 3: remove test-only allowlist entries. - for key := range testOnlyExports { - delete(defs, key) + // Phase 2.5: remove symbols used cross-package in + // test files. If a test in package B imports a + // symbol from package A, the symbol is test + // infrastructure — not dead. Same-package test + // usage does not count (those should be unexported). + testPkgs := loadTestPackages(t) + for _, pkg := range testPkgs { + for ident, obj := range pkg.TypesInfo.Uses { + if obj == nil || obj.Pkg() == nil { + continue + } + pos := pkg.Fset.Position(ident.Pos()) + if !isTestFile(pos.Filename) { + continue + } + // Cross-package: the test's package path + // differs from the symbol's defining package. + if pkg.PkgPath == obj.Pkg().Path() { + continue + } + key := obj.Pkg().Path() + "." + + obj.Name() + delete(defs, key) + } } // Phase 3b: remove Linux-only exports (used from @@ -210,6 +210,30 @@ func loadCmdPackages(t *testing.T) []*packages.Package { return pkgs } +// loadTestPackages loads internal/ packages WITH test +// files for cross-package test usage detection. +func loadTestPackages( + t *testing.T, +) []*packages.Package { + t.Helper() + cfg := &packages.Config{ + Mode: packages.NeedName | + packages.NeedFiles | + packages.NeedSyntax | + packages.NeedTypes | + packages.NeedTypesInfo, + Tests: true, + } + pkgs, loadErr := packages.Load( + cfg, + "github.com/ActiveMemory/ctx/internal/...", + ) + if loadErr != nil { + t.Fatalf("packages.Load tests: %v", loadErr) + } + return pkgs +} + // isExported reports whether name starts with an // uppercase letter. func isExported(name string) bool { diff --git a/internal/audit/doc_comments_test.go b/internal/audit/doc_comments_test.go index 0a1d19392..0698bf325 100644 --- a/internal/audit/doc_comments_test.go +++ b/internal/audit/doc_comments_test.go @@ -59,10 +59,20 @@ func TestDocComments(t *testing.T) { singleton := !d.Lparen.IsValid() isCfg := configPackage(pkg.PkgPath) - // Config const/var blocks: group doc covers - // all specs. Report once per undocumented - // block, not per constant. + // Config const/var blocks (excluding + // embed/): group doc covers all specs + // because names are self-documenting + // (Dir*, File*, Perm*, etc.). + // + // DO NOT widen this exemption. New code + // must have per-constant doc comments. + // Widening requires a dedicated PR with + // justification — not a drive-by allowlist + // change to make tests pass. if isCfg && d.Lparen.IsValid() && + !strings.Contains( + pkg.PkgPath, "config/embed/", + ) && (d.Tok == token.CONST || d.Tok == token.VAR) { if d.Doc == nil { diff --git a/internal/audit/magic_strings_test.go b/internal/audit/magic_strings_test.go index 1d938bd3c..bf6e8c93c 100644 --- a/internal/audit/magic_strings_test.go +++ b/internal/audit/magic_strings_test.go @@ -17,6 +17,10 @@ import ( "golang.org/x/tools/go/packages" ) +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // exemptStrings lists string values always acceptable. var exemptStrings = map[string]bool{ "": true, // empty string @@ -27,21 +31,23 @@ var exemptStrings = map[string]bool{ ": ": true, // key-value separator } +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // exemptStringPackages lists package paths fully exempt // from magic string checks. var exemptStringPackages = []string{ "internal/config/", "internal/config", "internal/assets/tpl", - "internal/err/", } // TestNoMagicStrings flags magic string literals in non-test // Go files under internal/. // // Exempt: empty string, single space, indentation strings, -// single characters, format verbs, regex replacements, HTML -// entities, URL scheme prefixes, config/tpl/err packages, +// regex capture references, config/tpl/err packages, // file-level const/var definitions, import paths, struct tags. // // Test files are exempt. @@ -74,10 +80,14 @@ func TestNoMagicStrings(t *testing.T) { return true } - if isConstDef(file, lit) || - isVarDef(file, lit) { - return true - } + // Const/var definitions in exempt packages + // are already skipped (line 61). Outside + // those packages, string constants are + // magic strings that belong in config/. + // + // DO NOT re-add a blanket isConstDef + // exemption. It masks constants defined + // in the wrong package. if isStructTag(file, lit) { return true @@ -127,27 +137,11 @@ func checkMagicString( return } - // Format verbs ("%s", "%d %s", etc.). - if isFormatString(s) { - return - } - // Regex capture group references. if isRegexRef(s) { return } - // HTML entities (<, >, etc.). - if strings.HasPrefix(s, "&") && - strings.HasSuffix(s, ";") { - return - } - - // URL scheme prefixes. - if strings.HasSuffix(s, "://") { - return - } - *violations = append(*violations, posString(pkg.Fset, lit.Pos())+ ": magic string "+lit.Value, @@ -165,17 +159,6 @@ func isExemptStringPackage(pkgPath string) bool { return false } -// fmtVerb matches a printf format directive. -var fmtVerb = regexp.MustCompile( - `%[-+#0 ]*\d*\.?\d*[sdvqwfegxXobctpT]`, -) - -// isFormatString reports whether s contains a printf -// format directive. -func isFormatString(s string) bool { - return fmtVerb.MatchString(s) -} - // regexRef matches regex capture group references. var regexRef = regexp.MustCompile(`\$\d|\$\{`) diff --git a/internal/audit/magic_values_test.go b/internal/audit/magic_values_test.go index 388f2074a..ee173949e 100644 --- a/internal/audit/magic_values_test.go +++ b/internal/audit/magic_values_test.go @@ -13,6 +13,10 @@ import ( "testing" ) +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // exemptIntLiterals lists integer values that are always acceptable. // 0, 1, -1: universal identity/sentinel values. // 2, 3: structural constants (split counts, field indices, ternary). @@ -50,6 +54,10 @@ var strconvFuncs = map[string]bool{ "AppendFloat": true, } +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // exemptPackagePaths lists package path substrings that are fully // exempt from magic value checks — config definitions, template // definitions, and error constructors. @@ -91,12 +99,14 @@ func TestNoMagicValues(t *testing.T) { return true } - // Skip file-level const/var definition sites only. - // Local consts inside function bodies are NOT exempt — - // they are just renamed magic numbers. - if isConstDef(file, lit) || isVarDef(file, lit) { - return true - } + // Const/var definitions in exempt packages + // are already skipped (line 86). Outside + // those packages, numeric constants are + // magic values that belong in config/. + // + // DO NOT re-add a blanket isConstDef + // exemption. It masks constants defined + // in the wrong package. if exemptIntLiterals[lit.Value] { return true @@ -149,28 +159,6 @@ func isExemptPackage(pkgPath string) bool { return false } -// isVarDef reports whether lit appears inside a var declaration. -func isVarDef(file *ast.File, lit *ast.BasicLit) bool { - for _, decl := range file.Decls { - gd, ok := decl.(*ast.GenDecl) - if !ok || gd.Tok != token.VAR { - continue - } - for _, spec := range gd.Specs { - vs, ok := spec.(*ast.ValueSpec) - if !ok { - continue - } - for _, val := range vs.Values { - if containsNode(val, lit) { - return true - } - } - } - } - return false -} - // isStrconvArg reports whether lit is an argument to a strconv // function (radix or bitsize parameter). func isStrconvArg(file *ast.File, lit *ast.BasicLit) bool { diff --git a/internal/audit/string_concat_paths_test.go b/internal/audit/string_concat_paths_test.go index c7e5f583c..f3acb684b 100644 --- a/internal/audit/string_concat_paths_test.go +++ b/internal/audit/string_concat_paths_test.go @@ -18,6 +18,10 @@ import ( // (case-insensitive), suggest the variable holds a filesystem path. var pathVarHints = []string{"path", "dir", "folder", "file"} +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // stringConcatPathAllowlist lists known false positives where string // concatenation is used for non-path purposes (e.g. substring search // patterns, extension appending). diff --git a/internal/audit/type_file_convention_test.go b/internal/audit/type_file_convention_test.go index 90ac260f7..4cfb5ed84 100644 --- a/internal/audit/type_file_convention_test.go +++ b/internal/audit/type_file_convention_test.go @@ -16,6 +16,10 @@ import ( "testing" ) +// DO NOT add entries here to make tests pass. New code must +// conform to the check. Widening requires a dedicated PR with +// justification for each entry. +// // exemptTypePackages lists package path segments where // types intentionally do NOT live in types.go. Each // has a documented reason. diff --git a/internal/bootstrap/cmd.go b/internal/bootstrap/cmd.go index 9eaf19f5d..0f74932e0 100644 --- a/internal/bootstrap/cmd.go +++ b/internal/bootstrap/cmd.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/ActiveMemory/ctx/internal/assets/read/desc" + cfgBootstrap "github.com/ActiveMemory/ctx/internal/config/bootstrap" "github.com/ActiveMemory/ctx/internal/config/cli" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" embedFlag "github.com/ActiveMemory/ctx/internal/config/embed/flag" @@ -30,7 +31,7 @@ import ( // version is set at build time via ldflags: // // -X github.com/ActiveMemory/ctx/internal/bootstrap.version=$(cat VERSION) -var version = "dev" +var version = cfgBootstrap.DefaultVersion // RootCmd creates and returns the root cobra command for the ctx CLI. // diff --git a/internal/cli/dep/core/python/python.go b/internal/cli/dep/core/python/python.go index 912da600e..eeb7facd3 100644 --- a/internal/cli/dep/core/python/python.go +++ b/internal/cli/dep/core/python/python.go @@ -20,14 +20,13 @@ import ( ctxLog "github.com/ActiveMemory/ctx/internal/log/warn" ) -// Ecosystem is the ecosystem label for Python projects. -const Ecosystem = "python" - // Builder implements GraphBuilder for Python projects. type Builder struct{} // Name returns the ecosystem label. -func (p *Builder) Name() string { return Ecosystem } +func (p *Builder) Name() string { + return cfgDep.EcosystemPython +} // Detect returns true if requirements.txt or pyproject.toml // exists. @@ -305,7 +304,7 @@ func ParsePyprojectDeps( ); idx > 0 { name := strings.TrimSpace(trimmed[:idx]) lower := strings.ToLower(name) - if lower == Ecosystem || + if lower == cfgDep.EcosystemPython || cfgDep.PyMetaKeys[lower] { continue } diff --git a/internal/cli/dep/core/rust/rust.go b/internal/cli/dep/core/rust/rust.go index 73533c0a3..edd14731f 100644 --- a/internal/cli/dep/core/rust/rust.go +++ b/internal/cli/dep/core/rust/rust.go @@ -16,14 +16,13 @@ import ( execDep "github.com/ActiveMemory/ctx/internal/exec/dep" ) -// Ecosystem is the ecosystem label for Rust projects. -const Ecosystem = "rust" - // Builder implements GraphBuilder for Rust projects. type Builder struct{} // Name returns the ecosystem label. -func (r *Builder) Name() string { return Ecosystem } +func (r *Builder) Name() string { + return cfgDep.EcosystemRust +} // Detect returns true if Cargo.toml exists in the current // directory. diff --git a/internal/cli/doctor/core/check/check.go b/internal/cli/doctor/core/check/check.go index 5e439fd3e..3cf71e5f0 100644 --- a/internal/cli/doctor/core/check/check.go +++ b/internal/cli/doctor/core/check/check.go @@ -25,6 +25,7 @@ import ( "github.com/ActiveMemory/ctx/internal/config/regex" "github.com/ActiveMemory/ctx/internal/config/reminder" "github.com/ActiveMemory/ctx/internal/config/stats" + cfgSysinfo "github.com/ActiveMemory/ctx/internal/config/sysinfo" cfgToken "github.com/ActiveMemory/ctx/internal/config/token" "github.com/ActiveMemory/ctx/internal/context/load" "github.com/ActiveMemory/ctx/internal/context/token" @@ -623,7 +624,7 @@ func AddResourceResults( snap.Memory.TotalBytes, text.DescKeyDoctorResourceMemoryFormat, doctor.CheckResourceMemory, - sysinfo.ResourceMemory, + cfgSysinfo.ResourceMemory, }, { snap.Memory.Supported, @@ -631,7 +632,7 @@ func AddResourceResults( snap.Memory.SwapTotalBytes, text.DescKeyDoctorResourceSwapFormat, doctor.CheckResourceSwap, - sysinfo.ResourceSwap, + cfgSysinfo.ResourceSwap, }, { snap.Disk.Supported, @@ -639,7 +640,7 @@ func AddResourceResults( snap.Disk.TotalBytes, text.DescKeyDoctorResourceDiskFormat, doctor.CheckResourceDisk, - sysinfo.ResourceDisk, + cfgSysinfo.ResourceDisk, }, } for _, bc := range byteChecks { @@ -669,7 +670,7 @@ func AddResourceResults( Name: doctor.CheckResourceLoad, Category: doctor.CategoryResources, Status: SeverityToStatus( - sevMap[sysinfo.ResourceLoad], + sevMap[cfgSysinfo.ResourceLoad], ), Message: msg, }) diff --git a/internal/cli/drift/core/fix/fix.go b/internal/cli/drift/core/fix/fix.go index 63f763ad6..f1d50cdd7 100644 --- a/internal/cli/drift/core/fix/fix.go +++ b/internal/cli/drift/core/fix/fix.go @@ -17,6 +17,7 @@ import ( "github.com/ActiveMemory/ctx/internal/assets/read/template" "github.com/ActiveMemory/ctx/internal/config/archive" cfgCtx "github.com/ActiveMemory/ctx/internal/config/ctx" + cfgDrift "github.com/ActiveMemory/ctx/internal/config/drift" "github.com/ActiveMemory/ctx/internal/config/embed/text" "github.com/ActiveMemory/ctx/internal/config/fs" "github.com/ActiveMemory/ctx/internal/config/marker" @@ -67,7 +68,7 @@ func Apply( for _, issue := range report.Warnings { switch issue.Type { - case drift.IssueStaleness: + case cfgDrift.IssueStaleness: if fixErr := Staleness(cmd, ctx); fixErr != nil { result.Errors = append(result.Errors, fmt.Sprintf( @@ -79,7 +80,7 @@ func Apply( result.Fixed++ } - case drift.IssueMissing: + case cfgDrift.IssueMissing: if fixErr := MissingFile(issue.File); fixErr != nil { result.Errors = append(result.Errors, fmt.Sprintf( @@ -91,20 +92,20 @@ func Apply( result.Fixed++ } - case drift.IssueDeadPath: + case cfgDrift.IssueDeadPath: writeDrift.SkipDeadPath( cmd, issue.File, issue.Line, issue.Path, ) result.Skipped++ - case drift.IssueStaleAge: + case cfgDrift.IssueStaleAge: writeDrift.SkipStaleAge(cmd, issue.File) result.Skipped++ } } for _, issue := range report.Violations { - if issue.Type == drift.IssueSecret { + if issue.Type == cfgDrift.IssueSecret { writeDrift.SkipSensitiveFile(cmd, issue.File) result.Skipped++ } diff --git a/internal/cli/drift/core/out/out.go b/internal/cli/drift/core/out/out.go index 510219831..633e50e7f 100644 --- a/internal/cli/drift/core/out/out.go +++ b/internal/cli/drift/core/out/out.go @@ -15,6 +15,7 @@ import ( "github.com/ActiveMemory/ctx/internal/assets/read/desc" "github.com/ActiveMemory/ctx/internal/cli/drift/core/sanitize" + cfgDrift "github.com/ActiveMemory/ctx/internal/config/drift" "github.com/ActiveMemory/ctx/internal/config/embed/text" "github.com/ActiveMemory/ctx/internal/drift" errDrift "github.com/ActiveMemory/ctx/internal/err/drift" @@ -31,11 +32,11 @@ import ( // - Violations: Constitution violations // - Passed: Names of checks that passed type JSONOutput struct { - Timestamp string `json:"timestamp"` - Status drift.StatusType `json:"status"` - Warnings []drift.Issue `json:"warnings"` - Violations []drift.Issue `json:"violations"` - Passed []drift.CheckName `json:"passed"` + Timestamp string `json:"timestamp"` + Status cfgDrift.StatusType `json:"status"` + Warnings []drift.Issue `json:"warnings"` + Violations []drift.Issue `json:"violations"` + Passed []cfgDrift.CheckName `json:"passed"` } // DriftText writes the drift report as formatted text with @@ -93,9 +94,9 @@ func DriftText( for _, w := range report.Warnings { switch w.Type { - case drift.IssueDeadPath: + case cfgDrift.IssueDeadPath: pathRefs = append(pathRefs, w) - case drift.IssueStaleness, drift.IssueStaleAge: + case cfgDrift.IssueStaleness, cfgDrift.IssueStaleAge: staleness = append(staleness, w) default: other = append(other, w) @@ -152,10 +153,10 @@ func DriftText( // Summary status := report.Status() switch status { - case drift.StatusViolation: + case cfgDrift.StatusViolation: writeDrift.StatusViolation(cmd) return errDrift.Violations() - case drift.StatusWarning: + case cfgDrift.StatusWarning: writeDrift.StatusWarning(cmd) default: writeDrift.StatusOK(cmd) diff --git a/internal/cli/drift/core/sanitize/sanitize.go b/internal/cli/drift/core/sanitize/sanitize.go index bea58c0f6..e3b39f124 100644 --- a/internal/cli/drift/core/sanitize/sanitize.go +++ b/internal/cli/drift/core/sanitize/sanitize.go @@ -8,8 +8,8 @@ package sanitize import ( "github.com/ActiveMemory/ctx/internal/assets/read/desc" + cfgDrift "github.com/ActiveMemory/ctx/internal/config/drift" "github.com/ActiveMemory/ctx/internal/config/embed/text" - "github.com/ActiveMemory/ctx/internal/drift" ) // FormatCheckName converts internal check identifiers to @@ -21,23 +21,23 @@ import ( // Returns: // - string: Human-readable description, or the original // name if unknown -func FormatCheckName(name drift.CheckName) string { +func FormatCheckName(name cfgDrift.CheckName) string { switch name { - case drift.CheckPathReferences: + case cfgDrift.CheckPathReferences: return desc.Text(text.DescKeyDriftCheckPathRefs) - case drift.CheckStaleness: + case cfgDrift.CheckStaleness: return desc.Text(text.DescKeyDriftCheckStaleness) - case drift.CheckConstitution: + case cfgDrift.CheckConstitution: return desc.Text(text.DescKeyDriftCheckConstitution) - case drift.CheckRequiredFiles: + case cfgDrift.CheckRequiredFiles: return desc.Text(text.DescKeyDriftCheckRequired) - case drift.CheckFileAge: + case cfgDrift.CheckFileAge: return desc.Text(text.DescKeyDriftCheckFileAge) - case drift.CheckTemplateHeaders: + case cfgDrift.CheckTemplateHeaders: return desc.Text( text.DescKeyDriftCheckTemplateHeader, ) default: - return string(name) + return name } } diff --git a/internal/cli/initialize/core/vscode/vscode.go b/internal/cli/initialize/core/vscode/vscode.go index f85c9d228..44b3b1bce 100644 --- a/internal/cli/initialize/core/vscode/vscode.go +++ b/internal/cli/initialize/core/vscode/vscode.go @@ -15,7 +15,7 @@ import ( writeVscode "github.com/ActiveMemory/ctx/internal/write/vscode" ) -// CreateVSCodeArtifacts generates VS Code workspace configuration files +// createVSCodeArtifacts generates VS Code workspace configuration files // (.vscode/) during ctx init. // // Creates extensions.json, tasks.json, and mcp.json as the @@ -27,7 +27,7 @@ import ( // // Returns: // - error: Non-nil if directory creation fails -func CreateVSCodeArtifacts(cmd *cobra.Command) error { +func createVSCodeArtifacts(cmd *cobra.Command) error { mkdirErr := ctxIo.SafeMkdirAll( cfgVscode.Dir, fs.PermExec, ) diff --git a/internal/cli/initialize/core/vscode/vscode_test.go b/internal/cli/initialize/core/vscode/vscode_test.go index f8739708a..9cc8de571 100644 --- a/internal/cli/initialize/core/vscode/vscode_test.go +++ b/internal/cli/initialize/core/vscode/vscode_test.go @@ -125,25 +125,25 @@ func TestCreateVSCodeArtifacts_CreatesMCPJSON(t *testing.T) { var buf bytes.Buffer cmd := testCmd(&buf) - if err := CreateVSCodeArtifacts(cmd); err != nil { - t.Fatalf("CreateVSCodeArtifacts() error = %v", err) + if err := createVSCodeArtifacts(cmd); err != nil { + t.Fatalf("createVSCodeArtifacts() error = %v", err) } // Verify mcp.json was created as part of the artifacts target := filepath.Join(cfgVscode.Dir, "mcp.json") if _, err := os.Stat(target); os.IsNotExist(err) { - t.Error("CreateVSCodeArtifacts did not create mcp.json") + t.Error("createVSCodeArtifacts did not create mcp.json") } // Verify extensions.json was also created extTarget := filepath.Join(cfgVscode.Dir, "extensions.json") if _, err := os.Stat(extTarget); os.IsNotExist(err) { - t.Error("CreateVSCodeArtifacts did not create extensions.json") + t.Error("createVSCodeArtifacts did not create extensions.json") } // Verify tasks.json was also created taskTarget := filepath.Join(cfgVscode.Dir, "tasks.json") if _, err := os.Stat(taskTarget); os.IsNotExist(err) { - t.Error("CreateVSCodeArtifacts did not create tasks.json") + t.Error("createVSCodeArtifacts did not create tasks.json") } } diff --git a/internal/cli/journal/core/lock/lock.go b/internal/cli/journal/core/lock/lock.go index 42f899408..157ac69ea 100644 --- a/internal/cli/journal/core/lock/lock.go +++ b/internal/cli/journal/core/lock/lock.go @@ -30,9 +30,9 @@ import ( writeRecall "github.com/ActiveMemory/ctx/internal/write/journal" ) -// LockedFrontmatterLine is the YAML line inserted into frontmatter when +// lockedFrontmatterLine is the YAML line inserted into frontmatter when // a journal entry is locked. -const LockedFrontmatterLine = session.FrontmatterLockedLine +const lockedFrontmatterLine = session.FrontmatterLockedLine // MatchJournalFiles returns journal .md filenames matching the given // patterns. If all is true, returns every .md file in the directory. diff --git a/internal/cli/journal/core/lock/lock_test.go b/internal/cli/journal/core/lock/lock_test.go index 87dad4fa0..c8a1d41cc 100644 --- a/internal/cli/journal/core/lock/lock_test.go +++ b/internal/cli/journal/core/lock/lock_test.go @@ -152,7 +152,7 @@ func TestUpdateLockFrontmatter_Lock(t *testing.T) { if readErr != nil { t.Fatal(readErr) } - if !strings.Contains(string(data), LockedFrontmatterLine) { + if !strings.Contains(string(data), lockedFrontmatterLine) { t.Error("lock should insert locked line into frontmatter") } if !strings.Contains(string(data), "# Body") { @@ -164,7 +164,7 @@ func TestUpdateLockFrontmatter_Unlock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "test.md") content := "---\ndate: \"2026-01-21\"\n" + - LockedFrontmatterLine + + lockedFrontmatterLine + "\ntitle: \"Test\"\n---\n\n# Body\n" writeErr := os.WriteFile( path, []byte(content), fs.PermFile, @@ -213,7 +213,7 @@ func TestUpdateLockFrontmatter_IdempotentLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "test.md") content := "---\ndate: \"2026-01-21\"\n" + - LockedFrontmatterLine + "\n---\n\n# Body\n" + lockedFrontmatterLine + "\n---\n\n# Body\n" writeErr := os.WriteFile( path, []byte(content), fs.PermFile, ) diff --git a/internal/cli/journal/core/lock/unlock_test.go b/internal/cli/journal/core/lock/unlock_test.go index d5e267b27..e5e3bdbf0 100644 --- a/internal/cli/journal/core/lock/unlock_test.go +++ b/internal/cli/journal/core/lock/unlock_test.go @@ -13,8 +13,8 @@ import ( "testing" "github.com/ActiveMemory/ctx/internal/cli/journal" - "github.com/ActiveMemory/ctx/internal/cli/journal/core/lock" "github.com/ActiveMemory/ctx/internal/config/fs" + "github.com/ActiveMemory/ctx/internal/config/session" "github.com/ActiveMemory/ctx/internal/journal/state" ) @@ -65,7 +65,7 @@ func TestRunLockUnlock_LockSingle(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(string(data), lock.LockedFrontmatterLine) { + if !strings.Contains(string(data), session.FrontmatterLockedLine) { t.Error("frontmatter should contain locked line") } } @@ -79,7 +79,7 @@ func TestRunLockUnlock_UnlockSingle(t *testing.T) { filename := "2026-01-21-test-abc12345.md" content := "---\ndate: \"2026-01-21\"\n" + - lock.LockedFrontmatterLine + "\n---\n\n# Test\n" + session.FrontmatterLockedLine + "\n---\n\n# Test\n" if err := os.WriteFile( filepath.Join(journalDir, filename), []byte(content), fs.PermFile, ); err != nil { @@ -183,7 +183,7 @@ func TestRunLockUnlock_AlreadyLocked(t *testing.T) { filename := "2026-01-21-test-abc12345.md" content := "---\ndate: \"2026-01-21\"\n" + - lock.LockedFrontmatterLine + "\n---\n\n# Test\n" + session.FrontmatterLockedLine + "\n---\n\n# Test\n" if err := os.WriteFile( filepath.Join(journalDir, filename), []byte(content), fs.PermFile, ); err != nil { @@ -302,7 +302,7 @@ func TestRunLockUnlock_LockMultipart(t *testing.T) { if readErr != nil { t.Fatal(readErr) } - if !strings.Contains(string(data), lock.LockedFrontmatterLine) { + if !strings.Contains(string(data), session.FrontmatterLockedLine) { t.Errorf("%s frontmatter should contain locked line", f) } } diff --git a/internal/cli/journal/core/source/format/format.go b/internal/cli/journal/core/source/format/format.go index 5bf8eac5b..7bb962971 100644 --- a/internal/cli/journal/core/source/format/format.go +++ b/internal/cli/journal/core/source/format/format.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "html" + "strconv" "strings" "github.com/ActiveMemory/ctx/internal/assets/read/desc" @@ -270,7 +271,7 @@ func JournalEntryPart( sb.WriteString(tpl.MetaDetailsClose + nl + nl) // Token stats as collapsible HTML table - turnStr := fmt.Sprintf("%d", s.TurnCount) + turnStr := strconv.Itoa(s.TurnCount) io.SafeFprintf(&sb, tpl.MetaDetailsOpen, turnStr) io.SafeFprintf(&sb, tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaTurns), turnStr) @@ -282,7 +283,7 @@ func JournalEntryPart( tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaTokens), tokenSummary) if totalParts > 1 { io.SafeFprintf(&sb, tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaParts), - fmt.Sprintf("%d", totalParts)) + strconv.Itoa(totalParts)) } sb.WriteString(tpl.MetaDetailsClose + nl + nl) diff --git a/internal/cli/journal/core/source/list.go b/internal/cli/journal/core/source/list.go index 31e436678..13246e6d3 100644 --- a/internal/cli/journal/core/source/list.go +++ b/internal/cli/journal/core/source/list.go @@ -8,6 +8,7 @@ package source import ( "fmt" + "strconv" "strings" goTime "time" @@ -154,7 +155,7 @@ func RunList(cmd *cobra.Command, opts Opts) error { time.DateTimeFmt, ) dur := srcFmt.Duration(s.Duration) - turns := fmt.Sprintf("%d", s.TurnCount) + turns := strconv.Itoa(s.TurnCount) tokens := "" if s.TotalTokens > 0 { tokens = sharedFmt.Tokens(s.TotalTokens) diff --git a/internal/cli/journal/core/wikilink/wikilink.go b/internal/cli/journal/core/wikilink/wikilink.go index 0a94f3abb..35736de08 100644 --- a/internal/cli/journal/core/wikilink/wikilink.go +++ b/internal/cli/journal/core/wikilink/wikilink.go @@ -14,6 +14,7 @@ import ( "github.com/ActiveMemory/ctx/internal/assets/read/desc" "github.com/ActiveMemory/ctx/internal/config/embed/text" "github.com/ActiveMemory/ctx/internal/config/file" + cfgHTTP "github.com/ActiveMemory/ctx/internal/config/http" "github.com/ActiveMemory/ctx/internal/config/obsidian" "github.com/ActiveMemory/ctx/internal/config/regex" "github.com/ActiveMemory/ctx/internal/config/token" @@ -40,9 +41,9 @@ func ConvertMarkdownLinks(content string) string { target := parts[2] // Skip external links - if strings.HasPrefix(target, "http://") || - strings.HasPrefix(target, "https://") || - strings.HasPrefix(target, "file://") { + if strings.HasPrefix(target, cfgHTTP.PrefixHTTP) || + strings.HasPrefix(target, cfgHTTP.PrefixHTTPS) || + strings.HasPrefix(target, cfgHTTP.PrefixFile) { return match } diff --git a/internal/cli/pad/core/export/timestamp.go b/internal/cli/pad/core/export/timestamp.go index 28d6d18cc..d8d1ee052 100644 --- a/internal/cli/pad/core/export/timestamp.go +++ b/internal/cli/pad/core/export/timestamp.go @@ -7,7 +7,7 @@ package export import ( - "fmt" + "strconv" "time" "github.com/ActiveMemory/ctx/internal/config/token" @@ -21,6 +21,6 @@ import ( // Returns: // - string: Label in the form "-