feat(web): add session diagnostic export - #1646
Conversation
🦋 Changeset detectedLatest commit: 106ead4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3631a0f555
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 106ead4e81
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| rawResponse: { | ||
| 200: { type: 'string', format: 'binary' }, | ||
| }, |
There was a problem hiding this comment.
Avoid using a binary 200 schema for JSON error replies
In the validation, missing-session, and too-large cases this route sends an errEnvelope without changing the status, so those failures are still 200 responses. Because the route registers the only 200 response schema as a binary string, Fastify will select that serializer for the envelope object instead of the JSON envelope shape that postZip() expects, so users can receive a malformed/opaque error rather than the intended SESSION_NOT_FOUND or FILE_TOO_LARGE code. Either send error envelopes with non-200 statuses or avoid installing a binary-only 200 serializer for the error path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
False positive for kap-server: schema.response never drives runtime serialization here. start.ts replaces both compilers with pass-throughs (app.setSerializerCompiler(() => (data) => JSON.stringify(data))), so error envelopes are serialized as plain JSON regardless of the binary 200 schema. This is covered by tests: test/sessions.test.ts asserts the missing-session path returns the 40401 envelope and the oversized-web-log path returns 40001 with details.
The response schema is only used for the OpenAPI document, and the served document is already accurate: openapi/transforms.ts rewrites this route's 200 response to describe both application/zip (binary) and application/json (error envelope) — see GET /openapi.json ("Session export archive or JSON error envelope").
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eb62178b3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try { | ||
| sessionLogSource = await openOptionalZipSource(sessionLogPath, input.signal); | ||
| if (input.request.includeGlobalLog === true && input.globalLogPath !== undefined) { | ||
| globalSource = await openOptionalZipSource(input.globalLogPath, input.signal); |
There was a problem hiding this comment.
Keep optional global-log failures from aborting export
When includeGlobalLog is true and the configured global log path exists but cannot be opened or is not a regular file (for example EACCES or EISDIR), this call rethrows through openOptionalZipSource and the whole session export fails. The global log is optional and the previous best-effort path omitted it on read failures, so a bad app log should not prevent users from downloading the session archive; catch non-abort global-log errors here and leave globalLogPath unset.
Useful? React with 👍 / 👎.
| export const exportSessionRequestSchema = z | ||
| .object({ |
There was a problem hiding this comment.
Accept an omitted export request body
Because the route installs validateBody with this schema, a POST /sessions/{id}/export that has no JSON body parses undefined and returns 40001 before the handler runs. Since web_log is optional and exporting does not otherwise require a body, direct REST clients or generated clients that omit the optional request body cannot download an archive unless they know to send {}; mirror the compact request schema by preprocessing undefined to {}.
Useful? React with 👍 / 👎.
Related Issue
No linked issue. The Web UI had no equivalent of the TUI diagnostic session export.
Problem
Web users could not download a session archive with browser-side diagnostics, which made failures in the REST/WebSocket/UI path difficult to investigate. Reusing the existing export code also required tighter resource ownership because active logs can grow or rotate while an archive is being written, and browser downloads buffer the final ZIP in memory.
What changed
/exportcommand andPOST /api/v1/sessions/{session_id}/export, returning either a ZIP or the existing JSON error envelope.Verification
pnpm --filter @moonshot-ai/agent-core-v2 exec vitest run test/app/sessionExport/sessionExport.test.ts(11 tests; repeated serially, including 320 files, append, abort, and size-limit cases)pnpm exec vitest run test/sessions.test.ts test/openapi.test.ts test/apiSurface.snapshot.test.tsinpackages/kap-server(58 tests)pnpm testinapps/kimi-web(497 tests)pnpm testinpackages/protocol(524 tests)pnpm exec oxlint --type-aware --quiet(0 errors)pnpm lint:domaininpackages/agent-core-v2git diff --checkChecklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.