Add table extension to mdConverter goldmark renderer

internal/richtext/richtext.go

@@ -20,6 +20,7 @@ import (
 	"github.com/yuin/goldmark"
 	"github.com/yuin/goldmark/ast"
 	"github.com/yuin/goldmark/extension"
+	east "github.com/yuin/goldmark/extension/ast"
 	"github.com/yuin/goldmark/parser"
 	"github.com/yuin/goldmark/renderer"
 	gmhtml "github.com/yuin/goldmark/renderer/html"
@@ -110,7 +111,7 @@ var reSGIDMention = regexp.MustCompile(`(^|[\s>(\["'])(@sgid:([\w+=/-]+))`)
 
 // Pre-compiled regexes for IsHTML detection
 var (
-	reSafeTag     = regexp.MustCompile(`<(p|div|span|a|strong|b|em|i|code|pre|ul|ol|li|h[1-6]|blockquote|br|hr|img|bc-attachment)\b[^>]*>`)
+	reSafeTag     = regexp.MustCompile(`<(p|div|span|a|strong|b|em|i|code|pre|ul|ol|li|h[1-6]|blockquote|br|hr|img|table|bc-attachment)\b[^>]*>`)
 	reFencedBlock = regexp.MustCompile("(?m)^```[^\n]*\n[\\s\\S]*?^```")
 )
 
@@ -128,7 +129,16 @@ var reMarkdownPatterns = []*regexp.Regexp{
 
 // mdConverter is the goldmark Markdown-to-HTML converter configured for Trix compatibility.
 var mdConverter = goldmark.New(
-	goldmark.WithExtensions(extension.Strikethrough),
+	goldmark.WithExtensions(
+		extension.Strikethrough,
+		// Emit the whitelisted `align` attribute rather than a `style` attribute:
+		// BC3's sanitizer keeps `align` but strips inline `style` (only color and
+		// background-color survive), so GFM column alignment is preserved through
+		// sanitization. Bare <table> is correct — BC3's WrapTablesFilter wraps it.
+		extension.NewTable(
+			extension.WithTableCellAlignMethod(extension.TableCellAlignAttribute),
+		),
+	),
 	goldmark.WithRendererOptions(gmhtml.WithUnsafe()),
 	goldmark.WithParserOptions(
 		parser.WithInlineParsers(
@@ -923,6 +933,36 @@ func unescapeHTML(s string) string {
 	return s
 }
 
+// tableDetectParser parses input with the GFM table extension so table
+// detection matches what MarkdownToHTML actually renders. Using the parser
+// (rather than a regex) authoritatively handles single-column tables, CRLF
+// line endings, and other pipe-row edge cases without pattern drift.
+var tableDetectParser = goldmark.New(goldmark.WithExtensions(extension.Table)).Parser()
+
+// hasMarkdownTable parses s and reports whether it contains a GFM table node.
+func hasMarkdownTable(s string) bool {
+	// Cheap-out before the full goldmark parse: a GFM table needs cell-delimiting
+	// pipes and a delimiter row on its own line, so it can't exist without both a
+	// '|' and a '\n'. IsMarkdown calls this on every input that matches none of the
+	// regex patterns — typically plain text on TUI submit/editor return — so
+	// skipping the parse in that common case avoids the cost. This matches
+	// goldmark's own behavior exactly (it likewise treats a CR-only "table" with
+	// no '\n' as not a table), so the guard introduces no false negatives.
+	if !strings.Contains(s, "|") || !strings.Contains(s, "\n") {
+		return false
+	}
+	doc := tableDetectParser.Parse(text.NewReader([]byte(s)))
+	found := false
+	_ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
+		if entering && n.Kind() == east.KindTable {
+			found = true
+			return ast.WalkStop, nil
+		}
+		return ast.WalkContinue, nil
+	})
+	return found
+}
+
 // IsMarkdown attempts to detect if the input string is Markdown rather than plain text or HTML.
 // This is a heuristic and may not be 100% accurate.
 func IsMarkdown(s string) bool {
@@ -936,7 +976,7 @@ func IsMarkdown(s string) bool {
 		}
 	}
 
-	return false
+	return hasMarkdownTable(s)
 }
 
 // AttachmentRef holds the metadata needed to embed a <bc-attachment> in HTML.
@@ -1362,6 +1402,20 @@ func IsHTML(s string) bool {
 	return false
 }
 
+// reTableHTML matches a real <table> tag — with attributes (`<table …>`), bare
+// (`<table>`), or self-closing (`<table/>`) — distinct from the Markdown table
+// detector. The trailing class requires a boundary after the name so longer
+// tags like <tablefoo> don't match. Used to gate the fail-closed TUI edit paths.
+var reTableHTML = regexp.MustCompile(`(?i)<table[\s/>]`)
+
+// HasTableHTML reports whether s contains an HTML table element. The TUI in-place
+// editors use this to refuse table-bearing content: HTMLToMarkdown has no table
+// handling and would strip the structure, so those edits fail closed rather than
+// silently flatten the table on resubmit.
+func HasTableHTML(s string) bool {
+	return reTableHTML.MatchString(s)
+}
+
 func isEscapedAt(s string, pos int) bool {
 	backslashes := 0
 	for i := pos - 1; i >= 0 && s[i] == '\\'; i-- {

internal/richtext/richtext_test.go

@@ -216,6 +216,14 @@ func TestMarkdownToHTML(t *testing.T) {
 			input:    "intro\n\n```\n<div>hello</div>\n```",
 			expected: "<p>intro</p>\n<br>\n<pre><code>&lt;div&gt;hello&lt;/div&gt;\n</code></pre>",
 		},
+		{
+			// Issue #405: a GFM table renders as a bare <table> (BC3's
+			// WrapTablesFilter supplies the figure wrapper). Doubles as the
+			// heading+table regression.
+			name:     "gfm table with heading (issue #405)",
+			input:    "# Report\n\n| Foo | Bar |\n| --- | --- |\n| Baz | Qux |",
+			expected: "<h1>Report</h1>\n<table>\n<thead>\n<tr>\n<th>Foo</th>\n<th>Bar</th>\n</tr>\n</thead>\n<tbody>\n<tr>\n<td>Baz</td>\n<td>Qux</td>\n</tr>\n</tbody>\n</table>",
+		},
 	}
 
 	for _, tt := range tests {
@@ -228,6 +236,21 @@ func TestMarkdownToHTML(t *testing.T) {
 	}
 }
 
+func TestMarkdownToHTMLTableAlignment(t *testing.T) {
+	// Column alignment must survive BC3's sanitizer, which whitelists the `align`
+	// attribute but strips inline `style` (only color/background-color survive).
+	got := MarkdownToHTML("| a | b |\n|:--|--:|\n| 1 | 2 |")
+	if !strings.Contains(got, `align="left"`) {
+		t.Errorf("expected align=\"left\" in %q", got)
+	}
+	if !strings.Contains(got, `align="right"`) {
+		t.Errorf("expected align=\"right\" in %q", got)
+	}
+	if strings.Contains(got, "style=") {
+		t.Errorf("expected no style= (stripped by BC3 sanitizer) in %q", got)
+	}
+}
+
 func TestMarkdownToHTMLBackslashEscapes(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -1159,6 +1182,31 @@ func TestIsMarkdown(t *testing.T) {
 			input:    "> Quote",
 			expected: true,
 		},
+		{
+			name:     "multi-column table",
+			input:    "| Foo | Bar |\n| --- | --- |\n| Baz | Qux |",
+			expected: true,
+		},
+		{
+			name:     "single-column table",
+			input:    "| value |\n| --- |\n| item |",
+			expected: true,
+		},
+		{
+			name:     "table with CRLF line endings",
+			input:    "| Foo | Bar |\r\n| --- | --- |\r\n| Baz | Qux |",
+			expected: true,
+		},
+		{
+			name:     "lone delimiter line is not a table",
+			input:    "---|---",
+			expected: false,
+		},
+		{
+			name:     "prose containing pipes is not a table",
+			input:    "run foo | grep bar to filter the output",
+			expected: false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -1262,6 +1310,14 @@ func TestIsHTML(t *testing.T) {
 			input:    `\<bc-attachment sgid="x">\</bc-attachment>`,
 			expected: false,
 		},
+		{
+			// A plain BC3 table response carries no other formatting tags, so
+			// IsHTML must recognize <table> itself, or the raw markup would leak
+			// to the reader instead of being converted for display.
+			name:     "bc3-wrapped table",
+			input:    "<figure><table><thead><tr><th>Foo</th></tr></thead><tbody><tr><td>Baz</td></tr></tbody></table></figure>",
+			expected: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -1274,6 +1330,30 @@ func TestIsHTML(t *testing.T) {
 	}
 }
 
+func TestHasTableHTML(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected bool
+	}{
+		{name: "lowercase table tag", input: "<table><tr><td>x</td></tr></table>", expected: true},
+		{name: "uppercase table tag with attrs", input: `<TABLE class="x"><tr></tr></TABLE>`, expected: true},
+		{name: "self-closing table tag", input: "<table/>", expected: true},
+		{name: "wrapped table", input: "<figure><table></table></figure>", expected: true},
+		{name: "plain text", input: "just some text", expected: false},
+		{name: "pipe markdown", input: "| a | b |\n| --- | --- |\n| 1 | 2 |", expected: false},
+		{name: "word starting with table", input: "the tablet is here", expected: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := HasTableHTML(tt.input); got != tt.expected {
+				t.Errorf("HasTableHTML(%q) = %v, want %v", tt.input, got, tt.expected)
+			}
+		})
+	}
+}
+
 func TestRoundTrip(t *testing.T) {
 	// Test that converting Markdown -> HTML -> Markdown preserves meaning
 	tests := []struct {

internal/tui/workspace/views/detail.go

@@ -757,6 +757,10 @@ func (v *Detail) startCommentEdit() tea.Cmd {
 		return nil
 	}
 	c := v.data.comments[v.focusedComment]
+	// Fail closed on table-bearing content (see startEditBody).
+	if richtext.HasTableHTML(c.content) {
+		return workspace.SetStatus("This comment contains a table — edit it on Basecamp web", true)
+	}
 	v.editingComment = true
 	v.commentEditComposer = widget.NewComposer(v.styles,
 		widget.WithMode(widget.ComposerRich),
@@ -1074,6 +1078,11 @@ func (v *Detail) startEditBody() tea.Cmd {
 	if v.data == nil {
 		return nil
 	}
+	// Fail closed on table-bearing content: HTMLToMarkdown has no table handling,
+	// so entering edit mode and resubmitting would strip the table. Block the edit.
+	if richtext.HasTableHTML(v.data.content) {
+		return workspace.SetStatus("This message contains a table — edit it on Basecamp web", true)
+	}
 	v.editingBody = true
 	v.bodyEditComposer = widget.NewComposer(v.styles,
 		widget.WithMode(widget.ComposerRich),

internal/tui/workspace/views/detail_test.go

@@ -573,6 +573,50 @@ func TestDetail_CommentEdit_Ignored_WhenNoFocus(t *testing.T) {
 	assert.False(t, v.editingComment)
 }
 
+const tableHTML = "<figure><table><thead><tr><th>Foo</th></tr></thead>" +
+	"<tbody><tr><td>Baz</td></tr></tbody></table></figure>"
+
+func TestDetail_EditBody_BlockedForTable(t *testing.T) {
+	v := testDetailWithSession("Message", false)
+	v.data.content = tableHTML
+
+	cmd := v.startEditBody()
+	assert.False(t, v.editingBody, "must not enter edit mode on table content")
+	assert.Nil(t, v.bodyEditComposer, "composer must not be built")
+
+	require.NotNil(t, cmd, "should return a status command")
+	status, ok := cmd().(workspace.StatusMsg)
+	require.True(t, ok, "cmd should produce a StatusMsg")
+	assert.Contains(t, status.Text, "table")
+	assert.True(t, status.IsError)
+}
+
+func TestDetail_EditBody_EntersForNonTable(t *testing.T) {
+	v := testDetailWithSession("Message", false)
+	v.data.content = "<p>plain body</p>"
+
+	cmd := v.startEditBody()
+	assert.True(t, v.editingBody, "should enter edit mode on table-free content")
+	require.NotNil(t, v.bodyEditComposer, "composer should be built")
+	assert.NotNil(t, cmd)
+}
+
+func TestDetail_CommentEdit_BlockedForTable(t *testing.T) {
+	v := detailWithComments()
+	v.focusedComment = 0
+	v.data.comments[0].content = tableHTML
+
+	cmd := v.startCommentEdit()
+	assert.False(t, v.editingComment, "must not enter edit mode on table content")
+	assert.Nil(t, v.commentEditComposer, "composer must not be built")
+
+	require.NotNil(t, cmd, "should return a status command")
+	status, ok := cmd().(workspace.StatusMsg)
+	require.True(t, ok, "cmd should produce a StatusMsg")
+	assert.Contains(t, status.Text, "table")
+	assert.True(t, status.IsError)
+}
+
 func TestDetail_CommentTrash_DoublePress(t *testing.T) {
 	v := detailWithComments()
 	v.focusedComment = 1

internal/tui/workspace/views/todos.go

@@ -1042,6 +1042,12 @@ func (v *Todos) startEditDescription() tea.Cmd {
 		}
 	}
 
+	// Fail closed on table-bearing content: HTMLToMarkdown has no table handling,
+	// so entering edit mode and resubmitting would strip the table. Block the edit.
+	if richtext.HasTableHTML(description) {
+		return workspace.SetStatus("This to-do description contains a table — edit it on Basecamp web", true)
+	}
+
 	v.editingDesc = true
 	v.descTodoID = todoID
 	v.descComposer.Reset()

internal/tui/workspace/views/todos_test.go

@@ -942,6 +942,41 @@ func TestTodos_BoostTarget_IncludesAccountID(t *testing.T) {
 	assert.Equal(t, int64(42), picker.Target.ProjectID)
 }
 
+// --- Edit description: table fail-closed guard ---
+
+const todoTableHTML = "<figure><table><thead><tr><th>Foo</th></tr></thead>" +
+	"<tbody><tr><td>Baz</td></tr></tbody></table></figure>"
+
+func TestTodos_EditDescription_BlockedForTable(t *testing.T) {
+	v := testTodosViewWithTodos()
+
+	todos := sampleTodos()
+	todos[0].Description = todoTableHTML
+	v.session.Hub().Todos(42, 10).Set(todos)
+
+	cmd := v.startEditDescription()
+	assert.False(t, v.editingDesc, "must not enter edit mode on table content")
+
+	require.NotNil(t, cmd, "should return a status command")
+	status, ok := cmd().(workspace.StatusMsg)
+	require.True(t, ok, "cmd should produce a StatusMsg")
+	assert.Contains(t, status.Text, "table")
+	assert.True(t, status.IsError)
+}
+
+func TestTodos_EditDescription_EntersForNonTable(t *testing.T) {
+	v := testTodosViewWithTodos()
+	v.descComposer = widget.NewComposer(v.styles, widget.WithMode(widget.ComposerRich))
+
+	todos := sampleTodos()
+	todos[0].Description = "<p>plain description</p>"
+	v.session.Hub().Todos(42, 10).Set(todos)
+
+	cmd := v.startEditDescription()
+	assert.True(t, v.editingDesc, "should enter edit mode on table-free content")
+	assert.NotNil(t, cmd)
+}
+
 // newTextInputWithValue creates a textinput with a preset value for testing.
 func newTextInputWithValue(val string) textinput.Model {
 	ti := textinput.New()

internal/tui/workspace/widget/composer_test.go

@@ -212,6 +212,19 @@ func TestComposerSubmitRich(t *testing.T) {
 	}
 }
 
+func TestComposerSubmitTableIsRich(t *testing.T) {
+	// A table-only submission has no other Markdown formatting; without AST-based
+	// table detection it would be misclassified as plain and sent as raw pipes.
+	c := NewComposer(testStyles(), WithMode(ComposerRich))
+	c.SetValue("| Foo | Bar |\n| --- | --- |\n| Baz | Qux |")
+	cmd := c.Submit()
+	msg := cmd()
+	submitMsg := msg.(ComposerSubmitMsg)
+	if submitMsg.Content.IsPlain {
+		t.Error("table-only content should not be plain text")
+	}
+}
+
 func TestComposerSubmitEmpty(t *testing.T) {
 	c := NewComposer(testStyles())
 	cmd := c.Submit()

skills/basecamp/SKILL.md

@@ -85,13 +85,20 @@ Full CLI coverage: 155 endpoints across todos, cards, messages, files, schedule,
 2. **Parse URLs first** with `basecamp url parse "<url>"` to extract IDs
 3. **Comments are flat** - reply to parent recording, not to comments
 4. **Check context** via `.basecamp/config.json` before assuming project
-5. **Content fields accept Markdown and @mentions** — message body and comment content accept Markdown syntax; the CLI converts to HTML automatically. Use Markdown formatting (lists, bold, links, code blocks) for rich content. Four mention syntaxes are available (prefer deterministic for agents):
+5. **Content fields accept Markdown and @mentions** — message body and comment content accept Markdown syntax; the CLI converts to HTML automatically. Use Markdown formatting (lists, bold, links, code blocks, tables) for rich content. Four mention syntaxes are available (prefer deterministic for agents):
    - **`[@Name](mention:SGID)`** — zero API calls, embeds SGID directly (preferred for agents)
    - **`[@Name](person:ID)`** — one API call, resolves person ID to SGID via pingable set
    - **`@sgid:VALUE`** — inline SGID embed for pipeline composability
    - **`@Name` / `@First.Last`** — fuzzy name resolution (may be ambiguous)
    For todos, documents, and cards, content is sent as-is — use plain text or HTML directly.
 
+   **Table boundary:** GFM tables render in message/comment bodies, but the TUI
+   in-place editors **refuse to open** table-bearing content (edit it on Basecamp
+   web, or replace the whole field via `messages update` / `comments update` /
+   `todos update --description`, which take fresh content and are unaffected), and
+   human-readable CLI/TUI **display** of such content may lose table structure —
+   both pending server-side Markdown support (BC3 #11986).
+
    **Multiline / non-ASCII content:** do not rely on bash ANSI-C quoting (`$'...\n...'`) — it is a bash/zsh extension. Under a POSIX `/bin/sh` (dash, busybox-ash, common in sandboxes) the `$` is passed through literally and posts a stray leading `$`, and `\n` stays a literal backslash-n. Pipe the content via stdin instead, using `-` as the content argument:
    ```bash
    printf '%s\n' '海报 mockup 方向稿：' '' '<bc-attachment ...>' | basecamp comments create <recording_id> - --in <project> --json