feat: preserve OpenClaw main-session continuity for TUI gateway client#48589
feat: preserve OpenClaw main-session continuity for TUI gateway client#48589doskoi wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR introduces stable TUI session continuity for the gateway client. It does so via three coordinated mechanisms: (1) a per-terminal-slot instance ID ( Key changes:
Minor issues found:
Confidence Score: 4/5
|
| /\b(which|choose|pick|option|a\/b|a or b|1 or 2|one or both|both)\b/i.test(normalized) || | ||
| /\b(both)\b/i.test(normalized) | ||
| ) { |
There was a problem hiding this comment.
Redundant second regex check
The second condition || /\b(both)\b/i.test(normalized) is entirely redundant — both is already included in the alternation group of the first regex on the line above. This dead branch will never add any new matches and looks like a copy-paste artefact.
| /\b(which|choose|pick|option|a\/b|a or b|1 or 2|one or both|both)\b/i.test(normalized) || | |
| /\b(both)\b/i.test(normalized) | |
| ) { | |
| if (/\b(which|choose|pick|option|a\/b|a or b|1 or 2|one or both|both)\b/i.test(normalized)) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/session.ts
Line: 274-276
Comment:
**Redundant second regex check**
The second condition `|| /\b(both)\b/i.test(normalized)` is entirely redundant — `both` is already included in the alternation group of the first regex on the line above. This dead branch will never add any new matches and looks like a copy-paste artefact.
```suggestion
if (/\b(which|choose|pick|option|a\/b|a or b|1 or 2|one or both|both)\b/i.test(normalized)) {
```
How can I resolve this? If you propose a fix, please make it concise.| export function isTuiMainSessionAlias(params: { | ||
| raw?: string | null; | ||
| currentAgentId: string; | ||
| sessionMainKey: string; | ||
| }): boolean { | ||
| const trimmed = (params.raw ?? "").trim().toLowerCase(); | ||
| if (!trimmed) { | ||
| return false; | ||
| } | ||
| if (trimmed === "main") { | ||
| return true; | ||
| } | ||
| const mainKey = normalizeMainKey(params.sessionMainKey); | ||
| if (trimmed === mainKey) { | ||
| return true; | ||
| } | ||
| return ( | ||
| trimmed === | ||
| buildAgentMainSessionKey({ | ||
| agentId: params.currentAgentId, | ||
| mainKey, | ||
| }).toLowerCase() | ||
| ); | ||
| } |
There was a problem hiding this comment.
Exported function with no consumers
isTuiMainSessionAlias is exported but has no callers in the current codebase (the only reference is the file itself). If this is scaffolding for a follow-up PR, consider adding a // TODO comment explaining the intended consumer, or keeping it unexported until it's actually used — exporting dead code inflates the public API surface and may confuse future readers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/client-affinity.ts
Line: 52-75
Comment:
**Exported function with no consumers**
`isTuiMainSessionAlias` is exported but has no callers in the current codebase (the only reference is the file itself). If this is scaffolding for a follow-up PR, consider adding a `// TODO` comment explaining the intended consumer, or keeping it unexported until it's actually used — exporting dead code inflates the public API surface and may confuse future readers.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aad59acdc5
ℹ️ 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".
| if (!resetTriggered && !entry) { | ||
| const orphanCandidate = findOrphanShortReplyCandidate({ | ||
| ctx: sessionCtxForState, | ||
| sessionKey, | ||
| sessionStore, |
There was a problem hiding this comment.
Limit orphan short-reply reattach to default TUI sessions
The new orphan-short-reply recovery runs for any missing session key, so an explicit session switch (for example /session project-x) can be overridden if the first message is a short token like yes/ok and another recent TUI session on the same OriginatingTo is awaiting confirmation. In that case, sessionKey is silently reassigned to the candidate session, causing the reply to be written to the wrong transcript and defeating the user’s explicit session choice. This should be gated to default/auto main-session aliases only, not arbitrary missing keys.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. What this changes: The branch adds stable TUI instance IDs, TUI affinity session keys/origin metadata, and orphan short-reply reattachment logic/tests for Gateway TUI main-session continuity. Required change before merge: The blockers are narrow, file-level repairs on an otherwise relevant implementation PR, so an automated worker can update the branch or prepare a replacement branch with focused tests and changelog coverage. Security review: Security review cleared: The PR diff does not add dependencies, workflows, release scripts, third-party code execution, secret handling, or broader permissions; the new persistent TUI instance state is local OpenClaw state with sanitized path segments. Review findings:
Review detailsBest possible solution: Rebase and repair the branch so stable per-terminal TUI affinity is preserved, origin metadata is synthesized for the real TUI client id, orphan short-reply recovery is limited to default/main TUI aliases, explicit session switching is covered by regression tests, and the user-facing feature gets a changelog entry. Do we have a high-confidence way to reproduce the issue? Yes. The source-level reproduction is high-confidence: current main sends TUI traffic as openclaw-tui while the PR fallback checks gateway-client, and the PR's orphan reattach block runs for any missing session key before proving it was a default/main TUI alias. Is this the best way to solve the issue? No. The overall direction is reasonable, but this is not yet the best fix because it misses the current client identity and can override explicit session choices; the safer implementation is to match GATEWAY_CLIENT_NAMES.TUI and gate recovery to recognized main-session aliases only. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against e26357fee8fc. |
No description provided.