Say that every rich-text field is Markdown-converted, todos and cards included - #692
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is scoped to documentation and aligns the skill guidance with existing CLI behavior, with only a minor formatting clarification suggested.
Pull request overview
Updates the Basecamp agent skill documentation to correctly describe how the CLI handles rich-text inputs, clarifying that rich-text fields are Markdown-converted to HTML and documenting the “raw HTML is all-or-nothing” behavior (including inline image implications), while noting that titles remain plain text.
Changes:
- Corrects the guidance around todo/card/document content by listing all rich-text fields that go through Markdown → HTML conversion.
- Documents the raw-HTML short-circuit rule (tags outside code span/fence skip conversion for the entire field, including inline image handling).
- Clarifies that titles (todo content argument, card/message titles) are plain text and never converted.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Updates agent-facing invariants to accurately describe Markdown conversion behavior and raw-HTML rules for rich-text fields. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d821ee7e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟡 Changes recommended
The updated invariant’s “any HTML tag” wording is broader than the CLI’s actual HTML detection rules and could still mislead agents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
A couple of documentation phrases remain ambiguous/overbroad and could still mislead agents about which exact fields are converted and what HTML is recognized for short-circuiting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new raw-HTML paragraph currently overstates “passes through unchanged” and incorrectly claims local inline image upload only works from Markdown, which can mislead agents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The updated invariant overstates inline image upload behavior for raw <img src> across all listed fields, which can mislead agents (e.g., notes explicitly don’t support attachments).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
… included The skill told agents that todo, document and card content is sent as-is, which sent them hand-writing HTML — and any HTML tag in a field skips the Markdown path entirely, inline image upload with it. The CLI has converted Markdown for all of those for a while; the invariant now names every field that is converted, the all-or-nothing rule for raw HTML, and the titles that stay plain text. Fixes #674
…ch tags trip the HTML rule
…aphs, and local img uploads The passthrough is not unchanged (insertParagraphSeparators runs on it) and local image upload is driven by scanning the HTML for img tags after conversion, so a raw <img src="/local/path"> uploads too; what HTML input loses is Markdown syntax, the image syntax included.
…oads everywhere else
8b1e596 to
98e0b5a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
One sentence remains ambiguous and can be read as claiming chat supports inline local-image uploads, which is inconsistent with the chat command implementation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98e0b5a585
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ent as written, and limit the fence exception to backticks
There was a problem hiding this comment.
🟢 Approval recommended
The documentation change matches verified CLI behavior for Markdown conversion, raw-HTML passthrough, and mention resolution.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
skills/basecamp/SKILL.mdtold agents that todo, document and card content is "sent as-is — use plain text or HTML directly". The CLI has runrichtext.MarkdownToHTMLon every one of those for a while (todos.go,cards.go,files.go, plus messages, comments, chat, check-ins, schedule entries and notes), and the guidance was actively harmful: an agent following it hand-writes HTML, and any HTML tag in a field makesIsHTML()skip the Markdown path for the whole field — inline image upload with it, sinceonly resolves from Markdown.Invariant 5 now names every field that is converted, states the all-or-nothing rule for raw HTML (a tag outside a code span or fence skips conversion for the whole field, images included), and notes that titles — a todo's content argument, card and message titles — are plain text and never converted.
Reproduced on main by reading the call sites (every
MarkdownToHTMLcaller ininternal/commands) and the existing unit coverage that asserts a todo description comes back as<strong>from**bold**. No prose guard exists for this line, so no new test;TestSkillMDQuickReferenceCommands,TestDocContract*andmake check-skill-driftpass, andbin/ciis green on Linux (thelio, Go 1.26.7).Fixes #674
Summary by cubic
Updates the Basecamp skill so agents know which rich-text fields the CLI converts from Markdown to HTML, which are sent as written, and that chat posts stay plain text unless they carry a mention or
--content-type text/html. The old guidance said todo, document, and card content was sent as-is, which led agents to hand-write HTML — and any HTML tag skips Markdown conversion for the whole field, breaking inline image upload.<p>blocks, that local<img>paths upload as attachments in all converted fields except notes, and that titles stay plain text.Fixes #674.
Written for commit de9d354. Summary will update on new commits.