Skip to content

fix: code review fixes and MCP endpoint corrections#136

Merged
fyxme merged 8 commits intomainfrom
fix/codex-review-issues
Mar 23, 2026
Merged

fix: code review fixes and MCP endpoint corrections#136
fyxme merged 8 commits intomainfrom
fix/codex-review-issues

Conversation

@fyxme
Copy link
Member

@fyxme fyxme commented Mar 23, 2026

Summary

Code review fixes

  • Status defaults to table: Use cobra's Changed bit so nullify status shows a table by default while -o json still works
  • Header value warning: Log a warning when a --header value looks like it contains multiple headers in one flag
  • Token error preservation: Return the real error from auth.GetValidToken instead of discarding it as generic ErrNoToken
  • CI report mutex: Add sync.Mutex to ci report goroutine stderr writes, matching ci gate
  • Chat exit code: Use ExitNetworkError for WebSocket failures instead of os.Exit(1)
  • Dead code removal: Delete unused mcpConfig struct
  • Runtime auth: Add shared resolveCommandAuth helper for chat/MCP commands

MCP endpoint fixes

  • metrics/overview & metrics/over-time: Switch from GET to POST with request bodies, fixing 405 errors
  • get_security_trends: Update composite tool to POST for both metrics calls
  • list_dependencies: Fix path (/context/deps), replace unsupported params with pageSize/cursor
  • get_dependency_exposure: Add required ecosystem and name parameters
  • nullify://posture resource: Switch to POST, fixing 405 on resource reads
  • metricsOverTimeBody: Add isArchived=false filter for consistency

Test plan

  • go test ./cmd/cli/cmd/... ./internal/lib/... ./internal/wizard/... passes
  • go test ./internal/chat/... ./internal/mcp/... passes
  • go vet ./... clean
  • Manual: nullify status shows table, nullify status -o json shows JSON

fyxme added 8 commits March 22, 2026 21:37
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.
@fyxme fyxme added the patch Patch version updates (fixes) label Mar 23, 2026
@fyxme fyxme added this pull request to the merge queue Mar 23, 2026
Merged via the queue into main with commit 4702764 Mar 23, 2026
3 checks passed
@fyxme fyxme deleted the fix/codex-review-issues branch March 23, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Patch version updates (fixes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants