Skip to content

feat(observability): serve session traces to the IDE extension - #1415

Merged
saravmajestic merged 2 commits into
mainfrom
feat/serve-traces-to-ide
Oct 8, 2026
Merged

saravmajestic merged 2 commits into
mainfrom
feat/serve-traces-to-ide

Conversation

@saravmajestic

@saravmajestic saravmajestic commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

The IDE extension's chat has no /traces: users who rely on it in the TUI cannot view a session's trace from VS Code.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

/traces exists only in the TUI (plugin/tui/altimate/trace-viewer.tsx). The VS Code chat runs altimate serve headless and builds its slash menu from GET /command, so it could never list or open a trace. This adds two routes for it:

  • GET /altimate/trace?offset&limit — a page of traces (newest first, default 50) with title, status, duration, tokens, cost and tool calls. Uses Trace.listTracesPaginated.
  • GET /altimate/trace/:sessionID/view — the trace's viewer page from renderTraceViewer.

Both read tracing.dir like the CLI and TUI. The session ID must match [A-Za-z0-9_-] before it names a file (../../etc → 400). Trace content includes prompts and tool output, so browser-originated requests are refused on the same terms as the workspace routes. That check moved from workspaceRouteRefusal into browserOriginRefusal so both can use it; the workspace messages are byte-identical.

renderTraceViewer gets an embedded option that leaves out Copy Link: inside an editor tab the page URL is an internal webview URL. The Copy Link handler is null-guarded so the rest of the viewer script still runs.

The extension's /traces (quick pick + editor-tab viewer) consumes these routes; an older CLI without them gets an "update Altimate Code" reply there.

How did you verify your code works?

  • test/server/altimate-trace-routes.test.ts (8 tests: list order and fields, paging, empty, view page, 404, path-like ID → 400, cross-site and foreign-origin refusals). Existing workspace, skill-publish and tracing-viewer tests pass; tsgo --noEmit clean.
  • serve from this branch against 89 real local traces: list total 89, viewer 200, unknown ID 404, ../../etc 400, Origin: https://evil.example 403.
  • Built linux-arm64, ran it in the extension's code-server container: /traces lists traces and the viewer renders and works (tabs, Share Trace download).

Screenshots / recordings

Routes only; the UI lives in the IDE extension.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

🤖 Generated with Claude Code


Summary by cubic

Adds /traces support to the IDE extension by serving session traces over HTTP, since VS Code runs altimate serve headless and can't use the TUI's /traces command.

  • GET /altimate/trace returns a paginated list of traces, newest first, with title, status, duration, tokens, cost, and tool calls; page size defaults to 50 and caps at 200.
  • GET /altimate/trace/:sessionID/view serves one trace's viewer page, honoring tracing.dir like the CLI and TUI.
  • Both routes validate the session ID before it names a file and refuse browser-originated requests; the refusal logic moved from workspaceRouteRefusal into a shared browserOriginRefusal helper with unchanged workspace messages.
  • Trace.listTraces skips a .json that parses but isn't trace-shaped, so a foreign file in a user-set tracing.dir no longer fails the listing.
  • renderTraceViewer gains an embedded option that omits the Copy Link button, since an editor-tab webview URL isn't shareable.

Written for commit 85656db. Summary will update on new commits.

Review in cubic Turn on auto-fix

Summary by CodeRabbit

  • New Features
    • Added trace browsing with a paginated summary and a detailed viewer for individual sessions.
    • Embedded trace viewers omit the Copy Link button.
    • Trace listings support pages of up to 200 entries and display a title based on the prompt when one is unavailable.
  • Bug Fixes
    • Invalid or unrelated JSON files are skipped in trace listings.
  • Security
    • Trace pages reject disallowed cross-site and browser-originated requests.

The TUI's `/traces` is a TUI-only command, so the VS Code chat (which runs
`altimate serve` headless) had no way to list or open a trace.

- `GET /altimate/trace?offset&limit`: a page of session traces, newest first,
  with each trace's title, status, duration, tokens, cost and tool calls
- `GET /altimate/trace/:sessionID/view`: one trace's self-contained viewer page
- Both honor `tracing.dir`, validate the session ID before it names a file, and
  refuse browser origins on the same terms as the workspace routes
- Extract `browserOriginRefusal` from `workspaceRouteRefusal` so both share it;
  workspace messages are unchanged
- `renderTraceViewer({ embedded: true })` leaves out Copy Link, whose URL is not
  shareable inside an editor tab

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2a4e87c5-5a08-4e7d-9ca4-61190ab42b67
📥 Commits

Reviewing files that changed from the base of the PR and between 8a54726 and 85656db.

📒 Files selected for processing (4)
  • packages/opencode/src/altimate/observability/tracing.ts
  • packages/opencode/src/server/server.ts
  • packages/opencode/test/server/altimate-trace-routes.test.ts
  • packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The server adds paginated trace-listing and trace-viewer routes. Both use shared browser-origin checks. Trace listings skip JSON files that do not match the trace shape. The trace viewer supports embedded rendering, which omits the Copy Link button.

Changes

Trace Routes and Viewer

Layer / File(s) Summary
Shared browser-origin checks
packages/opencode/src/server/server.ts
The workspace route delegates browser-origin checks to a shared helper. The helper applies subject-specific refusals based on fetch-site, origin, host, and password values.
Trace validation and directory setup
packages/opencode/src/altimate/observability/tracing.ts, packages/opencode/src/server/server.ts, packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts
Trace listing skips parsed JSON that does not have the required trace fields. Trace-directory configuration failures fall back to the default directory. The adversarial test fixture now includes a trace summary.
Trace routes and embedded viewer
packages/opencode/src/server/server.ts, packages/opencode/src/altimate/observability/viewer.ts, packages/opencode/test/server/altimate-trace-routes.test.ts
The server adds paginated trace summaries and a session viewer route with input validation and error responses. The viewer omits Copy Link when embedded. Tests cover listing, pagination, viewer output, and refusal cases.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant Server
  participant TraceDirectory
  participant renderTraceViewer
  Browser->>Server: Request trace summaries or a session viewer
  Server->>TraceDirectory: List paginated traces or load a session trace
  TraceDirectory-->>Server: Return summaries or trace data
  Server->>renderTraceViewer: Render trace with embedded mode enabled
  renderTraceViewer-->>Browser: Return viewer HTML
Loading

Suggested reviewers: anandgupta42

Merge Risk: 🟠 High · up to 85656

On an unpassworded network-bound server, clients can retrieve trace prompts and tool output; list requests also scan the entire archive, risking slowdown as it grows. Restrict trace access before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: serving session traces to the IDE extension.
Description check ✅ Passed The description covers the issue, feature type, implementation, verification, screenshots context, and checklist. The issue section does not include a linked issue number, but the description is other…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit reads the trace list bright,
It hops through pages, left to right.
Embedded views hide Copy Link,
While valid traces pass the check.
The browser finds its viewer there,
And leaves a neat, clean trail to share.

Comment @coderabbitai help to get the list of available commands.

// session traces and one trace's self-contained viewer page. Trace content carries prompts and
// tool output, so a browser origin is refused on the same terms as the workspace routes.
.get("/altimate/trace", async (c) => {
const refused = browserOriginRefusal(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Protect trace contents when the server has no password

browserOriginRefusal permits requests without Origin/Sec-Fetch-Site, and the global Basic-auth middleware also permits them when OPENCODE_SERVER_PASSWORD is unset. serve can bind to 0.0.0.0 (including via mDNS), so an unauthenticated network client can list all historical trace IDs here and fetch their viewer pages, including prompts and tool output, without any browser headers. These traces live in a shared directory, not just the requested project. Require authentication or otherwise restrict both new routes to a trusted local caller; origin headers alone cannot authenticate a raw HTTP client.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one; these routes add no new exposure. serve binds to 127.0.0.1 unless --hostname or --mdns is passed, and it prints "server is unsecured" when OPENCODE_SERVER_PASSWORD is unset. On a server reachable without a password, the same raw client can already read every prompt and all tool output via GET /session/:sessionID/message and /session/:sessionID/transcript, and can open a shell via /pty. A trace-only restriction would not protect that data. The origin check here closes the one new vector these routes would add: a web page reading traces through the browser.

if (refused) return c.json(refused.body, refused.status)
try {
const { Trace } = await import("../altimate/observability/tracing")
const page = await Trace.listTracesPaginated(await tracesDir(), {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Pagination still parses every trace file on every list request

Trace.listTracesPaginated first calls Trace.listTraces, which reads and JSON-parses every .json file (including all recorded tool I/O) and sorts the complete result before slicing. Thus even the default 50-row request costs I/O and memory proportional to the entire trace archive; limit also has no upper bound, allowing a single request to serialize all titles/prompts. With multi-MB traces or frequent quick-pick refreshes this can stall the headless server. Use bounded metadata/index reads for listing and cap the HTTP page size.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partly fixed in 85656db: limit is now capped at 200, so one request cannot serialize the whole archive. The parse-every-file cost is unchanged. It is the same Trace.listTraces path the TUI /traces dialog and trace list already run on every open, and tracing.maxFiles (default 100, with pruning) bounds it. A summary-only reader needs an index or sidecar written by the tracer, which changes the trace writer, so it belongs in its own PR rather than this one.

@kilo-code-bot

kilo-code-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
packages/opencode/src/server/server.ts 1281 Without a server password, origin-less network clients can enumerate and view historical traces from the shared traces directory, including prompts and tool output.

WARNING

File Line Issue
packages/opencode/src/server/server.ts 1291 Each list request still reads and parses every trace before pagination; the new 200-row response cap does not bound full-archive I/O and memory use.
Files Reviewed (4 files)
  • packages/opencode/src/altimate/observability/tracing.ts - 0 issues
  • packages/opencode/src/server/server.ts - 2 issues
  • packages/opencode/test/server/altimate-trace-routes.test.ts - 0 issues
  • packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts - 0 issues

Fix these issues in Kilo Cloud

Previous Review Summary (commit 8a54726)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 8a54726)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
packages/opencode/src/server/server.ts 1280 Passwordless network clients can read shared historical traces and sensitive prompts/tool output.

WARNING

File Line Issue
packages/opencode/src/server/server.ts 1290 Listing a page parses the entire trace archive; page size is unbounded.
Files Reviewed (3 files)
  • packages/opencode/src/altimate/observability/viewer.ts - 0 issues
  • packages/opencode/src/server/server.ts - 2 issues
  • packages/opencode/test/server/altimate-trace-routes.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by gpt-6-sol · Input: 38 · Output: 19K · Cached: 1.7M

Review guidance: REVIEW.md from base branch main

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/server/server.ts">

<violation number="1" location="packages/opencode/src/server/server.ts:1279">
P1: Require credentials or restrict both trace routes to trusted local callers; `browserOriginRefusal` accepts requests without `Origin`, allowing unauthenticated network clients to retrieve prompts and tool output when password auth is disabled.</violation>

<violation number="2" location="packages/opencode/src/server/server.ts:1290">
P2: `Trace.listTracesPaginated` reads and parses every full JSON trace before slicing. With `tracing.maxFiles: 0`, each page request can load an unbounded history into memory; use a summary-only paginated reader.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

// The TUI's `/traces` for the IDE extension, which runs this CLI headless: a page of the
// session traces and one trace's self-contained viewer page. Trace content carries prompts and
// tool output, so a browser origin is refused on the same terms as the workspace routes.
.get("/altimate/trace", async (c) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Require credentials or restrict both trace routes to trusted local callers; browserOriginRefusal accepts requests without Origin, allowing unauthenticated network clients to retrieve prompts and tool output when password auth is disabled.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/src/server/server.ts, line 1279:

<comment>Require credentials or restrict both trace routes to trusted local callers; `browserOriginRefusal` accepts requests without `Origin`, allowing unauthenticated network clients to retrieve prompts and tool output when password auth is disabled.</comment>

<file context>
@@ -1249,6 +1272,77 @@ export namespace Server {
+      // The TUI's `/traces` for the IDE extension, which runs this CLI headless: a page of the
+      // session traces and one trace's self-contained viewer page. Trace content carries prompts and
+      // tool output, so a browser origin is refused on the same terms as the workspace routes.
+      .get("/altimate/trace", async (c) => {
+        const refused = browserOriginRefusal(
+          "Traces",
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the reply on the kilo thread above: on an unsecured server these routes expose nothing that /session/:sessionID/message, /session/:sessionID/transcript and /pty do not already. serve binds to loopback by default and warns when no password is set. The origin check covers the browser vector specific to these routes.

if (refused) return c.json(refused.body, refused.status)
try {
const { Trace } = await import("../altimate/observability/tracing")
const page = await Trace.listTracesPaginated(await tracesDir(), {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Trace.listTracesPaginated reads and parses every full JSON trace before slicing. With tracing.maxFiles: 0, each page request can load an unbounded history into memory; use a summary-only paginated reader.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/src/server/server.ts, line 1290:

<comment>`Trace.listTracesPaginated` reads and parses every full JSON trace before slicing. With `tracing.maxFiles: 0`, each page request can load an unbounded history into memory; use a summary-only paginated reader.</comment>

<file context>
@@ -1249,6 +1272,77 @@ export namespace Server {
+        if (refused) return c.json(refused.body, refused.status)
+        try {
+          const { Trace } = await import("../altimate/observability/tracing")
+          const page = await Trace.listTracesPaginated(await tracesDir(), {
+            offset: Number(c.req.query("offset") ?? 0),
+            limit: Number(c.req.query("limit") ?? TRACE_PAGE_SIZE),
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the thread above: the page size is capped at 200 in 85656db. A summary-only reader needs the tracer to write an index, which is a separate change. With tracing.maxFiles: 0 the user has opted out of pruning, and the TUI and trace list load the same full history today.

@sahrizvi sahrizvi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

The new routes are well scoped. browserOriginRefusal is extracted cleanly with byte-identical workspace messages, the session-ID allowlist runs before any path is built, and the embedded Copy Link removal is null-guarded so the rest of the viewer script still runs. The 8 new tests pass locally.

One MAJOR issue is inline (the list returns 500 on a parseable non-trace .json). The rest is below.

Minor

1. Unbounded limit, and every request parses every trace file (Performance) — server.ts:1290-1293, tracing.ts:1477-1526
?limit=1e9 is accepted and echoed back as 1000000000. listTracesPaginated reads and JSON.parses every trace in full (prompts and tool I/O) before slicing, on every quick-pick open, and with tracing.maxFiles: 0 that's unbounded. Clamp limit at the route (e.g. ≤ 200). That only bounds the response; the full-directory read needs a summary-only/index reader, which can be a follow-up.

2. The IDE viewer is a static snapshot, but the TUI opens it live (Logic) — server.ts:1338
This route renders renderTraceViewer(trace, { embedded: true }), while the TUI uses { live: true, apiPath: "/api/" + sid } (plugin/tui/altimate/trace-viewer.tsx:70). A trace opened from the IDE during a running session keeps its initial spans and status until reopened. Either add a guarded JSON route (e.g. GET /altimate/trace/:sessionID) and render with live: true plus that apiPath, or document that the view is snapshot-only.

3. The origin guard doesn't authenticate GETs, and the route comment overstates it (Security; existing trust model) — server.ts:117-133, 1276-1279
With OPENCODE_SERVER_PASSWORD unset, an origin-less native client (curl) gets every trace across all projects. So does a same-origin browser GET (sec-fetch-site: same-origin with no Origin returns 200), e.g. via DNS rebinding, since there's no Host allowlist. An unsecured server already serves overlapping data (prompts, responses, tool output) via GET /session/:id/message and accepts prompts that run tools, so these routes add no new capability. But the comment "refused on the same terms as the workspace routes" is misleading for GET: the workspace routes are POSTs, and a browser always sends Origin on a POST. Soften the comment, or, when serve is bound to loopback, add a Host check (localhost/127.0.0.1/[::1]) for the local-only /altimate/* GETs. A blanket Host check would break non-loopback/mDNS binding.

4. List titles fall back to the full, uncapped prompt — server.ts:1301
title: trace.metadata.title || trace.metadata.prompt || sessionId sends the whole prompt for each of up to 50 rows. metadata.prompt is stored uncapped (setPrompt, tracing.ts:750); the 4000-char USER_MESSAGE_INPUT_MAX_CHARS cap applies only to user-message spans. The TUI truncates to 80 chars (cleanTitle(rawTitle).slice(0, 80)), so truncate server-side too.

Nits

  • Invalid limit falls back to 20, not 50 (server.ts:1291-1292). Number("abc") gives NaN, which hits the helper's default of 20, and limit= gives 0, which is clamped to 1. Parse at the route and fall back to TRACE_PAGE_SIZE.
  • tracesDir() swallows config errors silently (server.ts:138-145). The fallback is right and matches the TUI, but a log.warn in the catch would make a wrong directory diagnosable.
  • No Cache-Control: no-store on trace responses (server.ts:1294, 1338). It's defence-in-depth for prompt and tool-output content.
  • The 400 echoes the raw sessionID (server.ts:1327). It's JSON, so not an XSS risk, but a fixed message is enough.

Missing tests

  • A parseable non-trace or partial .json in the traces dir (the inline MAJOR).
  • With a password set: same-origin Origin allowed, foreign origin refused, Basic-auth path on the trace routes.
  • sec-fetch-site: same-origin / none allowed and same-site refused (only cross-site is covered, and only on the list route).
  • Invalid or huge offset / limit.
  • tracesDir() fallback when Config.get rejects.

limit: page.limit,
traces: page.traces.map(({ sessionId, trace }) => ({
sessionID: sessionId,
title: trace.metadata.title || trace.metadata.prompt || sessionId,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR · Bug — one parseable non-trace .json in the traces dir makes this route return 500

Trace.listTraces (tracing.ts:1484-1493) only skips files that fail JSON.parse. Any file that parses but has no metadata/summary reaches this mapping, and trace.metadata.title (and trace.summary.* below) throws. Then the whole list fails, not just that entry. Such a file can be a foreign file in a user-set tracing.dir, or an older or partial trace.

Reproduced against this branch with one valid trace plus notes.json = {"hello":1}:

500 {"ok":false,"error":"undefined is not an object (evaluating 'trace.metadata.title')"}

The TUI list already guards this case (plugin/tui/altimate/trace-viewer.tsx:162-167: item.trace.metadata ?? {}, summary?.status).

Suggestion: filter out entries that aren't trace-shaped instead of optional-chaining them, so a foreign file isn't listed as a trace:

traces: page.traces
  .filter(({ trace }) => trace && typeof trace.metadata === "object" && typeof trace.summary === "object")
  .map(({ sessionId, trace }) => ({ /* … */ })),

total would ideally be computed after the same filter, so better still do it inside listTraces. Please also add a route test with one valid trace and one parseable non-trace file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 85656db. The guard went into Trace.listTraces, as you suggested: a .json that parses but lacks sessionId, startedAt, metadata or summary is skipped like a corrupted file. total counts only traces, and trace list no longer crashes on such a file either. I checked 89 real local traces, including a running one; all have both metadata and summary, since the writer always builds them. New route test: one valid trace plus notes.json = {"hello":1} → 200, total: 1. The v0.5.20 pagination fixture had no summary, so it now gets the one every written trace has. Those tests pin offset/limit clamping, not trace shape.

…s page size

- `Trace.listTraces` skips a `.json` that parses but is not trace-shaped (no
  `sessionId`, `startedAt`, `metadata` or `summary`), like a corrupted one. A
  foreign file in a user-set `tracing.dir` made `GET /altimate/trace` (and
  `trace list`) fail with a 500; `total` now counts only traces
- `GET /altimate/trace` caps `limit` at 200
- The v0.5.20 pagination fixture gains the `summary` every written trace has

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/test/server/altimate-trace-routes.test.ts">

<violation number="1" location="packages/opencode/test/server/altimate-trace-routes.test.ts:104">
P2: This test does not exercise the cap: it writes only one trace and checks the echoed `limit`, so returning every matching trace would still pass. Seed more than 200 traces and assert that the response contains at most 200.</violation>
</file>

<file name="packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts">

<violation number="1" location="packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts:151">
P3: This fixture omits the required `summary.tokens` breakdown that the writer emits. Add the five token counters so pagination tests use representative trace data.</violation>
</file>

<file name="packages/opencode/src/altimate/observability/tracing.ts">

<violation number="1" location="packages/opencode/src/altimate/observability/tracing.ts:185">
P2: `summary: {}` passes this check, but `trace list` then calls `totalTokens.toLocaleString()` and formats `totalCost` with `.toFixed()`, crashing the command. Validate the summary fields consumed by the listing before accepting the file as a `TraceFile`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment on lines +104 to +108
test("caps the page size", async () => {
await writeTrace("ses_a", "2026-10-01T10:00:00.000Z")
const body = (await (await get("/altimate/trace?limit=100000")).json()) as Record<string, any>
expect(body.limit).toBe(200)
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test does not exercise the cap: it writes only one trace and checks the echoed limit, so returning every matching trace would still pass. Seed more than 200 traces and assert that the response contains at most 200.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/test/server/altimate-trace-routes.test.ts, line 104:

<comment>This test does not exercise the cap: it writes only one trace and checks the echoed `limit`, so returning every matching trace would still pass. Seed more than 200 traces and assert that the response contains at most 200.</comment>

<file context>
@@ -90,6 +90,23 @@ describe("GET /altimate/trace", () => {
+    expect(body.traces.map((t: { sessionID: string }) => t.sessionID)).toEqual(["ses_real"])
+  })
+
+  test("caps the page size", async () => {
+    await writeTrace("ses_a", "2026-10-01T10:00:00.000Z")
+    const body = (await (await get("/altimate/trace?limit=100000")).json()) as Record<string, any>
</file context>
Suggested change
test("caps the page size", async () => {
await writeTrace("ses_a", "2026-10-01T10:00:00.000Z")
const body = (await (await get("/altimate/trace?limit=100000")).json()) as Record<string, any>
expect(body.limit).toBe(200)
})
test("caps the page size", async () => {
await Promise.all(
Array.from({ length: 201 }, (_, i) =>
writeTrace(`ses_${i}`, new Date(Date.UTC(2026, 0, i + 1)).toISOString()),
),
)
const body = (await (await get("/altimate/trace?limit=100000")).json()) as Record<string, any>
expect(body.limit).toBe(200)
expect(body.traces).toHaveLength(200)
})

Comment on lines +185 to +186
typeof trace.summary === "object" &&
trace.summary !== null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: summary: {} passes this check, but trace list then calls totalTokens.toLocaleString() and formats totalCost with .toFixed(), crashing the command. Validate the summary fields consumed by the listing before accepting the file as a TraceFile.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/src/altimate/observability/tracing.ts, line 185:

<comment>`summary: {}` passes this check, but `trace list` then calls `totalTokens.toLocaleString()` and formats `totalCost` with `.toFixed()`, crashing the command. Validate the summary fields consumed by the listing before accepting the file as a `TraceFile`.</comment>

<file context>
@@ -172,6 +172,20 @@ export interface TraceExporter {
+    typeof trace.startedAt === "string" &&
+    typeof trace.metadata === "object" &&
+    trace.metadata !== null &&
+    typeof trace.summary === "object" &&
+    trace.summary !== null
+  )
</file context>
Suggested change
typeof trace.summary === "object" &&
trace.summary !== null
typeof trace.summary === "object" &&
trace.summary !== null &&
typeof trace.summary.totalTokens === "number" &&
typeof trace.summary.totalCost === "number" &&
typeof trace.summary.totalToolCalls === "number" &&
typeof trace.summary.duration === "number" &&
typeof trace.summary.status === "string"

spans: [],
metadata: { provider: "test", model: "test-model", directory: "/tmp" },
// Every trace the writer produces has a summary; listTraces skips files without one.
summary: { totalTokens: 0, totalCost: 0, totalToolCalls: 0, totalGenerations: 0, duration: 30_000, status: "completed" },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This fixture omits the required summary.tokens breakdown that the writer emits. Add the five token counters so pagination tests use representative trace data.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/test/skill/release-v0.5.20-adversarial.test.ts, line 151:

<comment>This fixture omits the required `summary.tokens` breakdown that the writer emits. Add the five token counters so pagination tests use representative trace data.</comment>

<file context>
@@ -147,6 +147,8 @@ describe("v0.5.20 release: listTracesPaginated adversarial", () => {
         spans: [],
         metadata: { provider: "test", model: "test-model", directory: "/tmp" },
+        // Every trace the writer produces has a summary; listTraces skips files without one.
+        summary: { totalTokens: 0, totalCost: 0, totalToolCalls: 0, totalGenerations: 0, duration: 30_000, status: "completed" },
       }
       await fs.writeFile(path.join(tmpDir, `${sessionId}.json`), JSON.stringify(trace))
</file context>
Suggested change
summary: { totalTokens: 0, totalCost: 0, totalToolCalls: 0, totalGenerations: 0, duration: 30_000, status: "completed" },
summary: {
totalTokens: 0,
totalCost: 0,
totalToolCalls: 0,
totalGenerations: 0,
duration: 30_000,
status: "completed",
tokens: { input: 0, output: 0, reasoning: 0, cacheRead: 0, cacheWrite: 0 },
},

@sahrizvi sahrizvi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 85656db: approving

The MAJOR is fixed. Trace.listTraces now skips any .json that parses but isn't trace-shaped, so total counts only real traces. The new route test covers the exact case (one valid trace plus notes.json → 200, total: 1). Every writer path goes through buildTraceFile, which always sets sessionId, startedAt, metadata and summary, so no real trace gets dropped by the new check. The limit cap works: 201 traces with ?limit=100000 returns exactly 200. The tracing suites (tracing*.test.ts, the v0.5.20/v0.8.4 adversarial files), the trace-route tests and the workspace-route tests all pass on this head.

The replies on the unauthenticated-access and parse-every-file threads hold up. On an unsecured server, /session/:id/message, /transcript and /pty already expose more than these routes do, and a summary-only reader needs a change to the trace writer, so a follow-up PR is the right place for it.

Still open (non-blocking, fine as follow-ups)

These are from the first review and are unchanged:

  • The IDE viewer doesn't refresh (server.ts:1340). The TUI opens the same viewer with live: true, so a trace opened from the IDE during a running session stays stale until reopened.
  • Route comment (server.ts:1277-1279). "Refused on the same terms as the workspace routes" overstates the GET case, because a same-origin browser GET sends no Origin. The reasoning in your thread reply is the accurate version and would make a better comment.
  • List titles fall back to the full prompt (server.ts:1303). metadata.prompt is stored uncapped, and 200 rows a page now makes this matter more. The TUI truncates to 80 chars.
  • Nits:
    • ?limit=abc falls back to the helper's 20, not TRACE_PAGE_SIZE (50).
    • tracesDir() swallows config errors without a log.warn.
    • There's no Cache-Control: no-store on trace responses.
    • The 400 echoes the raw sessionID.

New nits from this commit

  • loadTrace has no shape check (server.ts:1338). A non-trace file named like a session ID (e.g. ses_x.json containing {"hello":1}) still gets a 200 viewer page, which then breaks client-side. Reusing isTraceShaped there would return a 404 instead.
  • isTraceShaped accepts summary: {}. The route copes (that row just omits its fields), but the trace list CLI table calls totalTokens.toLocaleString() on it. trace list already crashed on any such file before this PR, so this isn't a regression.
  • The "caps the page size" test writes one trace and checks only the echoed limit. Seeding 201 traces and asserting traces.length === 200 would actually exercise the cap.

@saravmajestic
saravmajestic merged commit b9173a2 into main Oct 8, 2026
33 of 34 checks passed
@saravmajestic
saravmajestic deleted the feat/serve-traces-to-ide branch October 8, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants