From 26fa43e2ccbb65ba5512a3ed6b60df01bf25e430 Mon Sep 17 00:00:00 2001 From: nullkey Date: Sat, 16 May 2026 01:22:27 +0800 Subject: [PATCH] fix(cli): post-audit fixes (MCP MatchCount + sessions + auth + view) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four unrelated shipped-code drifts found during v0.5 audit cycles: 1. MCP search_chunks tool omitted MatchCount from SearchParams. Server fell back to its default cap; agents asking for limit:50 silently got fewer results. Adds MatchCount: limit to the struct literal. 2. search sessions printed UpdatedAt as raw RFC3339 while session list used a fuzzy "X hours ago" render — same SDK field, two human renderings. Switches to the shared text.FuzzyAgoStr helper for parity. 3. auth status --json omitted three operationally-meaningful AuthUser fields (username, is_active, can_access_all_tenants). Agents branching on can_access_all_tenants previously needed a second round-trip. 4. session view Long help claimed the SDK doesn't wrap session_messages; it does (LoadMessages / GetMessagesBefore / GetRecentMessages all exist in client/message.go). Rewrites the comment to be accurate. --- cli/cmd/auth/status.go | 24 +++++++++++++++++------- cli/cmd/search/sessions.go | 4 +++- cli/cmd/search/sessions_test.go | 20 ++++++++++++++++++++ cli/cmd/session/view.go | 8 ++++---- cli/internal/mcp/tools.go | 1 + cli/internal/mcp/tools_test.go | 13 +++++++++++++ 6 files changed, 58 insertions(+), 12 deletions(-) diff --git a/cli/cmd/auth/status.go b/cli/cmd/auth/status.go index 891dbff8..03988dc2 100644 --- a/cli/cmd/auth/status.go +++ b/cli/cmd/auth/status.go @@ -14,7 +14,8 @@ import ( // authStatusFields enumerates the fields surfaced for `--json` discovery // on `auth status`. Single-resource shape: filter applies to data itself. var authStatusFields = []string{ - "context", "user_id", "email", "tenant_id", "tenant_name", + "context", "user_id", "username", "email", "is_active", + "can_access_all_tenants", "tenant_id", "tenant_name", } // StatusService is the narrow SDK surface auth status depends on. @@ -22,13 +23,19 @@ type StatusService interface { GetCurrentUser(ctx context.Context) (*sdk.CurrentUserResponse, error) } -// statusResult is the typed payload emitted by `--json`. +// statusResult is the typed payload emitted by `--json`. Mirrors the +// SDK AuthUser + AuthTenant projection so agents can branch on +// can_access_all_tenants (cross-tenant admin) and is_active (disabled +// account) without a second round-trip. type statusResult struct { - Context string `json:"context"` - UserID string `json:"user_id,omitempty"` - Email string `json:"email,omitempty"` - TenantID uint64 `json:"tenant_id,omitempty"` - TenantName string `json:"tenant_name,omitempty"` + Context string `json:"context"` + UserID string `json:"user_id,omitempty"` + Username string `json:"username,omitempty"` + Email string `json:"email,omitempty"` + IsActive bool `json:"is_active,omitempty"` + CanAccessAllTenants bool `json:"can_access_all_tenants,omitempty"` + TenantID uint64 `json:"tenant_id,omitempty"` + TenantName string `json:"tenant_name,omitempty"` } // NewCmdStatus builds the `weknora auth status` command. @@ -80,7 +87,10 @@ func runStatus(ctx context.Context, jopts *cmdutil.JSONOptions, f *cmdutil.Facto result := statusResult{Context: cfg.CurrentContext} if user != nil { result.UserID = user.ID + result.Username = user.Username result.Email = user.Email + result.IsActive = user.IsActive + result.CanAccessAllTenants = user.CanAccessAllTenants result.TenantID = user.TenantID } if tenant != nil { diff --git a/cli/cmd/search/sessions.go b/cli/cmd/search/sessions.go index b5df648a..2eb8a028 100644 --- a/cli/cmd/search/sessions.go +++ b/cli/cmd/search/sessions.go @@ -6,6 +6,7 @@ import ( "sort" "strings" "text/tabwriter" + "time" "github.com/spf13/cobra" @@ -105,12 +106,13 @@ done: } tw := tabwriter.NewWriter(iostreams.IO.Out, 0, 0, 2, ' ', 0) fmt.Fprintln(tw, "ID\tTITLE\tUPDATED") + now := time.Now() for _, s := range matches { title := text.Truncate(50, s.Title) if title == "" { title = "-" } - fmt.Fprintf(tw, "%s\t%s\t%s\n", s.ID, title, s.UpdatedAt) + fmt.Fprintf(tw, "%s\t%s\t%s\n", s.ID, title, text.FuzzyAgoStr(now, s.UpdatedAt)) } return tw.Flush() } diff --git a/cli/cmd/search/sessions_test.go b/cli/cmd/search/sessions_test.go index eea23c6f..12b2197f 100644 --- a/cli/cmd/search/sessions_test.go +++ b/cli/cmd/search/sessions_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -75,3 +76,22 @@ func TestSessionsSearch_NetworkError(t *testing.T) { require.ErrorAs(t, err, &typed) assert.Equal(t, cmdutil.CodeServerError, typed.Code) } + +// TestSessionsSearch_RendersFuzzyTime is a regression guard for the v0.5 +// audit bug: `search sessions` printed UpdatedAt as the raw RFC3339 string +// while `session list` ran it through text.FuzzyAgoStr — same SDK field, +// two human renderings. Asserts the human output now renders relative time +// (and does NOT contain the RFC3339 "T" date/time separator). +func TestSessionsSearch_RendersFuzzyTime(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeSessionsSearchSvc{ + pages: map[int][]sdk.Session{1: { + {ID: "s1", Title: "needle", UpdatedAt: time.Now().Add(-2 * time.Hour).Format(time.RFC3339)}, + }}, + total: 1, + } + require.NoError(t, runSessionsSearch(context.Background(), &SessionsSearchOptions{Query: "needle", Limit: 10}, nil, svc)) + body := out.String() + assert.Contains(t, body, "hour", "must render relative time (e.g. 'about 2 hours ago'), not raw RFC3339") + assert.NotContains(t, body, "T0", "raw RFC3339 has 'T' between date and time; fuzzyTime output should not") +} diff --git a/cli/cmd/session/view.go b/cli/cmd/session/view.go index 95b3032f..ace97343 100644 --- a/cli/cmd/session/view.go +++ b/cli/cmd/session/view.go @@ -25,10 +25,10 @@ type ViewService interface { GetSession(ctx context.Context, id string) (*sdk.Session, error) } -// NewCmdView builds `weknora session view `. The server endpoint -// returns metadata only (title/description/timestamps); message content -// lives under a separate session_messages endpoint that the SDK doesn't -// currently wrap, which is why there's no --full flag. +// NewCmdView builds `weknora session view `. Renders session metadata +// only (title/description/timestamps). Full chat-history retrieval is a +// separate concern (the SDK has LoadMessages / GetMessagesBefore for it); +// surfacing it as `session view --full` is queued for v0.6. func NewCmdView(f *cmdutil.Factory) *cobra.Command { opts := &ViewOptions{} cmd := &cobra.Command{ diff --git a/cli/internal/mcp/tools.go b/cli/internal/mcp/tools.go index 8fadac37..e7532608 100644 --- a/cli/internal/mcp/tools.go +++ b/cli/internal/mcp/tools.go @@ -283,6 +283,7 @@ func addSearchChunks(server *mcpsdk.Server, svc knowledgeService) { } results, err := svc.HybridSearch(ctx, in.KBID, &sdk.SearchParams{ QueryText: in.Query, + MatchCount: limit, VectorThreshold: in.VectorThreshold, KeywordThreshold: in.KeywordThreshold, }) diff --git a/cli/internal/mcp/tools_test.go b/cli/internal/mcp/tools_test.go index c10397bd..99a8e622 100644 --- a/cli/internal/mcp/tools_test.go +++ b/cli/internal/mcp/tools_test.go @@ -334,6 +334,19 @@ func TestTool_SearchChunks_LimitCap(t *testing.T) { } } +// TestTool_SearchChunks_PassesMatchCountFromLimit is a regression guard for +// the v0.5 audit bug: the search_chunks dispatch built SearchParams without +// setting MatchCount, so the server fell back to its default cap and the +// client-side trim (results[:limit]) was a no-op when limit > server default. +// Verifies the limit arg is threaded into SearchParams.MatchCount. +func TestTool_SearchChunks_PassesMatchCountFromLimit(t *testing.T) { + svc := &fakeSvc{} + c, _ := newTestServer(t, svc) + callTool(t, c, "search_chunks", map[string]any{"kb_id": "kb_x", "query": "test", "limit": 50}, nil) + require.NotNil(t, svc.calls.hybridParams, "HybridSearch must be called with non-nil SearchParams") + assert.Equal(t, 50, svc.calls.hybridParams.MatchCount, "MCP search_chunks must thread limit into SearchParams.MatchCount") +} + func TestTool_Chat_AccumulateAnswerAndReferences(t *testing.T) { svc := &fakeSvc{ kbStreamEvents: []*sdk.StreamResponse{