test: stats display + MCP OAuth XSS prevention — 26 new tests

[anandgupta42]

What does this PR do?

Adds first-ever test coverage for two completely untested modules: the altimate-code stats CLI output formatter and the MCP OAuth callback server's HTML escaping (XSS prevention boundary).

1. displayStats and formatNumber — src/cli/cmd/stats.ts (14 new tests)

The altimate-code stats command had zero test coverage. formatNumber (module-private) converts token counts to human-readable K/M suffixes, and displayStats renders the full terminal output with box-drawing, tool usage bar charts, and cost summaries. Tests exercise these via the exported displayStats function with mock SessionStats objects. Coverage includes:

• formatNumber rendering: values under 1000, exactly 1000 (→ "1.0K"), 1500 (→ "1.5K"), 1M, 2.5M, and zero
• Cost display: NaN safety for zero costs, fractional cost rounding
• Tool usage: bar chart rendering with percentages, tool limit enforcement, empty tool list omission, long tool name truncation
• Overview section: session/message counts, box-drawing borders

2. escapeHtml + HTTP behavior — src/mcp/oauth-callback.ts (12 new tests)

The OAuth callback server renders error messages from external MCP servers in an HTML page. The escapeHtml function (module-private) is the XSS prevention boundary — if it fails, a malicious MCP server could inject scripts via error_description. Tests exercise the server at the HTTP level (since escapeHtml is not exported). Coverage includes:

• XSS prevention: <script> tag escaping, HTML entity escaping (&, <, >, "), <img onerror> vector, event handler injection via double-quote breakout
• HTTP behavior: 404 for non-callback paths, 400 for missing state parameter (CSRF protection), 400 for missing auth code, HTML content-type headers, invalid state rejection
• Server lifecycle: isRunning after start/stop, idempotent ensureRunning

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage from automated test discovery

How did you verify your code works?

bun test test/cli/stats.test.ts          # 14 pass, 27 assertions
bun test test/mcp/oauth-callback.test.ts # 12 pass, 20 assertions

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Summary by CodeRabbit

• Tests
  • Added comprehensive test coverage for CLI stats output formatting, token/cost rendering, and tool usage display.
  • Added test suite for OAuth callback server functionality, including XSS prevention validation and HTTP error handling.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Two new test suites added: one testing CLI stats output formatting (token counts, costs, tool usage, overview sections) and another testing MCP OAuth callback HTTP server behavior (XSS prevention, HTTP status codes, server lifecycle). Total 322 lines of test coverage.

Changes

Cohort / File(s)	Summary
CLI Stats Testing packages/opencode/test/cli/stats.test.ts	New test suite validating stats display output: token count suffixing rules (K/M), cost rendering with two decimal places and zero-value safety, tool usage section presence/formatting and name truncation, and overview formatting with comma-separated counts and box-drawing characters.
OAuth Callback Server Testing packages/opencode/test/mcp/oauth-callback.test.ts	New test suite exercising MCP OAuth callback server via network requests: XSS prevention through HTML escaping of error_description query parameters, HTTP behavior for invalid paths (404) and missing parameters (400), and server lifecycle state management via ensureRunning() and stop().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

contributor

Poem

🐰 A hop, a test, a coverage quest!
Token counts and XSS protected best,
With commas placed and suffixes tight,
These new tests make the bunny's heart light! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: adding 26 new tests for stats display and MCP OAuth XSS prevention.
Description check	✅ Passed	The description covers all required template sections with comprehensive details about what changed, how it was tested, and includes a complete checklist with items marked.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/hourly-20260327-2213

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (4)

packages/opencode/test/mcp/oauth-callback.test.ts (2)

102-106: Tighten the content-type assertion with an explicit status check.

Right now this can pass even if the server returns an unexpected non-400 HTML response. Add a status assertion to bind the test to the intended error path.

Suggested patch

 test("returns HTML content type for error pages", async () => {
   const url = `${BASE_URL}?error=access_denied&error_description=test&state=test-state`
   const res = await fetch(url)
+  expect(res.status).toBe(400)
   expect(res.headers.get("content-type")).toContain("text/html")
 })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/mcp/oauth-callback.test.ts` around lines 102 - 106, In
the test "returns HTML content type for error pages" in oauth-callback.test.ts,
add an explicit status assertion against the fetched response (res) — e.g.,
expect(res.status).toBe(400) — so the test verifies both the content-type and
that the endpoint returned the intended error status for the error query path;
keep the existing URL construction (BASE_URL with error and state) and
content-type assertion intact.

---

132-136: ensureRunning idempotency test is a bit weak; assert behavior after repeated calls.

Line 132 currently only checks isRunning() === true, which would also pass if the second call had side effects but the server remained up. Consider verifying callback behavior after repeated ensureRunning() invocations.

Suggested patch

 test("ensureRunning is idempotent", async () => {
   // Server already running from beforeEach
   await McpOAuthCallback.ensureRunning()
+  await McpOAuthCallback.ensureRunning()
   expect(McpOAuthCallback.isRunning()).toBe(true)
+  const res = await fetch(`${BASE_URL}?state=some-state`)
+  expect(res.status).toBe(400)
 })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/mcp/oauth-callback.test.ts` around lines 132 - 136,
Update the idempotency test to call McpOAuthCallback.ensureRunning() twice and
verify behavior afterwards by (1) asserting McpOAuthCallback.isRunning() is
true, and (2) making an actual request to the OAuth callback endpoint (e.g., via
supertest/fetch) both before and after the second ensureRunning() call and
asserting the responses are successful and identical; this proves no duplicate
servers or changed behavior were created by the second call. Use
McpOAuthCallback.ensureRunning and McpOAuthCallback.isRunning to locate the
subject under test and keep assertions limited to response status/body equality
and lack of errors.

packages/opencode/test/cli/stats.test.ts (2)

137-145: Tool limit test could verify more exclusions.

The test correctly verifies glob (5th by count) is excluded when toolLimit=2, but doesn't verify that bash (3rd) and edit (4th) are also absent. This would make the test more robust against off-by-one errors in the limiting logic.

💡 More comprehensive assertion

     // Only top 2 tools should appear (read and write by count)
     expect(out).toContain("read")
     expect(out).toContain("write")
+    expect(out).not.toContain("bash")
+    expect(out).not.toContain("edit")
     expect(out).not.toContain("glob")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/cli/stats.test.ts` around lines 137 - 145, Update the
test that checks tool limiting (in the test using emptyStats(), captureOutput(),
and displayStats(stats, 2)) to also assert that the 3rd and 4th tools are
excluded: after calling displayStats(stats, 2) assert that output does not
contain "bash" and "edit" in addition to "glob", ensuring only the top two tools
("read" and "write") appear; keep existing checks for presence of "read" and
"write" and add negative assertions for "bash" and "edit".

---

31-46: Consider adding explicit type annotation for compile-time safety.

The emptyStats function returns an object literal matching SessionStats, but without an explicit type annotation. If the SessionStats interface changes (e.g., a new required field is added), these tests will fail at runtime rather than at compile time.

💡 Suggested improvement

+import type { SessionStats } from "../../src/cli/cmd/stats"
+
 /** Minimal valid SessionStats — all zeroes. */
-function emptyStats() {
+function emptyStats(): SessionStats {
   return {
     totalSessions: 0,
     // ... rest unchanged
   }
 }

This requires SessionStats to be exported from the stats module. If it's not currently exported, the current approach is acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/cli/stats.test.ts` around lines 31 - 46, The helper
emptyStats should have an explicit return type to catch future interface changes
at compile time: annotate the function as returning SessionStats (i.e., change
function emptyStats() to function emptyStats(): SessionStats) and ensure the
SessionStats type is imported from or exported by the stats module if it isn’t
already, so the test will fail at compile time whenever SessionStats no longer
matches the literal returned by emptyStats.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/cli/stats.test.ts`:
- Around line 137-145: Update the test that checks tool limiting (in the test
using emptyStats(), captureOutput(), and displayStats(stats, 2)) to also assert
that the 3rd and 4th tools are excluded: after calling displayStats(stats, 2)
assert that output does not contain "bash" and "edit" in addition to "glob",
ensuring only the top two tools ("read" and "write") appear; keep existing
checks for presence of "read" and "write" and add negative assertions for "bash"
and "edit".
- Around line 31-46: The helper emptyStats should have an explicit return type
to catch future interface changes at compile time: annotate the function as
returning SessionStats (i.e., change function emptyStats() to function
emptyStats(): SessionStats) and ensure the SessionStats type is imported from or
exported by the stats module if it isn’t already, so the test will fail at
compile time whenever SessionStats no longer matches the literal returned by
emptyStats.

In `@packages/opencode/test/mcp/oauth-callback.test.ts`:
- Around line 102-106: In the test "returns HTML content type for error pages"
in oauth-callback.test.ts, add an explicit status assertion against the fetched
response (res) — e.g., expect(res.status).toBe(400) — so the test verifies both
the content-type and that the endpoint returned the intended error status for
the error query path; keep the existing URL construction (BASE_URL with error
and state) and content-type assertion intact.
- Around line 132-136: Update the idempotency test to call
McpOAuthCallback.ensureRunning() twice and verify behavior afterwards by (1)
asserting McpOAuthCallback.isRunning() is true, and (2) making an actual request
to the OAuth callback endpoint (e.g., via supertest/fetch) both before and after
the second ensureRunning() call and asserting the responses are successful and
identical; this proves no duplicate servers or changed behavior were created by
the second call. Use McpOAuthCallback.ensureRunning and
McpOAuthCallback.isRunning to locate the subject under test and keep assertions
limited to response status/body equality and lack of errors.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cbfe5a8e-71b4-426a-a2b1-70ba5e725074

📥 Commits

Reviewing files that changed from the base of the PR and between 9ba2114 and 01cd4b2.

📒 Files selected for processing (2)

• packages/opencode/test/cli/stats.test.ts
• packages/opencode/test/mcp/oauth-callback.test.ts

[anandgupta42]

Consolidated into #545