Make OpenClaw output clearer and add hands-free exec preset#67405
Make OpenClaw output clearer and add hands-free exec preset#67405IgorGanapolsky wants to merge 2 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c769a25a2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| super(); | ||
| this.body = new HyperlinkMarkdown(text, 0, 0, markdownTheme, { | ||
| this.timestamp = formatTuiTimestamp(); | ||
| this.body = new HyperlinkMarkdown(prefixTimestamp(text, this.timestamp), 0, 0, markdownTheme, { |
There was a problem hiding this comment.
AssistantMessageComponent currently prepends the timestamp directly into the Markdown payload via prefixTimestamp(...). This alters parsing for responses whose first line relies on Markdown-at-line-start syntax (for example, content beginning with ```, headings, or list markers), so those replies can render incorrectly in the TUI after this commit. The timestamp should be rendered out-of-band (separate text element) instead of mutating the assistant Markdown body.
Useful? React with 👍 / 👎.
| export class UserMessageComponent extends MarkdownMessageComponent { | ||
| constructor(text: string) { | ||
| super(text, 1, { | ||
| super(prefixTimestamp(text), 1, { |
There was a problem hiding this comment.
UserMessageComponent adds the timestamp only in its constructor, but it inherits setText unchanged from MarkdownMessageComponent. In ChatLog.addPendingUser, existing pending rows are updated with existing.component.setText(text), which replaces the message with raw text and drops the timestamp prefix, so updated pending-user entries no longer follow the new timestamped format.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR replaces informal/humorous TUI copy (waiting phrases, taglines, banner text) with neutral operator-facing language, adds Confidence Score: 5/5Safe to merge; all findings are P2 style suggestions with no impact on correctness or security. The preset, timestamp, and copy changes are all well-scoped and covered by targeted tests. The only flagged items are a locale-dependent 12-hour clock format and a minor word-repetition in the waiting message — neither affects runtime correctness. src/tui/components/timestamp.ts — consider hour12: false for deterministic 24-hour output Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/tui/components/timestamp.ts
Line: 2-6
Comment:
**Locale-dependent 12-hour format in TUI timestamps**
`toLocaleTimeString([], { hour: "2-digit", ... })` inherits the host's hour cycle. In `en-US` (12-hour default), this produces `02:30:45 PM`, so every bracket in the TUI reads `[02:30:45 PM]` — longer than intended and inconsistent across locales. Adding `hour12: false` forces 24-hour display regardless of locale.
```suggestion
return date.toLocaleTimeString([], {
hour: "2-digit",
minute: "2-digit",
second: "2-digit",
hour12: false,
});
```
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/tui/tui-waiting.ts
Line: 46
Comment:
**"working" appears twice when certain phrases are selected**
The template now hardcodes `• working ${params.elapsed}`. When the rotating phrase resolves to `"working on the request"`, the full output reads `working on the request... • working 3s | connected` — "working" twice in a row. The five other phrases don't overlap, but consider a label that doesn't duplicate the phrase vocabulary, e.g. `• elapsed: ${params.elapsed}`, to keep it neutral regardless of which phrase is selected.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Make OpenClaw output clearer and add han..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return date.toLocaleTimeString([], { | ||
| hour: "2-digit", | ||
| minute: "2-digit", | ||
| second: "2-digit", | ||
| }); |
There was a problem hiding this comment.
toLocaleTimeString([], { hour: "2-digit", ... }) inherits the host's hour cycle. In en-US (12-hour default), this produces 02:30:45 PM, so every bracket in the TUI reads [02:30:45 PM] — longer than intended and inconsistent across locales. Adding hour12: false forces 24-hour display regardless of locale.
| return date.toLocaleTimeString([], { | |
| hour: "2-digit", | |
| minute: "2-digit", | |
| second: "2-digit", | |
| }); | |
| return date.toLocaleTimeString([], { | |
| hour: "2-digit", | |
| minute: "2-digit", | |
| second: "2-digit", | |
| hour12: false, | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tui/components/timestamp.ts
Line: 2-6
Comment:
**Locale-dependent 12-hour format in TUI timestamps**
`toLocaleTimeString([], { hour: "2-digit", ... })` inherits the host's hour cycle. In `en-US` (12-hour default), this produces `02:30:45 PM`, so every bracket in the TUI reads `[02:30:45 PM]` — longer than intended and inconsistent across locales. Adding `hour12: false` forces 24-hour display regardless of locale.
```suggestion
return date.toLocaleTimeString([], {
hour: "2-digit",
minute: "2-digit",
second: "2-digit",
hour12: false,
});
```
How can I resolve this? If you propose a fix, please make it concise.| const cute = shimmerText(params.theme, `${phrase}…`, params.tick); | ||
| return `${cute} • ${params.elapsed} | ${params.connectionStatus}`; | ||
| const activity = shimmerText(params.theme, `${phrase}...`, params.tick); | ||
| return `${activity} • working ${params.elapsed} | ${params.connectionStatus}`; |
There was a problem hiding this comment.
The template now hardcodes • working ${params.elapsed}. When the rotating phrase resolves to "working on the request", the full output reads working on the request... • working 3s | connected — "working" twice in a row. The five other phrases don't overlap, but consider a label that doesn't duplicate the phrase vocabulary, e.g. • elapsed: ${params.elapsed}, to keep it neutral regardless of which phrase is selected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tui/tui-waiting.ts
Line: 46
Comment:
**"working" appears twice when certain phrases are selected**
The template now hardcodes `• working ${params.elapsed}`. When the rotating phrase resolves to `"working on the request"`, the full output reads `working on the request... • working 3s | connected` — "working" twice in a row. The five other phrases don't overlap, but consider a label that doesn't duplicate the phrase vocabulary, e.g. `• elapsed: ${params.elapsed}`, to keep it neutral regardless of which phrase is selected.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc0384e5e2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (entryKey === state.currentSessionKey) { | ||
| return true; | ||
| } | ||
| return normalizeMatchKey(entryKey) === normalizeMatchKey(state.currentSessionKey); |
There was a problem hiding this comment.
isCurrentSessionEntry compares parseAgentSessionKey(...).rest values, so an event for agent:ops:main is treated as the current session when the TUI is on agent:main:main. In applySessionInfoFromEvent, that false-positive path then updates state.currentSessionKey/agent state to the incoming key, so unrelated activity can silently switch the active session and route later sends to the wrong agent/session. This event-path matcher should require agent+rest equality (or exact key equality) instead of rest-only matching.
Useful? React with 👍 / 👎.
| const initialSessionInput = | ||
| explicitSessionInput || resolveTuiProjectSessionKey({ cwd: process.cwd() }); |
There was a problem hiding this comment.
When --session is omitted, runTui now forces initialSessionInput to a generated project key. That means resolveTuiSessionKey never receives the empty raw input that triggers configured defaults (global scope or agent main key), so users with session.scope=global (or default main-key workflows) are pushed into per-directory agent:<id>:project:... sessions instead of the expected shared default session unless they manually pass --session.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification