Merged
Conversation
Owner
cboone
commented
Feb 9, 2026
cboone
commented
- docs: add plan to fix bugs, harden tests, and improve search UX
- docs: fix plan counts and clarify live scrut strategy
- fix: fix text search, harden tests, and improve search UX
- test: exercise SearchEmails snippet path and harden live checks
Covers two bugs (broken text search, scrut test environment leakage), four functional improvements (search pagination/sort/unread, mailbox caching), two UX improvements (sort syntax flexibility, better error messages), and three test improvements (environment isolation, success-path tests, text format tests).
Fix broken text search by correcting the SearchSnippet/get JMAP result reference to point at Email/query (path /ids) instead of Email/get (path /list/*/id). Add --offset, --sort, and --unread flags to the search command for parity with list. Accept colon syntax in --sort (e.g. receivedAt:asc). Cache mailbox list per Client instance to reduce redundant API calls. Improve JMAP error messages to include the method name. Isolate scrut tests from ambient environment variables. Add opt-in live integration tests and unit test for snippet reference.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes jm search text-query behavior, expands search UX capabilities (offset/sort/unread), and hardens the CLI scrut suite by isolating environment-dependent configuration. It also introduces an opt-in live integration scrut suite and updates documentation accordingly.
Changes:
- Fix
SearchSnippet/getJMAP result referencing for text search, and add unit coverage for that request shape. - Add
searchpagination/sort/unread support (flags + client implementation), plus improved search method-error messaging. - Make CLI scrut tests deterministic by unsetting
JMAP_*env vars; add opt-intests/live.mdand splitmake test-clivsmake test-cli-live.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/client/email.go |
Fix snippet reference to Email/query /ids; add offset/sort/unread; improve method-error context; set result offset. |
internal/client/email_test.go |
Add unit test ensuring SearchSnippet/get references Email/query /ids and populates snippets. |
cmd/search.go |
Add --offset, --sort, --unread flags and validation; reuse parseSort. |
cmd/list.go |
Extend parseSort to accept colon syntax (e.g. from:asc). |
cmd/list_test.go |
Add tests for colon sort syntax. |
internal/client/mailbox.go |
Cache mailbox list for the lifetime of a Client instance. |
internal/client/client.go |
Add per-client mailbox cache storage and a doFunc hook to enable request capture in unit tests. |
tests/errors.md |
Unset JMAP_* env vars for auth-failure expectations. |
tests/flags.md |
Unset JMAP_* env vars for auth-failure expectations. |
tests/arguments.md |
Unset JMAP_* env vars for auth-failure expectations. |
tests/help.md |
Update search help snapshot for new flags. |
tests/live.md |
Add opt-in live scrut tests for success paths, snippets, and text output. |
Makefile |
Split deterministic scrut runs (test-cli) from opt-in live runs (test-cli-live). |
docs/CLI-REFERENCE.md |
Document new search flags and colon sort syntax. |
docs/plans/done/2026-02-08-fix-bugs-and-improve-search-tests-ux.md |
Add completed plan/design notes reflecting the implemented changes. |
docs/plans/todo/dapper-sauteeing-taco.md |
Remove obsolete TODO plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add HOME=/nonexistent to env commands in error, flag, and argument tests so that ~/.config/jm/config.yaml cannot leak a token or other settings into the test environment. Add --format json to live test subshells that parse output with python3 json.load, ensuring deterministic behavior regardless of JMAP_FORMAT env var or config file.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
cmd/list.go:68
- With the new colon-to-space normalization, inputs like
receivedAt:foowill now be treated as field=receivedAtwith an unrecognized direction silently falling back to descending. Consider validating the direction token when present (accept onlyasc/desc, error otherwise) so typos don’t change sort behavior silently.
func parseSort(s string) (field string, ascending bool, err error) {
s = strings.ReplaceAll(s, ":", " ")
parts := strings.Fields(s)
field = "receivedAt"
ascending = false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Each live test block now includes inline precondition checks (JMAP_LIVE_TESTS and JMAP_TOKEN) so blocks properly skip when conditions are unmet, since scrut exit 80 is block-scoped. Commands asserting JSON output now pass --format json explicitly to stay deterministic regardless of user config.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
cmd/list.go:79
parseSorttreats any second token other thanascas descending (including invalid values). This conflicts with the CLI help/docs that imply onlyasc/descare accepted, and can hide typos like--sort "receivedAt des". Consider validating the direction token explicitly (accept onlyasc/desc, error otherwise) so users get immediate feedback.
func parseSort(s string) (field string, ascending bool, err error) {
s = strings.ReplaceAll(s, ":", " ")
parts := strings.Fields(s)
field = "receivedAt"
ascending = false
if len(parts) >= 1 {
normalized, ok := validSortFields[strings.ToLower(parts[0])]
if !ok {
return "", false, fmt.Errorf("unsupported sort field %q", parts[0])
}
field = normalized
}
if len(parts) >= 2 && strings.EqualFold(parts[1], "asc") {
ascending = true
}
return field, ascending, nil
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace redundant standalone precondition scrut blocks in live tests with prose explaining the per-block guard pattern, since scrut exit 80 is block-scoped. Tighten parseSort to reject invalid sort direction values instead of silently falling back to descending.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.