Skip to content

CLI: add --session-key flag to openclaw agent#35241

Closed
zhangzhejian wants to merge 2 commits into
openclaw:mainfrom
zhangzhejian:feat/agent-session-key
Closed

CLI: add --session-key flag to openclaw agent#35241
zhangzhejian wants to merge 2 commits into
openclaw:mainfrom
zhangzhejian:feat/agent-session-key

Conversation

@zhangzhejian

Copy link
Copy Markdown

Summary

  • Add --session-key <key> option to openclaw agent CLI for explicit session key routing
  • Plumb sessionKey through AgentCliOpts, validation guard, resolveSessionKeyForRequest(), and the --local fallback path
  • Enables external integrations (e.g., agentgram polling scripts) to use custom session keys like agentgram:<room_id> for per-room session isolation

Test plan

  • pnpm tsgo — no new type errors
  • pnpm test -- src/commands/agent/session.test — all 8 tests pass
  • Manual: openclaw agent --session-key "agentgram:test-room" --message "hello" --json

🤖 Generated with Claude Code

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes commands Command implementations size: XS labels Mar 5, 2026
@greptile-apps

greptile-apps Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a --session-key <key> flag to the openclaw agent CLI command, enabling callers to explicitly specify a session key for routing — particularly useful for external integrations like polling scripts that need per-room session isolation (e.g., agentgram:<room_id>).

The change is well-scoped and correctly threads the new option through the type definition (AgentCliOpts), the validation guard, resolveSessionKeyForRequest(), and the --local embedded fallback path.

Key observations:

  • In agentCliCommand, the explicit field assignment for the session key inside localOpts is redundant — unlike agentId (mapped from agent) and replyAccountId (mapped from replyAccount), this field has the same name in both types and is already included by the ...opts spread.
  • The gateway execution path (agentViaGatewayCommand) does not include the --agent conflict check that the local path (agentCommandInternal) already performs when an explicit session key is also provided, creating a minor inconsistency: conflicting flags yield a clear local error but are silently forwarded to the gateway when run without --local.
  • No CLI help text example was added for the new flag, though this is cosmetic.

Confidence Score: 4/5

  • This PR is safe to merge; the changes are minimal, well-contained, and do not introduce any regressions.
  • The core logic is sound and correctly integrated across all affected call sites. The only findings are a redundant field assignment (style) and a minor inconsistency in conflict-detection behaviour between the gateway and local execution paths — neither of which would cause a runtime failure.
  • No files require special attention; src/commands/agent-via-gateway.ts has the minor issues noted above.

Last reviewed commit: 16a2dc8

Comments Outside Diff (1)

  1. undefined, line undefined (link)

    Missing --agent + --session-key conflict validation in the gateway path

    The local execution path (agentCommandInternal in agent.ts, line 366–373) explicitly validates that the agent ID embedded in --session-key matches the --agent flag, and throws a clear error if they conflict:

    if (agentIdOverride && opts.sessionKey) {
      const sessionAgentId = resolveAgentIdFromSessionKey(opts.sessionKey);
      if (sessionAgentId !== agentIdOverride) {
        throw new Error(`Agent id "..." does not match session key agent "...".`);
      }
    }

    This guard is absent in agentViaGatewayCommand. When the gateway path is used with conflicting --agent and --session-key values, the mismatch is silently passed through to the gateway. If the gateway returns an opaque error, the user will get less actionable feedback than they would from the --local path.

    Consider adding the same pre-flight check here (or at least in agentCliCommand before dispatching to either path) so the behaviour is consistent regardless of execution mode.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/commands/agent-via-gateway.ts
    Line: 116-122
    
    Comment:
    **Missing `--agent` + `--session-key` conflict validation in the gateway path**
    
    The local execution path (`agentCommandInternal` in `agent.ts`, line 366–373) explicitly validates that the agent ID embedded in `--session-key` matches the `--agent` flag, and throws a clear error if they conflict:
    
    ```ts
    if (agentIdOverride && opts.sessionKey) {
      const sessionAgentId = resolveAgentIdFromSessionKey(opts.sessionKey);
      if (sessionAgentId !== agentIdOverride) {
        throw new Error(`Agent id "..." does not match session key agent "...".`);
      }
    }
    ```
    
    This guard is absent in `agentViaGatewayCommand`. When the gateway path is used with conflicting `--agent` and `--session-key` values, the mismatch is silently passed through to the gateway. If the gateway returns an opaque error, the user will get less actionable feedback than they would from the `--local` path.
    
    Consider adding the same pre-flight check here (or at least in `agentCliCommand` before dispatching to either path) so the behaviour is consistent regardless of execution mode.
    
    How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 185 to 189
const localOpts = {
...opts,
agentId: opts.agent,
sessionKey: opts.sessionKey,
replyAccountId: opts.replyAccount,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unnecessary explicit field mapping

The sessionKey field name is identical in both AgentCliOpts and AgentCommandOpts, so the ...opts spread on line 185 already carries the value into localOpts — the explicit mapping on line 188 is a no-op.

This differs from agentId: opts.agent and replyAccountId: opts.replyAccount, which are necessary because the CLI prop names differ from the command type field names. Since the field is named identically in both types, the explicit assignment can be removed.

Suggested change
const localOpts = {
...opts,
agentId: opts.agent,
sessionKey: opts.sessionKey,
replyAccountId: opts.replyAccount,
const localOpts = {
...opts,
agentId: opts.agent,
replyAccountId: opts.replyAccount,
};
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/agent-via-gateway.ts
Line: 185-189

Comment:
**Unnecessary explicit field mapping**

The `sessionKey` field name is identical in both `AgentCliOpts` and `AgentCommandOpts`, so the `...opts` spread on line 185 already carries the value into `localOpts` — the explicit mapping on line 188 is a no-op.

This differs from `agentId: opts.agent` and `replyAccountId: opts.replyAccount`, which are necessary because the CLI prop names differ from the command type field names. Since the field is named identically in both types, the explicit assignment can be removed.

```suggestion
  const localOpts = {
    ...opts,
    agentId: opts.agent,
    replyAccountId: opts.replyAccount,
  };
```

How can I resolve this? If you propose a fix, please make it concise.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16a2dc84bd

ℹ️ 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".

.requiredOption("-m, --message <text>", "Message body for the agent")
.option("-t, --to <number>", "Recipient number in E.164 used to derive the session key")
.option("--session-id <id>", "Use an explicit session id")
.option("--session-key <key>", "Use an explicit session key")

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 Badge Validate --session-key before exposing it on CLI

Adding --session-key here makes malformed keys user-reachable, but the local execution path still does not validate key shape (src/commands/agent.ts:343-373) while the gateway path explicitly rejects malformed agent: keys (src/gateway/server-methods/agent.ts:296-306). In practice, openclaw agent --session-key agent:main ... can fail gateway validation, fall back to embedded mode, and then create/use an invalid session key, causing inconsistent behavior between normal and fallback runs and potentially orphaned session history.

Useful? React with 👍 / 👎.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 15, 2026
@himochen

Copy link
Copy Markdown

+1 from a downstream integration. We have a near-identical use case that would benefit from this flag merging:

Use case: an external agent harness (xenia) that runs in a separate Claude Code session and dispatches turns into an OpenClaw agent (elena) via subprocess for bidirectional threaded conversations. Current state without this flag:

  • --agent elena --session-id <xenia-thread-id> routes into agent:elena:main regardless of --session-id (verified: src/agents/command/session.ts:139-161, the !explicitSessionKey guard at line 161 short-circuits the session-id branch when --agent is provided), so external traffic collides with the agent's own main-session heartbeats.
  • AgentParamsSchema.sessionKey already exists at src/gateway/protocol/schema/agent.ts:134 and commands/agent-via-gateway.ts:138 already forwards it. Protocol + transport are already there; this PR just surfaces it on the CLI. No new architectural surface.
  • openclaw cron add already exposes --session-key (src/cli/cron-cli/register.cron-add.ts:76), so the CLI surface is asymmetric between cron add and agent today.

This gives us (and presumably anyone doing per-thread/per-room isolation — agentgram, xenia, or similar) a clean namespace like agent:<id>:xenia:<thread-id> that sits alongside main / explicit:<id> / cron session:<id> without collision.

Two small suggestions if the author is still around:

  1. The greptile bot's finding Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2 is a real asymmetry worth addressing here: the gateway path silently forwards conflicting --agent / --session-key combos, while the local path throws a clear error (agents/agent-command.ts:227-232). Mirroring the check in agentViaGatewayCommand would make the two paths behaviorally consistent.

  2. Separately (not blocking this PR), downstream integrations that manage their own gateway-availability checks would benefit from a companion --no-fallback flag to suppress the built-in gateway→embedded auto-fallback at commands/agent-via-gateway.ts:180-196. Happy to open a follow-up PR for that after this one lands, to keep the diff scope here clean.

Would like to see this unstaled. If @zhangzhejian is unavailable, I'm happy to pick up the rebase / address review comments.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Apr 17, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 25, 2026
@clawsweeper

clawsweeper Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and the useful part of this older PR is already implemented there.

Current main already carries the useful openclaw agent --session-key behavior through the merged replacement PR, including validation, embedded fallback handling, docs, tests, and real CLI proof; the remaining value in this older branch is obsolete.

So I’m closing this older PR as already covered on main rather than keeping a mostly-duplicated branch open.

Review details

Best possible solution:

Keep the current main implementation from the merged replacement PR and close this stale branch instead of trying to rebase its smaller, older diff.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a feature PR rather than a bug report. Source inspection confirms current main now exposes and routes --session-key, and the merged replacement PR includes terminal proof for the CLI behavior.

Is this the best way to solve the issue?

Yes for current main, not for this branch. The merged replacement PR is the maintainable solution because it adds shared validation, embedded-path parity, docs, tests, and proof beyond this older two-file plumbing branch.

Security review:

Security review cleared: The stale branch only adds TypeScript CLI option plumbing and the superseding implementation does not introduce dependency, workflow, secret, install, or package-resolution changes.

What I checked:

Likely related people:

  • Kaspre: Authored the merged replacement PR that implemented explicit CLI session keys with validation, docs, tests, and real behavior proof. (role: replacement feature implementer; confidence: high; commits: eb7f3b7b50c5, 2c76dd339f5f; files: src/commands/agent-via-gateway.ts, src/agents/agent-command.ts, src/routing/session-key.ts)
  • Peter Steinberger: Local history shows repeated recent work across the central agent CLI, gateway dispatch, routing helper, and session-resolution paths touched by this feature. (role: recent area contributor; confidence: medium; commits: 5e97045345a6, 310b5e4f6a22, 22e7b462c527; files: src/commands/agent-via-gateway.ts, src/cli/program/register.agent.ts, src/agents/command/session.ts)
  • Vincent Koc: Recent commits in the inspected history narrow and refactor agent session helper and command imports near the same CLI/session-routing boundary. (role: adjacent session/agent contributor; confidence: medium; commits: ce1fffa97e24, 99755fcb2f1f, f2e08295e6b3; files: src/commands/agent-via-gateway.ts, src/agents/command/session.ts, src/routing/session-key.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against bb5010b89a5a; fix evidence: commit eb7f3b7b50c5, main fix timestamp 2026-05-22T03:08:25Z.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Apr 26, 2026
@clawsweeper clawsweeper Bot added the P2 Normal backlog priority with limited blast radius. label May 16, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 16, 2026
@clawsweeper clawsweeper Bot added impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. and removed impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels May 17, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI command changes commands Command implementations P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants