feat(cli): add session export to markdown#51105
feat(cli): add session export to markdown#51105mvanhorn wants to merge 2 commits intoopenclaw:mainfrom
Conversation
🔒 Aisle Security AnalysisWe found 3 potential security issue(s) in this PR:
1. 🟡 Terminal escape/control sequence injection via session export to stdout
DescriptionThe
Vulnerable code: } else {
process.stdout.write(result);
}RecommendationOnly emit raw session content when stdout is not a TTY (piped/redirected), and sanitize/strip control sequences for interactive terminals. Example fix: import stripAnsi from "strip-ansi";
// ...
const isTty = Boolean(process.stdout.isTTY);
const safeForTerminal = isTty
? stripAnsi(result).replace(/[\u0000-\u001F\u007F-\u009F]/g, "")
: result;
process.stdout.write(safeForTerminal);Alternatively, add an explicit 2. 🔵 Markdown/HTML injection in session markdown export (potential XSS when rendered)
DescriptionThe If the exported
Vulnerable code: lines.push(text); // user/assistant/tool text emitted raw
...
lines.push(`<summary>Tool call: ${tc.name}</summary>`); // raw HTML with unescaped name
...
lines.push(`<summary>Tool result: ${msg.toolName}${msg.isError ? " (error)" : ""}</summary>`);Only triple backticks are escaped inside code fences; HTML special characters are not escaped/sanitized. RecommendationEscape/sanitize untrusted content before embedding it into Markdown/HTML. Option A (recommended): avoid raw HTML entirely and use pure Markdown constructs (headings, blockquotes, fenced code blocks) for tool calls/results. Option B: if keeping Example HTML escaping helper: function escapeHtml(s: string): string {
return s
.replace(/&/g, "&")
.replace(/</g, "<")
.replace(/>/g, ">")
.replace(/\"/g, """)
.replace(/'/g, "'");
}
// When outputting raw text:
lines.push(escapeHtml(text));
// When interpolating into HTML:
lines.push(`<summary>Tool call: ${escapeHtml(tc.name)}</summary>`);
lines.push(`<summary>Tool result: ${escapeHtml(msg.toolName)}${msg.isError ? " (error)" : ""}</summary>`);Also consider a 3. 🔵 Session export writes transcripts with default (potentially world-readable) file permissions
DescriptionThe
Vulnerable code: fs.writeFileSync(outputPath, result, "utf-8");RecommendationWrite exported transcripts with restrictive permissions (and consider creating directories with restrictive perms too). Example fix: if (!fs.existsSync(outputDir)) {
fs.mkdirSync(outputDir, { recursive: true, mode: 0o700 });
}
fs.writeFileSync(outputPath, result, { encoding: "utf-8", mode: 0o600 });This prevents other local users from reading exported transcripts by default. Analyzed PR: #51105 at commit Last updated on: 2026-03-20T16:33:05Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c623ed205f
ℹ️ 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".
| for (const block of msg.content) { | ||
| if (block.type === "text") { | ||
| textParts.push(block.text); |
There was a problem hiding this comment.
Handle legacy assistant string content
sessionEntriesToMarkdown calls extractAssistantText, but that helper assumes msg.content is an array of typed blocks and iterates it directly. Existing transcript readers in this repo already handle assistant content as either string or array (for example extractPreviewText in src/gateway/session-utils.fs.ts), so legacy sessions with string assistant content will export as empty assistant turns. This drops real conversation text from exported history and makes the new command unreliable for older transcripts.
Useful? React with 👍 / 👎.
| lines.push("```"); | ||
| lines.push(truncated); | ||
| lines.push("```"); |
There was a problem hiding this comment.
Escape code-fence delimiters in tool result export
Tool results are inserted verbatim inside a triple-backtick fence, but the payload is not escaped. If a tool output includes ``` (common when tools return markdown or code snippets), the fence closes early and the rest of the export is malformed or reinterpreted as markdown. This breaks the primary output format for realistic transcripts; use a safer fence strategy (e.g., dynamic fence length) or an escaping approach before writing tool text.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds A few issues to address before merging:
Confidence Score: 3/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 217
Comment:
**`console.log` bypasses runtime abstraction and test suppression**
`runtime.log` should be used here instead of `console.log`. Looking at `src/runtime.ts`, `runtime.log` wraps `console.log` with a `shouldEmitRuntimeLog()` guard that suppresses output when running under Vitest (unless `console.log` is mocked or `OPENCLAW_TEST_RUNTIME_LOG=1`). Calling `console.log` directly skips this gate, so any future test that invokes `sessionExportCommand` with `--output` will emit noisy output to the test console. The rest of this function already uses `runtime.error` and `runtime.exit` — using `console.log` for the success path breaks that consistency.
```suggestion
runtime.log(`Exported session "${sessionKey}" to ${outputPath}`);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 209-211
Comment:
**Fragile tilde expansion — use `os.homedir()`**
`output.replace("~", process.env.HOME ?? "")` has two problems:
1. `process.env.HOME` is `undefined` on Windows, falling back to `""`, which causes `path.resolve("")` to resolve to the current working directory rather than the home directory. `os.homedir()` from Node's built-in `os` module is portable and always returns a valid path.
2. `String.replace(string, string)` only replaces the first occurrence. For the typical `~/path` pattern this is fine in practice, but the idiomatic form uses a regex anchored to the start: `/^~/`.
```suggestion
const outputPath = path.resolve(
output.startsWith("~") ? output.replace(/^~/, os.homedir()) : output,
);
```
Also add `import os from "node:os";` at the top of the file.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 134-135
Comment:
**Tool call `<summary>` is missing the tool name**
Tool result summaries include the tool name (`Tool result: weather.get`), but tool call summaries just say `"Tool call"` — forcing the user to expand every `<details>` block to find out which tool was invoked. Since `extractAssistantText` already returns the tool name embedded inside the code string (`tc` starts with `${block.name}(`), it's straightforward to include the name in the summary for at-a-glance readability, consistent with how tool results are labelled.
One way to fix this is to change `extractAssistantText` to return structured objects instead of formatted strings:
```ts
// extractAssistantText returns { name: string; argsStr: string }[]
toolCalls.push({ name: block.name, argsStr: JSON.stringify(block.arguments, null, 2) });
```
Then in the rendering loop:
```ts
lines.push(`<summary>Tool call: ${tc.name}</summary>`);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/commands/session-export.test.ts
Line: 159-160
Comment:
**Truncation assertion is indirect and may give false confidence**
`expect(md.length).toBeLessThan(longText.length)` passes because the truncated content + markdown boilerplate (~650 chars) is less than 1000 chars. This will break silently if the boilerplate ever grows past 1000 chars. A more direct assertion would verify that the raw tool result text is actually truncated to 500 characters:
```suggestion
// Should not contain the full 1000 chars
expect(md).not.toContain("x".repeat(501));
expect(md).toContain("x".repeat(500) + "...");
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat(cli): add sessi..." |
| fs.mkdirSync(outputDir, { recursive: true }); | ||
| } | ||
| fs.writeFileSync(outputPath, result, "utf-8"); | ||
| console.log(`Exported session "${sessionKey}" to ${outputPath}`); |
There was a problem hiding this comment.
console.log bypasses runtime abstraction and test suppression
runtime.log should be used here instead of console.log. Looking at src/runtime.ts, runtime.log wraps console.log with a shouldEmitRuntimeLog() guard that suppresses output when running under Vitest (unless console.log is mocked or OPENCLAW_TEST_RUNTIME_LOG=1). Calling console.log directly skips this gate, so any future test that invokes sessionExportCommand with --output will emit noisy output to the test console. The rest of this function already uses runtime.error and runtime.exit — using console.log for the success path breaks that consistency.
| console.log(`Exported session "${sessionKey}" to ${outputPath}`); | |
| runtime.log(`Exported session "${sessionKey}" to ${outputPath}`); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 217
Comment:
**`console.log` bypasses runtime abstraction and test suppression**
`runtime.log` should be used here instead of `console.log`. Looking at `src/runtime.ts`, `runtime.log` wraps `console.log` with a `shouldEmitRuntimeLog()` guard that suppresses output when running under Vitest (unless `console.log` is mocked or `OPENCLAW_TEST_RUNTIME_LOG=1`). Calling `console.log` directly skips this gate, so any future test that invokes `sessionExportCommand` with `--output` will emit noisy output to the test console. The rest of this function already uses `runtime.error` and `runtime.exit` — using `console.log` for the success path breaks that consistency.
```suggestion
runtime.log(`Exported session "${sessionKey}" to ${outputPath}`);
```
How can I resolve this? If you propose a fix, please make it concise.| const outputPath = path.resolve( | ||
| output.startsWith("~") ? output.replace("~", process.env.HOME ?? "") : output, | ||
| ); |
There was a problem hiding this comment.
os.homedir()
output.replace("~", process.env.HOME ?? "") has two problems:
process.env.HOMEisundefinedon Windows, falling back to"", which causespath.resolve("")to resolve to the current working directory rather than the home directory.os.homedir()from Node's built-inosmodule is portable and always returns a valid path.String.replace(string, string)only replaces the first occurrence. For the typical~/pathpattern this is fine in practice, but the idiomatic form uses a regex anchored to the start:/^~/.
| const outputPath = path.resolve( | |
| output.startsWith("~") ? output.replace("~", process.env.HOME ?? "") : output, | |
| ); | |
| const outputPath = path.resolve( | |
| output.startsWith("~") ? output.replace(/^~/, os.homedir()) : output, | |
| ); |
Also add import os from "node:os"; at the top of the file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 209-211
Comment:
**Fragile tilde expansion — use `os.homedir()`**
`output.replace("~", process.env.HOME ?? "")` has two problems:
1. `process.env.HOME` is `undefined` on Windows, falling back to `""`, which causes `path.resolve("")` to resolve to the current working directory rather than the home directory. `os.homedir()` from Node's built-in `os` module is portable and always returns a valid path.
2. `String.replace(string, string)` only replaces the first occurrence. For the typical `~/path` pattern this is fine in practice, but the idiomatic form uses a regex anchored to the start: `/^~/`.
```suggestion
const outputPath = path.resolve(
output.startsWith("~") ? output.replace(/^~/, os.homedir()) : output,
);
```
Also add `import os from "node:os";` at the top of the file.
How can I resolve this? If you propose a fix, please make it concise.| lines.push("<details>"); | ||
| lines.push(`<summary>Tool call</summary>`); |
There was a problem hiding this comment.
<summary> is missing the tool name
Tool result summaries include the tool name (Tool result: weather.get), but tool call summaries just say "Tool call" — forcing the user to expand every <details> block to find out which tool was invoked. Since extractAssistantText already returns the tool name embedded inside the code string (tc starts with ${block.name}(), it's straightforward to include the name in the summary for at-a-glance readability, consistent with how tool results are labelled.
One way to fix this is to change extractAssistantText to return structured objects instead of formatted strings:
// extractAssistantText returns { name: string; argsStr: string }[]
toolCalls.push({ name: block.name, argsStr: JSON.stringify(block.arguments, null, 2) });Then in the rendering loop:
lines.push(`<summary>Tool call: ${tc.name}</summary>`);Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/session-export.ts
Line: 134-135
Comment:
**Tool call `<summary>` is missing the tool name**
Tool result summaries include the tool name (`Tool result: weather.get`), but tool call summaries just say `"Tool call"` — forcing the user to expand every `<details>` block to find out which tool was invoked. Since `extractAssistantText` already returns the tool name embedded inside the code string (`tc` starts with `${block.name}(`), it's straightforward to include the name in the summary for at-a-glance readability, consistent with how tool results are labelled.
One way to fix this is to change `extractAssistantText` to return structured objects instead of formatted strings:
```ts
// extractAssistantText returns { name: string; argsStr: string }[]
toolCalls.push({ name: block.name, argsStr: JSON.stringify(block.arguments, null, 2) });
```
Then in the rendering loop:
```ts
lines.push(`<summary>Tool call: ${tc.name}</summary>`);
```
How can I resolve this? If you propose a fix, please make it concise.| // Should not contain the full 1000 chars | ||
| expect(md.length).toBeLessThan(longText.length); |
There was a problem hiding this comment.
expect(md.length).toBeLessThan(longText.length) passes because the truncated content + markdown boilerplate (~650 chars) is less than 1000 chars. This will break silently if the boilerplate ever grows past 1000 chars. A more direct assertion would verify that the raw tool result text is actually truncated to 500 characters:
| // Should not contain the full 1000 chars | |
| expect(md.length).toBeLessThan(longText.length); | |
| // Should not contain the full 1000 chars | |
| expect(md).not.toContain("x".repeat(501)); | |
| expect(md).toContain("x".repeat(500) + "..."); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/session-export.test.ts
Line: 159-160
Comment:
**Truncation assertion is indirect and may give false confidence**
`expect(md.length).toBeLessThan(longText.length)` passes because the truncated content + markdown boilerplate (~650 chars) is less than 1000 chars. This will break silently if the boilerplate ever grows past 1000 chars. A more direct assertion would verify that the raw tool result text is actually truncated to 500 characters:
```suggestion
// Should not contain the full 1000 chars
expect(md).not.toContain("x".repeat(501));
expect(md).toContain("x".repeat(500) + "...");
```
How can I resolve this? If you propose a fix, please make it concise.Add `openclaw sessions export` subcommand that converts session JSONL transcripts to clean markdown or JSON. Tool calls and results are collapsed into <details> blocks. Long tool results are truncated. Avoids the Prettier template corruption that has broken the existing HTML /export-session command (openclaw#41862, openclaw#22595, openclaw#49957) by using plain text formatting with no template placeholders. Also fixes pre-existing oxfmt issue in docs/automation/standing-orders.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Handle legacy assistant string content in extractAssistantText (Codex P1) - Escape triple backticks in tool result/call code blocks (Codex P2) - Use runtime.log instead of console.log (Greptile P2) - Use os.homedir() instead of process.env.HOME for tilde expansion (Greptile P2) - Include tool name in tool call summary tags (Greptile P2) - Improve truncation test assertion directness (Greptile P2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c623ed2 to
c2c916b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2c916b16d
ℹ️ 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".
| const storePath = resolveDefaultSessionStorePath(agentId); | ||
| const sessionStore = loadSessionStore(storePath); | ||
| const entry = sessionStore[sessionKey]; |
There was a problem hiding this comment.
Resolve export store from configured session targets
sessionExportCommand hardcodes resolveDefaultSessionStorePath(agentId) and then reads sessionStore[sessionKey], which bypasses the config-driven store resolution used by other session commands. In setups with a non-main default agent or a custom session.store path/template, openclaw sessions export will read the wrong sessions.json and incorrectly fail with “Session not found” for valid sessions.
Useful? React with 👍 / 👎.
|
Closing — CI failures and no reviewer interest. Happy to resubmit if there's demand. |
Summary
Adds
openclaw sessions export- a CLI subcommand that converts session JSONL transcripts to clean markdown or pretty-printed JSON. Tool calls and tool results are collapsed into<details>blocks. Long tool results are truncated at 500 characters.Why this matters
The existing
/export-sessionslash command generates HTML, but it has been broken for months because Prettier keeps corrupting the template's JS placeholders:Five open PRs are trying to fix the HTML template corruption. This PR takes a different approach: plain markdown output with no template placeholders, so there is nothing for formatters to break.
Users also want portable session history (#50701 - WebUI history resets after updates, #50865 - access to old sessions). A CLI export to markdown gives users a working way to archive conversations.
Changes
src/commands/session-export.ts: New command that reads session JSONL viaSessionManager.open(), walksSessionMessageEntryentries, and formats them as markdown with timestamps and speaker labels. User messages shown directly, assistant text blocks shown, tool calls collapsed into<details>with the tool name and arguments, tool results collapsed with truncation. Also supports--format jsonfor raw output.src/commands/session-export.test.ts: 10 vitest tests covering empty sessions, user/assistant exchanges, tool calls, tool results, error results, image content, compaction markers, long result truncation, and null headers.src/cli/program/register.status-health-sessions.ts: Registerssessions exportas a subcommand alongsidesessions cleanup. Accepts--session,--agent,--format,--outputflags.Usage
Testing
All 10 tests pass. Files pass oxfmt format check and oxlint individually. Note:
pnpm tsgohas pre-existing type errors inextensions/msteams/on upstream/main that are unrelated to this change.This contribution was developed with AI assistance (Claude Code).