feat: auto-interactive markdown checkboxes

[adnaan]

Summary

• Markdown task lists (- [ ] text) under headings automatically become interactive checkboxes with zero configuration — no frontmatter, no code blocks, no separate data files
• Changes persist to the original markdown file via the existing MarkdownSource pipeline
• Pre-processing transforms in-memory markdown to inject lvt code blocks and source configs, reusing the entire existing pipeline unchanged
• Watcher detects same-file sources (page file IS the data source) to avoid full page reloads on checkbox toggle
• Fixes concurrent websocket write panic when watcher refresh races with action response
• Fixes Docker Chrome E2E setup on macOS (port mapping instead of --network host)

Closes #119

Test plan

• 14 unit tests for detection, preprocessing, slugification, template generation
• 5 E2E browser tests (Docker Chrome): BasicToggle, AddTask, MultipleSections, PersistAcrossReload, NoFullReload
• All internal package tests pass
• go vet clean
• Manual testing of check/uncheck/add in browser

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR introduces “zero-config” interactive Markdown task lists by auto-detecting task-list-only sections under headings and transforming them (in-memory) into lvt blocks wired to the existing markdown source persistence pipeline. It also adjusts live-reload behavior to avoid full page reloads on checkbox toggles, serializes WebSocket writes to prevent concurrent write panics, and updates Docker Chrome E2E helpers for macOS.

Changes:

• Add markdown preprocessor to auto-detect task list sections and inject lvt blocks + auto-generated markdown sources.
• Update watcher logic to refresh sources (instead of full reload) for same-file sources and add WebSocket write serialization.
• Add auto-tasks example plus unit/E2E coverage; update Docker Chrome E2E setup for macOS.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
page.go	Runs auto-task preprocessing before markdown parsing and injects auto-generated sources into the page config.
auto_tasks.go	Implements task-section detection, preprocessing into lvt blocks, and heading slugification.
auto_tasks_test.go	Adds unit tests for detection, preprocessing, slugification, and block generation.
auto_tasks_e2e_test.go	Adds E2E tests validating toggle/add/persist and “no full reload” behavior.
internal/server/websocket.go	Adds writeMu to serialize WebSocket writes and exposes TracksSourceFile for watcher decisions.
internal/server/server.go	Adjusts watcher behavior to refresh sources for same-file sources and adds tracked-source-file detection.
e2e_helpers_test.go	Switches Docker Chrome networking strategy on macOS to port mapping and updates URL conversion.
e2e_helpers_external_test.go	Same Docker Chrome macOS changes for external tests package.
examples/auto-tasks/index.md	Adds a simple example page demonstrating zero-config task lists.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Even with writeMu in WebSocketHandler, there’s still an un-serialized WebSocket write path: Server.BroadcastReload calls conn.WriteMessage directly (outside the handler), which can still race with handler.sendMessage writes and trigger gorilla/websocket concurrent write panics. Consider ensuring reload broadcasts also go through the same per-connection write serialization mechanism.

[Copilot]

In EnableWatch, treating any page file that is also tracked as a source as “refresh sources only” means normal edits to that markdown page (adding/removing headings/blocks, changing non-task content) will no longer trigger Discover()+BroadcastReload, so the served page can get stuck showing stale HTML. Consider narrowing this optimization to writes that originate from the auto-task persistence path (e.g., track recently-written files/timestamps) or otherwise still perform a full reload when the page structure/content might have changed.

Suggested change

	if isPageFile && isSourceFile {
	// Same-file source (e.g., auto-tasks): refresh sources only, no full reload.
	// This prevents a full page reload on every checkbox toggle.
	s.RefreshSourcesForFile(filePath)
	} else if isPageFile {
	if isPageFile {
	// For page files, always re-discover and reload so that structural/content
	// changes are reflected in the served HTML.
	// If the page file also contains tracked sources (e.g., auto-tasks),
	// refresh those sources first so clients see the latest task state.
	if isSourceFile {
	s.RefreshSourcesForFile(filePath)
	}

[Copilot]

slugifyHeading can return an empty string for headings that contain no [a-z0-9] characters, which would produce sourceName "auto" and Anchor "#". That can cause collisions between sections and makes anchor matching ambiguous. Consider validating sec.anchor and either skipping such sections or falling back to a deterministic non-empty name (e.g., based on heading hash/line).

[Copilot]

This regexp is compiled on every heading line encountered. Since detectTaskListSections can run on every page parse, it would be better to precompile the explicit anchor regexp once (similar to the other patterns) to avoid repeated compilation overhead.

[Copilot]

The Docker networking workaround is applied only for darwin. Docker Desktop on Windows also doesn’t support --network host, and Chrome-in-Docker typically needs host.docker.internal + -p port mapping there as well. Consider extending the OS check to treat all non-Linux platforms similarly (or explicitly handle windows) to keep E2E helpers cross-platform.

[Copilot]

Same as e2e_helpers_test.go: limiting the Docker networking behavior to darwin will likely break E2E runs on Windows (no --network host, and host.docker.internal is typically required). Consider handling windows (or all non-Linux OSes) the same way as macOS.

Suggested change

	if runtime.GOOS == "darwin" {
	// macOS: use port mapping, don't override the debugging port
	cmd = exec.Command("docker", "run", "-d",
	"--rm",
	"-p", fmt.Sprintf("%d:9222", debugPort),
	"--memory", "512m",
	"--cpus", "0.5",
	"--name", containerName,
	dockerImage,
	)
	} else {
	// Linux: use --network host for reliable localhost access
	cmd = exec.Command("docker", "run", "-d",
	"--rm",
	"--network", "host",
	"--memory", "512m",
	"--cpus", "0.5",
	"--name", containerName,
	dockerImage,
	"--remote-debugging-port="+fmt.Sprintf("%d", debugPort),
	if runtime.GOOS == "linux" {
	// Linux: use --network host for reliable localhost access
	cmd = exec.Command("docker", "run", "-d",
	"--rm",
	"--network", "host",
	"--memory", "512m",
	"--cpus", "0.5",
	"--name", containerName,
	dockerImage,
	"--remote-debugging-port="+fmt.Sprintf("%d", debugPort),
	)
	} else {
	// Non-Linux (e.g., macOS, Windows): use port mapping, don't override the debugging port
	cmd = exec.Command("docker", "run", "-d",
	"--rm",
	"-p", fmt.Sprintf("%d:9222", debugPort),
	"--memory", "512m",
	"--cpus", "0.5",
	"--name", containerName,
	dockerImage,

[claude]

Pull Request Review: Auto-Interactive Markdown Checkboxes

Summary

This PR implements a clean zero-configuration feature that automatically transforms markdown task lists into interactive checkboxes. The implementation is well-architected, making clever use of in-memory preprocessing to reuse the existing LiveTemplate pipeline without modifying files on disk.

Code Quality & Architecture ✅

Strengths:

• Excellent separation of concerns: The preprocessing layer (preprocessAutoTasks) cleanly separates detection from rendering
• Reuses existing infrastructure: Leverages the entire MarkdownSource pipeline instead of reinventing it
• Clear naming conventions: _auto_* prefix for auto-generated sources prevents naming conflicts
• Well-documented: Functions have clear docstrings explaining their purpose

Suggestions:

1. auto_tasks.go:37-38 - The early return optimization could miss edge cases:

if !bytes.Contains(content, []byte("- [ ]")) && !bytes.Contains(content, []byte("- [x]")) && !bytes.Contains(content, []byte("- [X]")) {

This won't detect tasks with extra whitespace like - [ ] or - [ ]. Consider using the regex pattern for this check instead, or document this limitation.

2. auto_tasks.go:77-80 - Inline regex compilation in hot path:

explicitAnchorRe := regexp.MustCompile(`\s*\{#([^}]+)\}\s*$`)

This regex is compiled on every heading encountered. Move to package-level var with other pre-compiled patterns for better performance.

3. auto_tasks.go:205-212 - Slugification divergence risk:

// This must match the slugify() function in internal/source/markdown.go for anchor matching.
func slugifyHeading(text string) string {

The comment warns about keeping this in sync, but there's no enforcement. Consider:

• Extracting to a shared package
• Adding a test that compares both implementations
• Using the same function via import

Potential Bugs 🐛

Critical Issues:

1. Race condition in line index calculation (auto_tasks.go:142-170)

The code processes sections in reverse order but the line adjustment logic is complex:

for i := len(sections) - 1; i >= 0; i-- {
    sec := sections[i]
    // ... build replacement ...
    startIdx := sec.startLine - 1 + fmLineCount
    endIdx := sec.endLine + fmLineCount

Issue: When multiple sections exist, modifying one section changes line numbers for others. Even processing in reverse, the startLine and endLine are calculated from the original content, but you're progressively modifying the lines slice.

Test case that might fail: A file with 3+ task sections where the first section is very long. After processing the third section, the line indices for the second section may be off.

Recommendation: Either:

• Recalculate indices after each modification
• Build an entirely new line array without in-place modification
• Add a test with 3+ sections of varying sizes

2. Frontmatter detection (auto_tasks.go:122-124)

if loc := frontmatterPattern.FindIndex(content); loc != nil {
    bodyOffset = loc[1]
}

The frontmatterPattern uses \A---\n which requires exactly one newline. Files with \r\n line endings (Windows) or ---\r\n won't match, causing the frontmatter to be processed as content.

Fix: Use \A---\r?\n(.+?)\r?\n---\r?\n to handle both line ending styles.

3. Missing validation in preprocessAutoTasks (auto_tasks.go:142-149)

There's no check if a source name already exists before creating _auto_* sources. While the comment says "checked later during injection in ParseFile", if two sections have the same slugified anchor, the second will silently overwrite the first in the sources map.

Example:

## Tasks
- [ ] Item 1

## TASKS
- [ ] Item 2

Both slug to "tasks", so only one _auto_tasks source exists.

Fix: Detect collisions and either skip, append a number suffix, or log a warning.

Minor Issues:

4. No error handling for malformed headings (auto_tasks.go:74-88)

The explicit anchor regex could match malformed syntax like {#} or {#with spaces}, creating invalid anchors.

5. Template injection risk (auto_tasks.go:186-202)

The sourceName is directly interpolated into HTML without escaping:

return fmt.Sprintf(`<div lvt-source="%s">`, sourceName)

While sourceName is controlled (always "_auto_" + sec.anchor), if sec.anchor contains quotes or special chars, it could break the HTML. Use html.EscapeString() or template the output properly.

Performance Considerations ⚡

Good:

• Early return optimization (line 37-39) prevents unnecessary processing
• Reverse iteration preserves line numbers efficiently
• Pre-compiled regex patterns (mostly)

Concerns:

1. Regex compilation in loop (mentioned above) - line 77
2. String splitting/joining (auto_tasks.go:41, 182) - For large files, consider working with byte slices or a buffer
3. O(n²) potential in section detection - The flushSection function is called for every heading, but the complexity is acceptable for typical markdown files

Security Concerns 🔒

Medium Risk:

1. Arbitrary file path in SourceConfig (auto_tasks.go:176)

File: absPath,

While absPath comes from ParseFile, there's no validation that the file is within the project directory. If ParseFile can be called with paths outside the workspace (via symlinks, .., etc.), this could expose files.

Recommendation: Add path validation to ensure files are within the allowed directory tree.

Low Risk:

2. No input sanitization on anchor IDs

User-controlled heading text becomes anchor IDs. While slugification removes most dangerous chars, Unicode or very long headings could cause issues downstream.

Test Coverage ✅

Excellent coverage overall!

Unit Tests (auto_tasks_test.go):

• ✅ Detection logic (basic, multiple, mixed content, no heading)
• ✅ Preprocessing (with/without frontmatter, multiple sections)
• ✅ Slugification edge cases
• ✅ Template generation

E2E Tests (auto_tasks_e2e_test.go):

• ✅ Basic toggle functionality
• ✅ Add task functionality
• ✅ Multiple independent sections
• ✅ Persistence across reloads
• ✅ No full reload with file watcher (excellent!)

Missing test cases:

1. Edge case: Empty task text

   - [ ]

2. Edge case: Very long heading names (>200 chars)

3. Edge case: Duplicate slugified anchors (mentioned above)

4. Edge case: Task list at end of file (no trailing newline)

5. Integration: Interaction between auto-sources and manually configured sources with same anchor

6. Cross-platform: Windows line endings (\r\n)

WebSocket Concurrency Fix 🎯

Excellent fix! (websocket.go:142, 1158-1161)

Adding writeMu to serialize WebSocket writes is exactly right. This prevents the panic when:

1. File watcher detects change → sends refresh
2. Action handler completes → sends response
3. Both try to write simultaneously

Suggestion: Consider using a buffered channel-based write queue instead of a mutex if you expect high-frequency concurrent writes. The current mutex approach is simpler and sufficient for now.

Watcher Same-File Source Detection 🎯

Smart optimization! (server.go:2405-2429, 2418-2429)

The isTrackedSourceFile check prevents full page reloads when the page file is also the data source. This is crucial for the auto-tasks UX.

Minor issue: The O(n×m) complexity (connections × sources per connection) could be optimized with a reverse index (map[filePath][]connectionID), but unlikely to matter at reasonable scale.

Breaking Changes / API Impact

None - This is purely additive. Existing markdown files work unchanged, and the feature activates only when task lists are detected.

Documentation

The PR description is excellent and comprehensive. Consider adding:

• User-facing documentation on how to opt-out if needed
• Note about the heading requirement (tasks without headings are ignored)
• Mention of the {#custom-anchor} syntax support

Recommendations Summary

Must Fix (before merge):

1. ✅ Fix frontmatter pattern for Windows line endings
2. ✅ Add collision detection for duplicate anchor slugs
3. ⚠️ Verify line index calculation with 3+ sections (add test)

Should Fix (before merge):
4. Move explicitAnchorRe to package level
5. Escape sourceName in template generation
6. Add test for Windows line endings

Nice to Have:
7. Share slugifyHeading with internal/source/markdown.go
8. Add missing edge case tests
9. Document the whitespace requirements for task detection

Overall Assessment

Recommendation: Approve with minor fixes ✅

This is a well-designed feature with excellent test coverage and thoughtful implementation. The preprocessing approach is elegant, and the watcher optimization shows deep understanding of the system. The issues identified are mostly edge cases and minor improvements, not fundamental flaws.

The code quality is high, the tests are comprehensive, and the feature delivers real value with zero configuration. Great work!

---

Review Score:

• Code Quality: 8.5/10
• Test Coverage: 9/10
• Documentation: 8/10
• Architecture: 9/10
• Overall: 8.5/10 - Excellent PR, ready after addressing the frontmatter and collision detection issues.

🤖 Generated with Claude Code

[claude]

Pull Request Review: Auto-Interactive Markdown Checkboxes

Great work on this feature! The implementation is clean, well-tested, and follows good architectural patterns.

✅ Strengths

Code Quality & Architecture

• Clean separation of concerns: The preprocessing approach (auto_tasks.go:46-108) is elegant - transforming markdown in-memory without modifying source files
• Excellent test coverage: 14 unit tests + 5 E2E tests with comprehensive scenarios
• Deterministic slugification: slugifyHeading() (auto_tasks.go:204-209) properly matches internal markdown.go implementation
• Zero-config design: Requiring only headings + task lists is excellent UX

Concurrency & Thread Safety

• Critical fix: The writeMu mutex (websocket.go:104) properly serializes WebSocket writes, preventing race conditions between watcher refresh and action responses
• Smart watcher optimization: isRecentSourceWrite() mechanism distinguishes user checkbox toggles from external file edits, avoiding unnecessary full-page reloads

Implementation Details

• Proper pattern matching: Pre-compiled regexes (auto_tasks.go:20-25) for performance
• Mixed content detection: Correctly skips sections with both tasks and prose
• Explicit anchor support: Handles {#custom-anchor} syntax (auto_tasks.go:72-77)
• E2E testing with Docker Chrome: Proper containerized browser testing

---

🔍 Issues & Concerns

1. Potential Race Condition in isRecentSourceWrite() ⚠️
Location: internal/server/server.go:2519-2526

The entry is deleted on first read. If the watcher fires twice quickly for the same file (which can happen with some filesystems), the second check will return false and trigger a full reload.

Suggested fix: Keep entry until timeout to allow multiple watcher events, and add cleanup goroutine to prevent unbounded growth.

2. Missing Validation for Empty Anchors
Location: auto_tasks.go:139-142

Headings like ## !!! or ## 123 will have empty anchors after slugification and be silently ignored. Consider: fall back to generated anchor, log warning, or add test coverage.

3. Hardcoded Timeout Value
Location: internal/server/server.go:2526

Extract 2*time.Second to a constant like sourceWriteDebounceWindow.

4. BroadcastReload Race Condition
Location: internal/server/server.go:2419-2432

The else branch doesn't use the mutex when handler is nil, which could still cause concurrent write issues.

---

🔧 Minor Issues

5. Magic string prefix (auto_tasks.go:144): Extract "auto" to a constant
6. Template string (auto_tasks.go:184-199): Could be externalized for easier maintenance

---

🚀 Performance & Security

✅ Early return optimization (auto_tasks.go:31-34) is excellent - avoids regex for files without tasks
✅ No XSS vulnerabilities detected - template properly uses Go's HTML template escaping
✅ No path traversal issues - absPath handling is safe

---

🎯 Recommendations

HIGH PRIORITY:

1. Fix isRecentSourceWrite() deletion timing to handle multiple watcher events
2. Add cleanup goroutine for recentSourceWrites map to prevent memory leak
3. Protect the handler == nil write path in BroadcastReload()

LOW PRIORITY:
4. Extract magic constants and template strings
5. Add concurrent modification test

---

📊 Overall Assessment

Rating: 8.5/10

This is a solid, well-architected feature that demonstrates good understanding of Go concurrency patterns and careful attention to edge cases. The preprocessing approach is elegant, and the test coverage is comprehensive.

The main concerns are around the potential race condition in the debounce logic and ensuring the WebSocket write serialization is complete. The feature delivers excellent user value with its zero-config approach, and the implementation integrates smoothly with the existing pipeline.

Recommendation: Approve with minor revisions to address the race condition issues.

Great work! 🎉

[claude]

PR Review: Auto-Interactive Markdown Checkboxes

Summary

This is a well-designed feature that brings zero-config interactive checkboxes to markdown task lists. The implementation is clean, well-tested, and addresses important edge cases.

Strengths

1. Excellent Architecture

• Clean separation of concerns with detection, preprocessing, and rendering phases
• In-memory transformation preserves original files - very elegant approach
• Reuses existing MarkdownSource pipeline instead of reinventing it
• Pre-compiled regex patterns for performance (auto_tasks.go:26-31)

2. Comprehensive Test Coverage

• 14 unit tests covering detection, preprocessing, slugification, and template generation
• 5 E2E browser tests with Docker Chrome validating real-world scenarios
• Tests cover edge cases: mixed content, no headings, explicit anchors, multiple sections
• E2E tests verify critical behaviors: persistence, no-reload on toggle, multiple sections

3. Important Bug Fixes

• Fixes concurrent WebSocket write panic with writeMu serialization (websocket.go:1274)
• Fixes watcher to detect same-file sources and avoid unnecessary full-page reloads (server.go:2203-2213)
• 2-second debounce window to distinguish source writes from external edits

4. Thoughtful Design Decisions

• Skips sections with mixed content (prose + tasks) to avoid breaking existing pages
• Requires headings for task sections - prevents ambiguous behavior
• Supports explicit anchor syntax {#custom-anchor}
• GitHub-style slugification matches existing markdown anchor behavior

Areas for Improvement

1. XSS Security (Medium Priority)

Location: auto_tasks.go:190-205 in generateAutoTaskLvtBlock()

The task text is rendered with {{.Text}} which is HTML-escaped by default in Go templates, so this is SAFE. The conditional style is server-controlled, not user-controlled.

Recommendation: Add a comment documenting this security consideration for future maintainers.

2. Race Condition in Source Write Tracking (Low Severity)

Location: server.go:2251-2262 in isRecentSourceWrite()

The debounce cleanup happens inside the check, which could be more robust with proper defer usage.

3. Performance - Early Exit Check (Minor)

Location: auto_tasks.go:38-40

The 3 separate Contains calls could be optimized, though this is minor since it runs once per page load.

4. Code Quality Nitpicks

• Magic value at auto_tasks.go:158 needs comment explaining blank line spacing
• Field recentSourceWrites could be renamed to sourceWriteTimestamps for clarity
• preprocessAutoTasks could return error for defensive programming

5. Test Coverage Gaps (Low Priority)

• Test for toggling checkbox then externally editing same file
• Test for special characters in headings
• Test for Unicode task text
• XSS test case to verify template escaping

Security Assessment

Overall: SECURE

• No SQL injection risk
• No command injection
• No path traversal issues
• XSS protection via Go template auto-escaping
• No unsafe reflection or eval
• Proper mutex protection for concurrent access

Recommendation: Add integration test verifying XSS protection.

Performance Assessment

Overall: EFFICIENT

• Early exit optimization
• Pre-compiled regex patterns
• Reverse processing preserves line numbers
• Single-pass detection algorithm
• Debounce prevents watcher spam

Code Quality

Overall: EXCELLENT

• Clear, descriptive function names
• Comprehensive comments explaining WHY, not just WHAT
• Consistent error handling
• Thread-safe with proper mutex usage
• Well-structured with single-responsibility functions

Recommendations

High Priority

1. Add XSS test case to verify template escaping works correctly
2. Fix the race condition in isRecentSourceWrite() with defer

Medium Priority

3. Add comment explaining XSS safety in generateAutoTaskLvtBlock()
4. Add test for external file edit after source write
5. Consider returning error from preprocessAutoTasks() for future-proofing

Low Priority

6. Add comments for magic values
7. Consider renaming recentSourceWrites to sourceWriteTimestamps
8. Add Unicode test case

Conclusion

This is high-quality work that adds significant value with minimal risk. The implementation is well-thought-out, properly tested, and follows Go best practices. The bug fixes (WebSocket race, watcher optimization) are valuable improvements on their own.

Recommendation: APPROVE with minor suggestions for hardening.

The zero-config approach is exactly what users want - markdown checkboxes just work without frontmatter setup.

---

Review generated with Claude Code