From 0021e98164d503ca956ce76a7623a4e55b0bf22b Mon Sep 17 00:00:00 2001 From: doguhanniltextra Date: Wed, 18 Feb 2026 10:17:11 +0300 Subject: [PATCH 1/2] fix(add): prevent decision insertion inside HTML comment block On a fresh DECISIONS.md the template contains a multi-line comment that includes an example heading '## [YYYY-MM-DD] Decision Title'. Two bugs caused new decisions to land inside that comment block, making them invisible when rendered: 1. insertDecision used strings.Index to find the first '## [' occurrence, which matched the template example inside the comment. 2. insertAfterHeader (the fallback path) only skipped ctx-specific markers (, ), so it stopped at and inserted before the index table and format-guide comment. Fix: - Add isInsideHTMLComment(content, idx) helper that checks whether a byte position falls between a . - Rewrite insertDecision and insertLearning to walk all '## [' occurrences in a loop, skipping any that isInsideHTMLComment identifies as being inside a comment block. - Update insertAfterHeader to skip any block (not just ctx-specific markers), so the fallback path clears the INDEX markers and the DECISION FORMATS guide before placing the entry. Add TestInsertDecisionNotInsideComment regression test that reproduces the exact fresh-file structure from the bug report and asserts the new entry appears after the closing --> of the DECISION FORMATS block. Signed-off-by: doguhanniltextra --- internal/cli/add/add_test.go | 53 ++++++++++++++++ internal/cli/add/insert.go | 114 +++++++++++++++++++++++++---------- 2 files changed, 136 insertions(+), 31 deletions(-) diff --git a/internal/cli/add/add_test.go b/internal/cli/add/add_test.go index 9a965f0a..32b1814d 100644 --- a/internal/cli/add/add_test.go +++ b/internal/cli/add/add_test.go @@ -371,6 +371,59 @@ func TestAppendEntry(t *testing.T) { }) } +// TestInsertDecisionNotInsideComment is a regression test for the bug where +// ctx add decision inserts the entry inside the +// HTML comment block on a fresh DECISIONS.md, making it invisible when rendered. +func TestInsertDecisionNotInsideComment(t *testing.T) { + // This is the exact structure of a freshly scaffolded DECISIONS.md. + freshDecisions := "# Decisions\n\n" + + "\n" + + "\n\n" + + "\n" + + entry := "## [2026-02-18] My Important Decision\n\n" + + "**Status**: Accepted\n\n" + + "**Context**: What prompted it\n\n" + + "**Rationale**: Why this choice\n\n" + + "**Consequences**: What changes\n" + + result := AppendEntry([]byte(freshDecisions), entry, "decision", "") + resultStr := string(result) + + // The entry must appear in the output. + if !strings.Contains(resultStr, "My Important Decision") { + t.Fatalf("decision not found in result:\n%s", resultStr) + } + + // The closing --> of the DECISION FORMATS comment block must appear BEFORE + // the new entry. Use LastIndex because the INDEX:START/END markers also + // contain --> on the same line; the last --> in the original template is + // the one that closes the multi-line DECISION FORMATS block. + // + // We search in the original template content (before the entry was added) + // to find where the DECISION FORMATS block closes, then verify the new + // entry appears after that position. + formatBlockCloseIdx := strings.LastIndex(freshDecisions, "-->") + entryIdx := strings.Index(resultStr, "My Important Decision") + if formatBlockCloseIdx == -1 { + t.Fatal("closing --> not found in template") + } + if entryIdx <= formatBlockCloseIdx { + t.Errorf( + "decision was inserted inside the HTML comment block: "+ + "entry at index %d, DECISION FORMATS block closes at index %d\n\nFull result:\n%s", + entryIdx, formatBlockCloseIdx, resultStr, + ) + } +} + // TestInsertTaskDefaultPlacement tests task insertion without --section. func TestInsertTaskDefaultPlacement(t *testing.T) { t.Run("inserts before first unchecked task", func(t *testing.T) { diff --git a/internal/cli/add/insert.go b/internal/cli/add/insert.go index 1d7a8569..4e9451b8 100644 --- a/internal/cli/add/insert.go +++ b/internal/cli/add/insert.go @@ -14,8 +14,10 @@ import ( // insertAfterHeader finds a header line and inserts content after it. // -// Skips blank lines and ctx markers between the header and insertion point. -// Falls back to appending at the end if the header is not found. +// Skips blank lines and HTML comment blocks () between the header +// and the insertion point, so new entries land after index tables, format +// guides, and other comment-wrapped metadata. Falls back to appending at the +// end if the header is not found. // // Parameters: // - content: Existing file content @@ -39,19 +41,20 @@ func insertAfterHeader(content, entry, header string) []byte { insertPoint := idx + lineEnd insertPoint = skipNewline(content, insertPoint) - // Skip blank lines and ctx markers + // Skip blank lines and any HTML comment blocks (). + // This handles INDEX markers, format-guide comments, and ctx markers alike. for insertPoint < len(content) { if n := skipNewline(content, insertPoint); n > insertPoint { insertPoint = n continue } - // No context marker: we found the insertion point. - if !startsWithCtxMarker(content[insertPoint:]) { + // Not an HTML comment: we found the insertion point. + if !strings.HasPrefix(content[insertPoint:], config.CommentOpen) { break } - // Skip past the closing marker + // Skip past the closing --> of this comment block. hasCommentEnd, endIdx := containsEndComment(content[insertPoint:]) if !hasCommentEnd { break @@ -150,11 +153,37 @@ func insertTaskAfterSection(entry, content, section string) []byte { return []byte(content + config.NewlineLF + entry) } +// isInsideHTMLComment reports whether the position idx in content falls +// inside an HTML comment block (). +// +// Parameters: +// - content: String to check +// - idx: Position to test +// +// Returns: +// - bool: True if idx is between a +func isInsideHTMLComment(content string, idx int) bool { + // Find the last closes that block before idx + closeIdx := strings.Index(content[openIdx:], config.CommentClose) + if closeIdx == -1 { + // Unclosed comment — treat as inside + return true + } + // The comment closes at openIdx+closeIdx; if that position is >= idx, + // the position is still inside the comment. + return openIdx+closeIdx+len(config.CommentClose) > idx +} + // insertDecision inserts a decision entry before existing entries. // -// Finds the first "## [" marker and inserts before it, maintaining -// reverse-chronological order. Falls back to insertAfterHeader if no entries -// exist. +// Finds the first "## [" marker that is NOT inside an HTML comment block +// and inserts before it, maintaining reverse-chronological order. +// Falls back to insertAfterHeader if no real entries exist yet. // // Parameters: // - content: Existing file content @@ -164,27 +193,39 @@ func insertTaskAfterSection(entry, content, section string) []byte { // Returns: // - []byte: Modified content with entry inserted func insertDecision(content, entry, header string) []byte { - // Find the first entry marker "## [" (timestamp-prefixed sections) - entryIdx := strings.Index(content, "## [") - if entryIdx != -1 { - // Insert before the first entry, with a separator after - return []byte( - content[:entryIdx] + entry + - config.NewlineLF + config.Separator + - config.NewlineLF + config.NewlineLF + - content[entryIdx:], - ) + // Walk through all "## [" occurrences, skipping those inside HTML comments + // (e.g. the template example inside ). + search := content + offset := 0 + for { + rel := strings.Index(search, "## [") + if rel == -1 { + break + } + entryIdx := offset + rel + if !isInsideHTMLComment(content, entryIdx) { + // Found a real entry — insert before it. + return []byte( + content[:entryIdx] + entry + + config.NewlineLF + config.Separator + + config.NewlineLF + config.NewlineLF + + content[entryIdx:], + ) + } + // This match is inside a comment — skip past it and keep looking. + offset = entryIdx + len("## [") + search = content[offset:] } - // No existing entries - find the header and insert after it + // No existing real entries - find the header and insert after it return insertAfterHeader(content, entry, header) } // insertLearning inserts a learning entry before existing entries. // -// Finds the first "## [" marker and inserts before it, maintaining -// reverse-chronological order. Falls back to insertAfterHeader if no entries -// exist. +// Finds the first "## [" marker that is NOT inside an HTML comment block +// and inserts before it, maintaining reverse-chronological order. +// Falls back to insertAfterHeader if no real entries exist yet. // // Parameters: // - content: Existing file content @@ -193,14 +234,25 @@ func insertDecision(content, entry, header string) []byte { // Returns: // - []byte: Modified content with entry inserted func insertLearning(content, entry string) []byte { - // Find the first entry marker "## [" (timestamp-prefixed sections) - entryIdx := strings.Index(content, config.HeadingLearningStart) - if entryIdx != -1 { - return []byte( - content[:entryIdx] + entry + config.NewlineLF + - config.Separator + config.NewlineLF + config.NewlineLF + - content[entryIdx:], - ) + // Walk through all "## [" occurrences, skipping those inside HTML comments. + search := content + offset := 0 + for { + rel := strings.Index(search, config.HeadingLearningStart) + if rel == -1 { + break + } + entryIdx := offset + rel + if !isInsideHTMLComment(content, entryIdx) { + return []byte( + content[:entryIdx] + entry + config.NewlineLF + + config.Separator + config.NewlineLF + config.NewlineLF + + content[entryIdx:], + ) + } + // This match is inside a comment — skip past it and keep looking. + offset = entryIdx + len(config.HeadingLearningStart) + search = content[offset:] } // No existing entries - find the header and insert after it From 77a93d18b11359ea3fb9c070267e82a046afd9f7 Mon Sep 17 00:00:00 2001 From: doguhanniltextra Date: Wed, 18 Feb 2026 10:21:40 +0300 Subject: [PATCH 2/2] test(add): expand coverage for HTML comment insertion fix - Split TestInsertDecisionNotInsideComment into sub-tests: * first decision lands after comment block * second decision prepends before first (verifies prepend order still works correctly after the fix) - Add TestIsInsideHTMLComment with 8 table-driven cases covering: * position before, inside, and after a comment * inline single-line comment () * multi-line comment with heading inside vs after close * unclosed comment (treated as inside) * content with no comment at all Signed-off-by: doguhanniltextra --- internal/cli/add/add_test.go | 69 ++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/internal/cli/add/add_test.go b/internal/cli/add/add_test.go index 32b1814d..4932d041 100644 --- a/internal/cli/add/add_test.go +++ b/internal/cli/add/add_test.go @@ -554,3 +554,72 @@ func TestAddFromFile(t *testing.T) { t.Error("content from file was not added to LEARNINGS.md") } } + +// TestIsInsideHTMLComment unit-tests the isInsideHTMLComment helper directly. +func TestIsInsideHTMLComment(t *testing.T) { + cases := []struct { + name string + content string + idx int + want bool + }{ + { + name: "position before any comment", + content: "hello world", + idx: 3, // inside "hello" + want: false, + }, + { + name: "position inside comment", + content: "hello world", + idx: 10, // inside " comment " + want: true, + }, + { + name: "position after comment close", + content: "hello world", + idx: 23, // inside " world" + want: false, + }, + { + name: "inline single-line comment: position after close", + content: "\n## [real entry]", + idx: 20, // start of "## [" + want: false, + }, + { + name: "multi-line comment: heading inside", + content: "\n## [real]", + idx: 13, // start of "## [YYYY" + want: true, + }, + { + name: "multi-line comment: heading after close", + content: "\n## [real]", + idx: 41, // start of "## [real]" + want: false, + }, + { + name: "unclosed comment treated as inside", + content: "