Conversation
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.
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.
d0a8fc8 to
9c45fea
Compare
There was a problem hiding this comment.
💡 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".
There was a problem hiding this comment.
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 to298ms · 1 request(no “Stats:” prefix). - Improves search UX (CLI humanized rows + TUI resubmits) and adjusts project presentation (list shows
id, detail includesbookmarked). - Adds
basecamp upgrade(checks GitHub latest release and upgrades via Homebrew when applicable) and improvesbasecamp commandsstyled 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
There was a problem hiding this comment.
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.
d089707 to
6929060
Compare
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
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.
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
298ms · 1 request)--hints/--no-hints)ResolveProject()roundtrip on barebasecampinvocationbasecamp upgrade: new command — checks GitHub releases, auto-upgrades via Homebrew when detectedCI/CD hardening
GOBIN/GOPATHbinary over system PATH to match active Go toolchaininstall-only: trueon golangci-lint-action (no double lint), govulncheck step addedworkflow_call), release gates on[test, security]exit 1after::error::annotation — step turns red in checks UI whilecontinue-on-error: truekeeps it non-blockingfind_sha256_cmdhelper triessha256sumthenshasum -a 256;grep -Ffor fixed-string checksum matching--certificate-identity) with release workflow + tag ref instead of broad regexpgo installso gosec inherits git credentials for private modules;-no-failwith SARIF upload for visibility (100+ pre-existing findings are accepted CLI patterns)continue-on-error: true— requires GitHub Advanced Security not enabled on this repoTest plan
basecamp— instant (46ms, 0 requests), no API calls, shows version + auth + accountbasecamp projects— showsid+namecolumns, no "active" status; stats line shows3.0s · 5 requestsbasecamp projects --hints— shows "Hints:" section;--no-hintsstrips breadcrumbsbasecamp commands— grouped categories with aligned command names, descriptions, actionsbasecamp search "query"— clean table: id, title, type (simplified), project, relative timebasecamp upgrade— version check, handles dev build correctlymake checkpasses clean (vet, lint, unit tests, 267 e2e tests, naming check)bash -n scripts/install.sh— syntax OKmake lintresolves to GOBIN binary, not system PATHinstall-only: true, govulncheck step, security workflow gate, no duplicate lintcontinue-on-error)