Skip to content

V1 CLI ship polish#160

Merged
jeremy merged 59 commits intomainfrom
v1-ship-polish
Feb 27, 2026
Merged

V1 CLI ship polish#160
jeremy merged 59 commits intomainfrom
v1-ship-polish

Conversation

@jeremy
Copy link
Member

@jeremy jeremy commented Feb 25, 2026

Summary

Hands-on audit of the CLI surface ahead of v1 — fixing poor defaults, broken output, unnecessary latency, missing self-update, and hardening CI/CD + install.

CLI polish

  • Stats line: drop "Stats:" prefix, use middle dot separator (298ms · 1 request)
  • Hints gating: rename "Next:" → "Hints:", off by default in release builds (--hints/--no-hints)
  • Root command: zero API calls — remove ResolveProject() roundtrip on bare basecamp invocation
  • Commands listing: grouped render with bold category headers, aligned columns, and muted action lists
  • Search results: humanized output — simplified type names, relative timestamps, truncated titles, project names from bucket
  • Projects list: show IDs instead of redundant "active" status column; add bookmarked to detail view
  • basecamp upgrade: new command — checks GitHub releases, auto-upgrades via Homebrew when detected
  • TUI search: fix query dedup guard that silently blocked re-searches; clamp view height to prevent chrome overflow
  • Theme live-reload: fsnotify watcher with atomic-save and symlink-repoint support; safe double-close

CI/CD hardening

  • Makefile lint resolution: prefer GOBIN/GOPATH binary over system PATH to match active Go toolchain
  • Release quality gate: install-only: true on golangci-lint-action (no double lint), govulncheck step added
  • Release security parity: security.yml exposed as reusable workflow (workflow_call), release gates on [test, security]
  • Benchmark regression: exit 1 after ::error:: annotation — step turns red in checks UI while continue-on-error: true keeps it non-blocking
  • Install script portability: find_sha256_cmd helper tries sha256sum then shasum -a 256; grep -F for fixed-string checksum matching
  • Cosign identity: exact match (--certificate-identity) with release workflow + tag ref instead of broad regexp
  • Gosec scanning: switched from Docker action to go install so gosec inherits git credentials for private modules; -no-fail with SARIF upload for visibility (100+ pre-existing findings are accepted CLI patterns)
  • Dependency review: continue-on-error: true — requires GitHub Advanced Security not enabled on this repo

Test plan

  • basecamp — instant (46ms, 0 requests), no API calls, shows version + auth + account
  • basecamp projects — shows id + name columns, no "active" status; stats line shows 3.0s · 5 requests
  • basecamp projects --hints — shows "Hints:" section; --no-hints strips breadcrumbs
  • basecamp commands — grouped categories with aligned command names, descriptions, actions
  • basecamp search "query" — clean table: id, title, type (simplified), project, relative time
  • basecamp upgrade — version check, handles dev build correctly
  • TUI: 18 search tests pass — Enter re-fires same query, height clamp prevents chrome overflow
  • make check passes clean (vet, lint, unit tests, 267 e2e tests, naming check)
  • bash -n scripts/install.sh — syntax OK
  • make lint resolves to GOBIN binary, not system PATH
  • release.yml: install-only: true, govulncheck step, security workflow gate, no duplicate lint
  • test.yml: benchmark regression step exits 1 (visible in checks UI, non-blocking via continue-on-error)
  • security.yml: gosec scans full codebase (private modules resolved), gitleaks + trivy pass, dependency-review non-blocking

Before: Stats: 298ms | 1 request
After:  298ms · 1 request
Add --hints/--no-hints flags (default: on in dev builds, off in release).
When disabled, breadcrumbs are stripped from output via WithoutBreadcrumbs().
Rename "Next:" label to "Hints:" across all output modes.
Remove ResolveProject() call that added ~200ms latency on every bare
`basecamp` invocation. Project ID from config is sufficient; name
resolution was nice-to-have but not worth a synchronous API roundtrip.
Show active --profile name in summary when set.
Styled output now shows category headers with aligned command names,
descriptions, and available actions. JSON/markdown still passes full
structure through app.OK() for machine consumption.
Transform raw SDK structs into clean table columns: id, title (truncated
to 60 chars), type (simplified: Chat::Lines::RichText → chat), project
(from Bucket.Name), and created (relative time). Drops noisy columns
like url, app_url, bookmark, visible_to_clients, raw timestamps.
Replace [name, status] columns with [id, name] — every project in the
list is active so the status column wasted space. Add bookmarked field
to detail view for completeness.
Copilot AI review requested due to automatic review settings February 25, 2026 22:18
Checks GitHub for latest release, compares with current version.
Detects Homebrew installations and runs brew upgrade automatically;
otherwise directs to GitHub releases. Updates doctor hint to suggest
`basecamp upgrade` when an update is available.
submitQuery() no longer silently skips when the user re-submits the same
query (e.g., pressing Enter again to refresh results). The debounce path
still deduplicates to avoid redundant in-flight requests during typing.

Add height clamp to Search.View() to prevent the list from overflowing
its allocated viewport and pushing chrome off-screen.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0a8fc8256

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Polishes the v1 CLI surface by refining default output (stats + hints), improving responsiveness on the root command, making search output more readable, and adding a self-upgrade command, alongside a couple of TUI fixes.

Changes:

  • Renames “Next” → “Hints”, gates breadcrumbs behind --hints/--no-hints, and updates stats rendering to 298ms · 1 request (no “Stats:” prefix).
  • Improves search UX (CLI humanized rows + TUI resubmits) and adjusts project presentation (list shows id, detail includes bookmarked).
  • Adds basecamp upgrade (checks GitHub latest release and upgrades via Homebrew when applicable) and improves basecamp commands styled listing.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
internal/tui/workspace/views/search_test.go Updates tests to match new submit/debounce behavior.
internal/tui/workspace/views/search.go Allows re-submitting the same query; clamps view height to avoid chrome overflow.
internal/presenter/schemas/project.yaml Changes project list columns to [id, name]; adds bookmarked to detail.
internal/presenter/render.go Renames affordance header from “Next:” to “Hints:”.
internal/presenter/presenter_test.go Updates markdown affordance heading assertion to “Hints”.
internal/output/render.go Renames breadcrumbs label to “Hints:” and updates stats separator/prefix.
internal/output/output_test.go Updates output tests for “Hints” heading and stats formatting changes.
internal/output/envelope.go Adds WithoutBreadcrumbs() response option; updates markdown heading to “Hints”.
internal/commands/url_test.go Enables hints in test app flags so breadcrumbs are present.
internal/commands/upgrade.go Introduces new basecamp upgrade command (GitHub latest + Homebrew upgrade).
internal/commands/search.go Humanizes search results for display (type simplification, relative time, truncated titles).
internal/commands/quickstart.go Removes project name resolution API call; adds profile name to summary.
internal/commands/people_test.go Enables hints in test app flags so breadcrumbs are present.
internal/commands/doctor.go Updates upgrade hint to “Run: basecamp upgrade”.
internal/commands/commands_test.go Registers new upgrade command in catalog test.
internal/commands/commands.go Adds styled grouped rendering for basecamp commands; includes upgrade in catalog.
internal/cli/root.go Adds --hints/--no-hints global flags; registers upgrade command.
internal/appctx/context.go Implements breadcrumb gating in App.OK; updates stats stderr formatting separator.

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

Only humanize search output for styled terminal display; --json/--agent
get the full SDK SearchResult structs with all fields intact. Fall back
to Subject when Title is empty (some Basecamp record types use Subject).
- upgrade.go: use cmd.OutOrStdout() instead of fmt.Println/os.Stdout
- upgrade.go: extract versionChecker/homebrewChecker vars for testability
- upgrade_test.go: add 4 tests covering dev build, current, available, writer
- commands.go: accept io.Writer param, account for experimental prefix in alignment
- search.go: use rune-based truncation for UTF-8 safety
Copilot AI review requested due to automatic review settings February 25, 2026 22:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.


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

Pointer fields (*bool/*int) enable three-state semantics: nil = not set
(fall through to version.IsDev()), explicit true/false = user preference.

Load from config files and BASECAMP_HINTS/BASECAMP_STATS env vars,
with standard layering: env > local > repo > global > system > defaults.

Strict validation: env bools must be true/false/1/0 (invalid values
ignored to preserve nil semantics); verbose range-checked to 0-2.
Change --hints/--stats defaults to false so Changed() can detect explicit
flag usage. New resolvePreferences() in PersistentPreRunE checks:
explicit flag > config value > version.IsDev() fallback.

Negative flags (--no-stats, --no-hints) only suppress config resolution
when their value is true, not merely when Changed.
Add hints, stats, verbose to validKeys. Bool validation for hints/stats,
int 0-2 validation for verbose. Show preferences in config show when
explicitly set.
Copilot AI review requested due to automatic review settings February 26, 2026 16:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.


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

The OAuth server expects "full" for write access, not "write".
The wizard was sending an invalid scope, causing auth to silently
fall back to the default.
The config key is "account_id" not "account", so the suggested
command was producing a config error.
len(v) counts bytes, not characters. Multi-byte UTF-8 strings
(e.g. Japanese, emoji) could be sliced mid-rune, producing
invalid output. Use utf8.RuneCountInString and []rune conversion.
Passing theme.Primary (AdaptiveColor) instead of
lipgloss.Color(theme.Primary.Dark) lets lipgloss resolve
Light vs Dark at render time based on terminal background
detection. Fixes incorrect colors on light terminal themes.
Include Rename events so editors using temp-file+rename are detected.
Re-add the resolved directory on re-arm so symlink repoints are tracked.
Replaces byte-slicing loop with the existing widget.Truncate()
which handles multi-byte UTF-8 correctly.
Remove -no-fail from gosec so findings fail the build. Replace the
deferred TODO with an actual dependency-review job that runs on PRs.
Add lint, test-e2e, and race detection to the release gate. Install
golangci-lint and BATS (with cache) to support the expanded checks.
Drop the dogfooding banner from README, remove GOPRIVATE from go install
instructions, and clean up goreleaser release notes and deferred comments.
Download checksums.txt and verify SHA256 before extracting the archive.
When cosign is available, also verify the Sigstore signature against the
GitHub Actions OIDC issuer. Shared tmpdir between download and verify.
Use ::error:: so regressions surface in the PR checks UI with a red
indicator. Job-level continue-on-error keeps it non-blocking.
Copilot AI review requested due to automatic review settings February 27, 2026 18:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 67 out of 68 changed files in this pull request and generated 2 comments.


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

In mixed-tooling environments (e.g. Homebrew binary built with a
different Go version), command -v may find the wrong golangci-lint
first. Try GOBIN, then GOPATH/bin, then fall back to PATH to ensure
the binary matching the active Go toolchain wins.
The golangci-lint-action was running lint by default then make lint
ran it again. Set install-only: true so make lint is the single
invocation. Add govulncheck as a release-path security check.
The error annotation logged but the step exited 0, making the
regression invisible in the checks UI. The job already has
continue-on-error: true so exit 1 turns the step red without
blocking merge.
Add find_sha256_cmd helper that tries sha256sum before shasum for
Linux compatibility. Tighten cosign identity from a regexp matching
any workflow to an exact match on the release workflow at the
specific tag ref.
Tag pushes don't trigger security.yml (it runs on push-to-main and
PRs). Add workflow_call to security.yml and invoke it from
release.yml so gitleaks, gosec, and trivy must pass before a release
proceeds.
grep with a regex pattern treats dots as wildcards, which could match
unintended lines in checksums.txt. Use grep -F for a literal match.
Copilot AI review requested due to automatic review settings February 27, 2026 19:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 67 out of 68 changed files in this pull request and generated 6 comments.


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

CloseWatcher now sets w.themeWatcher = nil after closing, which
makes repeated calls safe and prevents the ThemeChangedMsg handler
from re-arming the watcher goroutine after shutdown.
The securego/gosec Docker action runs its own Go toolchain in a
container that cannot inherit the host's git config, so it fails to
resolve the private SDK module. Switch to go install + direct
invocation so gosec uses the host's Go and git credentials.

Mark dependency-review as continue-on-error since it requires GitHub
Advanced Security which is not enabled on this repo.
Copilot AI review requested due to automatic review settings February 27, 2026 19:27
gosec now resolves private modules correctly (go install instead of
Docker), which means it actually scans the full codebase and finds
100+ pre-existing findings (G304 file inclusion, G104 unhandled
errors in TUI, G117 token fields). These are accepted patterns in a
CLI tool. Use -no-fail so findings are reported via SARIF without
blocking CI.

dependency-review requires GitHub Advanced Security which is not
enabled on this repo — mark continue-on-error so it does not block
the security workflow.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 67 out of 68 changed files in this pull request and generated 6 comments.


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

@jeremy jeremy merged commit 53d2873 into main Feb 27, 2026
10 of 11 checks passed
@jeremy jeremy deleted the v1-ship-polish branch February 27, 2026 22:53
@jeremy jeremy added the enhancement New feature or request label Feb 28, 2026
@jeremy jeremy mentioned this pull request Feb 28, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants