fix: code review fixes and MCP endpoint corrections#136
Merged
Conversation
Add shared auth context resolution, WebSocket chat dialer, MCP server toolset filtering, and supporting test coverage.
Delete the unused mcpConfig struct in wizard/steps.go. mcpServerConfig (still used at line 181) is preserved.
Log a warning when a header value matches the pattern ", Key: " suggesting the user may have passed multiple headers in a single --header flag instead of repeating the flag.
When auth.GetValidToken fails (e.g. token refresh network error), return the wrapped error instead of discarding it and returning the generic ErrNoToken sentinel.
Use cobra's Changed bit to distinguish "user didn't pass -o" from "user explicitly passed -o json", so nullify status shows a table by default while nullify status -o json still works. Also distinguish ErrNoToken from real credential errors in the status command.
- Distinguish ErrNoToken from real credential errors in chat, mcp, and ci commands so users see actionable messages on token refresh failures. - Add sync.Mutex to ci report's goroutine stderr writes, matching the existing pattern in ci gate. - Use ExitNetworkError instead of os.Exit(1) for WebSocket failures in the chat command.
… exposure - metrics/overview and metrics/over-time: switch from GET to POST with sensible default request bodies (sort by false positives, date range computed from period param). Fixes 405 Method Not Allowed errors. - get_security_trends: update composite tool to use POST for both underlying metrics calls. - list_dependencies: fix endpoint path from /context/dependencies to /context/deps to match the actual API route. - get_dependency_exposure: add required ecosystem and name parameters to the MCP tool schema. Fixes 400 Bad Request error.
- nullify://posture resource: switch from doGet to doPost with metricsOverviewBody(), fixing 405 errors for MCP resource reads. - list_dependencies: replace unsupported repository/limit params with pageSize/cursor to match the /context/deps API contract. Use a custom handler since makeGetHandler only serializes numeric args for "limit". - metricsOverTimeBody: add isArchived=false filter to match metricsOverviewBody, ensuring consistent filtering when combined in get_security_trends.
tim-thacker-nullify
approved these changes
Mar 23, 2026
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.
Summary
Code review fixes
Changedbit sonullify statusshows a table by default while-o jsonstill works--headervalue looks like it contains multiple headers in one flagauth.GetValidTokeninstead of discarding it as genericErrNoTokensync.Mutextoci reportgoroutine stderr writes, matchingci gateExitNetworkErrorfor WebSocket failures instead ofos.Exit(1)mcpConfigstructresolveCommandAuthhelper for chat/MCP commandsMCP endpoint fixes
/context/deps), replace unsupported params withpageSize/cursorecosystemandnameparametersisArchived=falsefilter for consistencyTest plan
go test ./cmd/cli/cmd/... ./internal/lib/... ./internal/wizard/...passesgo test ./internal/chat/... ./internal/mcp/...passesgo vet ./...cleannullify statusshows table,nullify status -o jsonshows JSON