Skip to content

Add Bubble Up Now and Pop commands - #164

Closed
jr-lillard wants to merge 1 commit into
basecamp:mainfrom
jr-lillard:feat/bubble-up-now-pop
Closed

Add Bubble Up Now and Pop commands#164
jr-lillard wants to merge 1 commit into
basecamp:mainfrom
jr-lillard:feat/bubble-up-now-pop

Conversation

@jr-lillard

@jr-lillard jr-lillard commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add hey bubble-up-now <box-item-id> --topic-id <thread-id> and hey pop <box-item-id> --topic-id <thread-id>.
  • Read every Imbox and Bubble Up page before acting, require one exact box item/thread pair, and fail closed on absent, mismatched, incomplete, or truncated target data.
  • Return structured changed/no-op state and verify every successful or ambiguous mutation with bounded fresh reads.
  • Route both mutations through the generated HEY SDK posting operations; no CLI HTTP transport was added.

SDK dependency

Current main already uses github.com/basecamp/hey-sdk/go v0.21.0, which exposes Postings().BubbleUpNow and Postings().CancelBubbleUp. This PR changes no dependency or generated SDK source.

Safety and automation behavior

  • Bubble Up Now is non-idempotent and the generated SDK does not retry it. The CLI never replays it after an ambiguous response; it verifies by reading and tells callers to rerun the high-level command, which becomes a no-op if already applied.
  • Pop uses the SDK idempotent cancel operation and is also followed by exact-state verification.
  • Both IDs must come from the same complete box row. No subject, newest-message, or partial-record targeting is used.
  • A box item present without a resolvable thread ID returns retryable target_incomplete without mutation.
  • Complete pagination and exact-pair checks use the current shared mail-source reader, so named box routes and current topic_id URL handling stay centralized.

Verification

  • Focused Bubble Up/Pop unit tests cover exact routes and payloads, all-page traversal, mismatch/absence/incomplete shape, exact-pair preference, verified no-op, scheduled state, ambiguous response recovery, and single-submit behavior.
  • All non-TUI internal packages pass.
  • Formatting, vet, pinned golangci-lint v2.11.1, dependency tidiness, generated surface, release-lockstep checks, build, help/command inspection, and smoke-suite compilation pass.
  • Two unrelated local internal/tui calendar assertions fail identically on pristine current main 79b5a1a; the PR does not touch TUI code.
  • check-surface-compat also reproduces current main's stale .surface-breaking entries against v1.0.0; this PR only adds four surface entries and removes none.

No production mailbox mutation was performed while rebuilding this current-main port.


Summary by cubic

Adds hey bubble-up-now <posting-id> --topic-id <topic-id> and hey pop <posting-id> --topic-id <topic-id> to move one exact thread into or out of Bubble Up. Previously the CLI couldn’t control Bubble Up; now it requires an exact posting/topic pair and fails closed on incomplete, mismatched, or missing targets.

  • Reads every Imbox and Bubble Up page before mutating; aborts with a retryable error on incomplete pagination, not_found on absence, target_mismatch on ID conflicts, and target_incomplete when a row is missing a parseable topic in app_url.
  • Bubble Up Now is non-idempotent and is never replayed; on ambiguous responses the CLI verifies via fresh reads without re-submitting. Pop is idempotent. Both return structured results with changed/no_op, verified, and before/after.
  • Registers two new commands and routes mutations through the SDK (Postings().BubbleUpNow, Postings().CancelBubbleUp). Updates .surface, README.md, skills/hey/SKILL.md, API-COVERAGE.md, and adds focused unit and smoke tests.

Migration

  • Pass both IDs from the same Imbox row (id and topic_id); do not infer. Retry only after a complete box read shows the exact pair.

Written for commit fe52c28. Summary will update on new commits.

Review in cubic

Copilot AI balanced review requested due to automatic review settings August 17, 2026 20:04
@jr-lillard

Copy link
Copy Markdown
Contributor Author

@basecamp/cli The Test, CI, and Security workflows are currently waiting for maintainer approval (action_required). The focused tests, full make check, surface target, build, and smoke-suite compilation all pass locally. When convenient, please approve the fork workflows. The PR also calls out the temporary SDK-main pseudo-version; it can be replaced with the next SDK tag when published.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Bubble Up mutation commands to the HEY CLI (bubble an exact posting now, or pop it back out), including verification logic and documentation updates, alongside SDK-driven API signature adjustments.

Changes:

  • Introduces hey bubble-up-now and hey pop commands with exact posting/topic preflight and post-mutation verification.
  • Adds unit tests for Bubble Up behaviors and a new smoke test exercising the real CLI against an authenticated account.
  • Updates docs/surface area and adapts callsites to updated SDK types/signatures (calendar recordings params, timetrack start, mute vs ignore in TUI).

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/smoke/bubble_up_test.go Adds an end-to-end smoke test for Bubble Up + Pop using real CLI calls
skills/hey/SKILL.md Documents new Bubble Up commands and the “exact posting/topic pair” requirement
internal/tui/mail.go Renames “ignore” action/binding to “mute” and routes to SDK mute call
internal/tui/calendar.go Adjusts recordings query params to new SDK pointer fields
internal/cmd/timetrack.go Updates timetrack start to new SDK method signature
internal/cmd/sdk.go Updates recordings params to new SDK pointer fields
internal/cmd/root.go Registers the new Bubble Up commands
internal/cmd/reply.go Refactors reply logic into helper for reuse
internal/cmd/recordings.go Updates recordings params to new SDK pointer fields
internal/cmd/compose.go Reuses reply helper for thread-targeted compose flow
internal/cmd/bubble_up_test.go Adds comprehensive unit tests for Bubble Up/Pop behaviors
internal/cmd/bubble_up.go Implements Bubble Up Now / Pop commands with guarded mutation + verification
go.mod Bumps hey-sdk and related deps to match new API/types
go.sum Updates checksums for bumped dependencies
README.md Adds examples and behavior notes for new Bubble Up commands
API-COVERAGE.md Documents new endpoints/coverage for Bubble Up mutations
.surface Adds new commands/flags to surfaced CLI trigger list

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/smoke/bubble_up_test.go Outdated
Comment thread internal/cmd/bubble_up.go Outdated
Comment thread internal/cmd/bubble_up.go Outdated
@jr-lillard

Copy link
Copy Markdown
Contributor Author

@basecamp/cli PR #164 is now rebased onto current main at 8d3e675, uses released hey-sdk v0.7.0, and has all review threads resolved at head 7bcbcf6. Focused tests, full make check, make build, surface compatibility, smoke-suite compilation, and the 70.8% coverage floor pass locally. Test, Security, and CodeQL Advanced are action_required; please approve the fork workflows when convenient.

@jr-lillard
jr-lillard force-pushed the feat/bubble-up-now-pop branch from 7bcbcf6 to fe52c28 Compare August 25, 2026 01:15
@jr-lillard

Copy link
Copy Markdown
Contributor Author

@basecamp/cli PR #164 is now rebuilt as one commit on current main 79b5a1a at head fe52c28 and is mergeable. Test, Security, CodeQL Advanced, and Dependabot Auto-Merge are action_required; please approve the fork workflows when convenient. Focused Bubble Up tests, all non-TUI internal tests, formatting, vet, pinned lint, dependency tidiness, generated surface, release lockstep, build, and smoke-suite compilation pass. The two local TUI calendar assertions and stale surface-compat exemptions reproduce on pristine current main and are documented in the refreshed PR body.

@monorkin

Copy link
Copy Markdown
Collaborator

Thank you @jr-lillard for the PR.
I wanted to change the interface a bit and created a new PR with you as the co-author on it.
#326

@monorkin monorkin closed this Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants