Fix schedule update silently dropping --attach without --description

[jeremy]

Summary

• basecamp schedule update <id> --summary "New" --attach foo.pdf silently dropped the attachment because the attach-only fallback was gated on !hasChanges — any other flag (summary, starts-at, etc.) set hasChanges = true first, skipping the fallback
• Changes guard from !hasChanges to description == "" so attachments are processed whenever no description text is provided, regardless of other flags
• The cards update command handles this correctly (attach block is outside the content guard); this aligns schedule with that pattern

Test plan

• make test — all pass
• make lint — clean
• Manual: basecamp schedule update <id> --summary "test" --attach <file> should upload the attachment

---

Summary by cubic

Fixes basecamp schedule update dropping --attach when used with other flags like --summary and no description. Attachments now process whenever the description is empty, matching cards update.

• Bug Fixes
  • Changed guard from !hasChanges to description == "" for attachment handling.
  • Honors --attach alongside flags like --summary and --starts-at when no description text is passed.

^{Written for commit 8a9343a. Summary will update on new commits.}

[Copilot]

Pull request overview

This PR fixes basecamp schedule update so --attach is not silently ignored when the user updates other fields (e.g. --summary) without providing --description, aligning schedule’s behavior with the existing cards update pattern.

Changes:

• Adjust attachment-handling guard to key off description == "" (instead of !hasChanges) so attachments are processed whenever no description text is provided.
• Preserve the existing “embed attachments into rich text” flow, but ensure it triggers even when other update flags are present.

Comments suppressed due to low confidence (1)

internal/commands/schedule.go:686

• This change fixes a previously silent behavior regression, but there’s no unit test covering the specific case where other update flags are set (e.g. --summary) while --attach is provided without --description. Since this file already has schedule update request-body tests, please add a test that runs schedule update with --summary + --attach and asserts the outgoing update body includes a non-empty description containing the embedded attachment HTML (and that the attachment upload endpoint is invoked).

			// Handle attachments when no description text provided
			if description == "" && len(attachFiles) > 0 {
				refs, attachErr := uploadAttachments(cmd, app, attachFiles)
				if attachErr != nil {
					return attachErr
				}
				req.Description = richtext.EmbedAttachments("", refs)

---

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 959025a45e

ℹ️ 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".

[cubic-dev-ai]

1 issue found across 1 file

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="internal/commands/schedule.go">

<violation number="1" location="internal/commands/schedule.go:681">
P1: This condition now triggers the attachment fallback for any update that omits `--description` (including `--summary ... --attach ...`), and that path rebuilds `req.Description` from attachment markup only. That can overwrite existing schedule entry description text during metadata-only updates, so preserve existing description content when attaching without an explicit description update.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

---

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

[Copilot]

This change fixes the runtime behavior, but there’s no regression test covering the previously-broken case (e.g., schedule update with --summary + --attach and no --description). Consider adding a unit test in schedule_test.go that exercises update with another field set plus --attach, and asserts the outgoing PUT body includes an embedded attachment in description (and that the attachment upload request was made).