feat(chat): add chat update command for editing campfire messages#462
Conversation
github-actions
Bot
added
commands
There was a problem hiding this comment.
Pull request overview
Adds a new basecamp chat update / basecamp campfire update command so existing campfire lines can be edited in place, extending the chat command set to cover the SDK’s new UpdateLine capability.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Adds the new chat update subcommand, including project/room resolution, optional rich-text content type handling, and mention resolution behavior aligned with
chat post. - Updates command metadata and generated CLI surface/docs so the new action appears in help, skill docs, and smoke coverage.
- Temporarily swaps the SDK dependency to a forked revision that exposes
CampfiresService.UpdateLine.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
skills/basecamp/SKILL.md |
Adds a chat update example to the Basecamp skill docs. |
internal/commands/commands.go |
Registers update in the chat command catalog. |
internal/commands/chat.go |
Implements the new chat line update command and wires it into chat. |
internal/commands/chat_test.go |
Adds unit coverage for update request payloads and empty-content rejection. |
go.sum |
Updates module checksums for the temporary SDK fork. |
go.mod |
Adds the temporary replace to the forked SDK revision. |
e2e/smoke/smoke_campfire.bats |
Adds smoke coverage for editing an existing campfire message. |
e2e/chat.bats |
Adds a CLI error-path test for missing chat update arguments. |
.surface |
Regenerates command/flag surface metadata for the new subcommand. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="e2e/smoke/smoke_campfire.bats">
<violation number="1" location="e2e/smoke/smoke_campfire.bats:57">
P3: The new update smoke test does not verify that the message body actually changed, so a no-op update could still pass.</violation>
</file>
<file name="e2e/chat.bats">
<violation number="1" location="e2e/chat.bats:103">
P2: Missing-argument test is under-specified: it only checks for a non-zero exit, so unrelated failures would still make it pass.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
91d7567 to
ce934ec
Compare
ce934ec to
112e6fd
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
112e6fd to
6aaf95b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6aaf95b to
96cf589
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
96cf589 to
a0dd509
Compare
![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: 5065671927
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a6bb02b90
ℹ️ 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.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
internal/commands/chat.go:834
- When
effectiveChatIDis already known (via URL or--room), this still setsprojectHintfromapp.Config.ProjectIDand then resolves it for breadcrumbs. That makeschat update --room ...perform an unnecessary project lookup and can fail the update entirely if the configured project is stale/unresolvable (even though the PUT only needs chat+line IDs). Consider only using URL / explicit flag values for breadcrumb project resolution, not the config default.
projectHint = app.Flags.Project
}
if projectHint == "" {
projectHint = app.Config.ProjectID
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56f28ee7ae
ℹ️ 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".
Adds 'basecamp chat update <id|url> [content]' for editing existing
campfire lines in place via PUT /chats/{c}/lines/{l}, backed by the
SDK's CampfiresService.UpdateLine.
Content handling has four explicit modes, validated before any request:
- default: Markdown converted to HTML unconditionally, then @mentions
resolved (edits always render as rich text server-side);
- --content-type text/html: HTML supplied as-is, then @mentions resolved;
- --content-type text/plain: serialized verbatim via richtext.PlainToHTML
(escaped, line breaks preserved as <br>), mention resolution skipped;
- any other value: rejected with a usage error.
--content-type is applied client-side only; the SDK PUT carries just the
content, and the server coerces every edit to rich text.
URL arguments route through the shared urlarg parser (which gains a
CampfireID field for the chat-line route) plus a hostutil trusted-host
gate, so a pasted card/todo URL, a boundary-evading line ID, or a
look-alike on an untrusted host is rejected rather than silently
misinterpreted; a URL whose account differs from the configured account
is refused. The 204 PUT is followed by a GetLine re-fetch so the response
carries the canonical post-update line.
Bumps the SDK to CampfiresService.UpdateLine (basecamp/basecamp-sdk#295,
merge commit 09f7b326), pinned to the merge commit pending a release tag.
Wires up the surface snapshot, catalog, e2e + smoke coverage, and the
basecamp skill documentation.
|
Thanks @nnemirovsky! |
Summary
Adds
basecamp chat update <id|url> [content]for editing existing campfire lines in place, mirroring the shape ofchat post. Until now the CLI could only post or delete chat lines — editing required deleting and reposting, which bumps the message and is disruptive in active rooms.Backed by the SDK's
CampfiresService.UpdateLine(PUT /chats/{c}/lines/{l}, returns 204). Verified live: the command round-trips an edit against a real campfire line and re-fetches the canonical post-update line.Content handling
Chat edits always render as rich text server-side. Content is resolved into the exact body sent, in four explicit modes validated before any request:
@mentionsresolved (matchesmessages). Fixes a class of bug where**bold**with no mention was previously sent raw.--content-type text/html— HTML supplied as-is, then@mentionsresolved.--content-type text/plain— serialized verbatim viarichtext.PlainToHTML(HTML-special chars escaped, line breaks preserved as<br>), mention resolution skipped.--content-typeis applied client-side only; the SDK PUT carries justcontent, and the server coerces every edit to rich text.URL, host & argument safety
urlargparser (which gains aCampfireIDfield for the chat-line route) plus a newhostutil.IsTrustedBasecampHostgate (http/https only). A pasted card/todo/message URL, a chat-line collection URL (/chats/{c}/lines), a boundary-evading line ID, an untrusted/look-alike host, or an account mismatch is rejected rather than silently misinterpreted.--roomor a URL in hand, the PUT proceeds on the chat + line IDs alone, so a stale/unreachable default project can't fail the edit — it only drops--infrom breadcrumbs.<id|url>yields a purpose-specific usage error rather than the generic arg-count rewrite.SDK dependency
Pinned to the v0.8.0 SDK release, which carries
CampfiresService.UpdateLine(basecamp/basecamp-sdk#295) — no fork, noreplace.go.mod,go.sum,sdk-provenance.json, andAPI-COVERAGE.mdall point at the release.Test plan
bin/cigreen on the committed tree (unit, e2e, surface, skill drift, provenance, bare-groups, tidy, lint)