Skip to content

Add opt-in multiplayer mode for Codex sessions#8

Merged
steipete merged 10 commits into
openclaw:mainfrom
RomneyDa:add-session-speaker-attribution
May 23, 2026
Merged

Add opt-in multiplayer mode for Codex sessions#8
steipete merged 10 commits into
openclaw:mainfrom
RomneyDa:add-session-speaker-attribution

Conversation

@RomneyDa
Copy link
Copy Markdown
Member

@RomneyDa RomneyDa commented May 23, 2026

messages with tag are from multiplayer mode

image

Summary

  • Add an opt-in multiplayer mode setting for interactive Codex sessions
  • Add owner/maintainer actions to enable or disable multiplayer mode from the session controls
  • When enabled, submitted terminal prompts are sent to Codex as raw text prefixed with the current actor, e.g. dallin:\nmessage
  • Leave terminal input unchanged when multiplayer mode is off

Checks

  • pnpm run check
  • pnpm test

@RomneyDa RomneyDa force-pushed the add-session-speaker-attribution branch 3 times, most recently from 47046fb to 1bc2e89 Compare May 23, 2026 18:32
@RomneyDa RomneyDa changed the title Add multiplayer terminal input attribution Track multiplayer terminal input actors May 23, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 23, 2026

Codex review: needs changes before merge.

Latest ClawSweeper review: 2026-05-23 21:39 UTC / May 23, 2026, 5:39 PM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The branch adds persisted opt-in multiplayer attribution for interactive Codex terminal submissions, local dev identity switching, UI/API controls, tests, docs, migrations, and changelog text.

Reproducibility: yes. at source level: the new parser returns null when a buffered line is completed by a non-empty payload ending in Enter, and the server updates input state before checking whether multiplayer is enabled. I did not run a live browser/PTY session in this read-only review.

PR rating
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
Summary: Useful feature work with focused tests, but patch confidence is limited by source-level attribution and mode-state blockers.

Rank-up moves:

  • Fix the terminal input parser for batched text-plus-Enter frames and disabled-mode buffering.
  • Make API docs and server authorization agree on who can toggle multiplayer mode.
  • Add live or logged PTY proof for sender attribution, disabled passthrough, and two writable actors after the fix.
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Not applicable: The external-contributor proof gate does not apply because the PR author is a repository member; the body includes a screenshot and check list, but live PTY proof would still help review.

Mantis proof suggestion
A live browser/PTY proof would materially show the multiplayer toggle, sender-tagged submissions, paste or batched Enter behavior, and disabled-mode passthrough. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify the multiplayer toggle prefixes Codex terminal submissions with sender tags, including pasted text ending with Enter, disabled-mode passthrough, and two writable actors.

Risk before merge

  • Enabled multiplayer can miss attribution when a terminal frame appends text and Enter to an already buffered line, so a submitted prompt may reach Codex without the expected sender tag.
  • Disabled-mode input still mutates multiplayer input state, so toggling the mode on around a partially typed line can wrongly rewrite or attribute text that was entered while the feature was off.
  • The API documentation says owner/maintainer can toggle multiplayer, while the server-side code restricts the action to the session creator; maintainers should confirm which policy is intended before landing.
  • The PR has checks and a screenshot, but no live browser/PTY proof for batched input, toggle transitions, or two-writer behavior after the latest fix.

Maintainer options:

  1. Fix attribution parser before merge (recommended)
    Update the new multiplayer input handling so enabled mode attributes buffered lines completed by any payload containing Enter, disabled mode does not populate attribution state, and regression tests cover both cases.
  2. Confirm toggle ownership policy
    Before merge, decide whether the toggle is creator-only or owner/maintainer-managed and make the API docs, UI affordance, and server authorization say the same thing.
  3. Pause for live PTY proof
    If maintainers are unsure about terminal transport behavior, pause merge until a live browser/PTY recording or logs show sender-tagged submissions for ordinary typing, paste, and two writable actors.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Update multiplayer terminal input attribution so disabled-mode input never mutates the multiplayer attribution buffer, enabled mode attributes buffered lines when a payload completes them with \r or \n even if the payload also contains text, and add focused regression coverage for batched text-plus-Enter submissions and toggling from disabled to enabled without retroactively rewriting prior input.

Next step before merge
A narrow automated repair can adjust the new terminal attribution parser, mode-state handling, docs mismatch, and regression tests without changing the broader feature direction.

Security
Cleared: No concrete security or supply-chain regression found; the diff adds no dependencies or workflows and gates the new dev-identity login to local requests.

Review findings

  • [P2] Attribute buffered submissions that include Enter — src/terminal-multiplayer.ts:40-45
  • [P2] Avoid buffering input while multiplayer is off — src/index.ts:2668-2676
  • [P3] Align the documented toggle role with the server check — docs/api.md:331-332
Review details

Best possible solution:

Land the feature only after terminal attribution state is scoped to enabled mode, line submission parsing covers batched text-plus-Enter frames, and the documented toggle policy matches the server behavior.

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

Yes at source level: the new parser returns null when a buffered line is completed by a non-empty payload ending in Enter, and the server updates input state before checking whether multiplayer is enabled. I did not run a live browser/PTY session in this read-only review.

Is this the best way to solve the issue?

No: the feature direction is reasonable, but the current implementation is not yet the narrowest maintainable solution because attribution parsing and toggle-state behavior can mis-handle real terminal input frames.

Label justifications:

  • P2: This is a normal-priority feature PR with limited blast radius but blocking correctness risks in terminal prompt attribution.
  • merge-risk: 🚨 message-delivery: Merging as-is could send prompts to Codex without the expected sender tag or with text attributed to the wrong actor.
  • merge-risk: 🚨 session-state: The new shared per-session input buffer can retain disabled-mode or partial-line state across mode transitions.
  • rating: 🦐 gold shrimp: Current PR rating is 🦐 gold shrimp because proof is 🌊 off-meta tidepool, patch quality is 🦐 gold shrimp, and Useful feature work with focused tests, but patch confidence is limited by source-level attribution and mode-state blockers.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply because the PR author is a repository member; the body includes a screenshot and check list, but live PTY proof would still help review.

Full review comments:

  • [P2] Attribute buffered submissions that include Enter — src/terminal-multiplayer.ts:40-45
    When state.line already has text, a payload such as world\r skips both submission branches, updateTerminalInputLine clears the buffer, and the PR forwards the raw payload. Terminals can batch data this way, especially for paste or buffered input, so multiplayer mode can submit hello world without any <sender .../> attribution. Handle trailing Enter even when the line buffer is non-empty.
    Confidence: 0.88
  • [P2] Avoid buffering input while multiplayer is off — src/index.ts:2668-2676
    This calls terminalSubmittedLine before checking the persisted multiplayer flag, so default disabled-mode typing still mutates the shared input buffer. If the mode is enabled while a line is partially typed, the next submitted line can be rewritten and attributed using text that was entered while multiplayer was off, violating the passthrough guarantee.
    Confidence: 0.82
  • [P3] Align the documented toggle role with the server check — docs/api.md:331-332
    The new API docs say owner/maintainer can enable and disable multiplayer, but the server code rejects everyone except the session creator. Pick the intended policy and update either the docs or the authorization check so the public API contract matches runtime behavior.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.86

Acceptance criteria:

  • pnpm run check
  • pnpm test

What I checked:

  • Protected cleanup boundary: The GitHub context marks the PR author association as MEMBER, which this review workflow must keep open for explicit maintainer handling. (d61da2c94d15)
  • Current main terminal input baseline: Current main forwards terminal input frames directly to the upstream PTY after control checks; the PR replaces this with attribution-aware payload generation. (src/index.ts:1672, cd3072ed45c5)
  • PR source review: The new terminalSubmittedLine helper only treats payloads as submitted when the whole payload is exactly Enter or when there is no buffered line, so a buffered line completed by a payload like world\r is forwarded without attribution. (src/terminal-multiplayer.ts:40, d61da2c94d15)
  • PR mode-state review: The PR calls terminalSubmittedLine before checking whether multiplayer mode is enabled, so disabled-mode input can still populate the shared attribution buffer and affect a later enabled submission. (src/index.ts:2668, d61da2c94d15)
  • Feature history: git blame shows the current interactive terminal hub and control surface originate in commit 7ded20e, giving a routing anchor for this area. (src/index.ts:1548, 7ded20e07362)

Likely related people:

  • Peter Steinberger: git blame attributes the current interactive terminal hub, session decoration, and UI action surfaces to the grafted initial implementation commit, and shortlog shows the most commits in the central files. (role: introduced behavior; confidence: medium; commits: 7ded20e07362, c0328d73ae0e; files: src/index.ts, src/app/main.jsx, docs/api.md)
  • RomneyDa: Recent merged history shows multiple session, sandbox, and local-auth fixes by the same contributor, so they are relevant beyond authorship of this PR. (role: recent area contributor; confidence: medium; commits: ce6e5f2c2f1a, 23e3fd69410e, fab178b; files: src/index.ts, src/app/main.jsx, CHANGELOG.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against cd3072ed45c5.

@RomneyDa RomneyDa marked this pull request as draft May 23, 2026 18:38
@RomneyDa RomneyDa closed this May 23, 2026
@RomneyDa RomneyDa force-pushed the add-session-speaker-attribution branch from 1bc2e89 to 14ea065 Compare May 23, 2026 18:38
@RomneyDa RomneyDa reopened this May 23, 2026
@RomneyDa RomneyDa changed the title Track multiplayer terminal input actors Add opt-in multiplayer terminal prompts May 23, 2026
@RomneyDa RomneyDa changed the title Add opt-in multiplayer terminal prompts Add opt-in multiplayer mode for Codex sessions May 23, 2026
@RomneyDa RomneyDa marked this pull request as ready for review May 23, 2026 18:44
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. labels May 23, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 23, 2026

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@RomneyDa RomneyDa force-pushed the add-session-speaker-attribution branch from 1e3a306 to 7063cf4 Compare May 23, 2026 18:55
@RomneyDa RomneyDa marked this pull request as draft May 23, 2026 19:39
@RomneyDa RomneyDa force-pushed the add-session-speaker-attribution branch from 7063cf4 to 45c0290 Compare May 23, 2026 21:19
@RomneyDa
Copy link
Copy Markdown
Member Author

@clawsweeper re-review

Addressed the P2 shared PTY input finding:

  • moved multiplayer pending input tracking from TerminalHubSubscription to a shared per-session input state
  • preserved multiplayerMode=false passthrough
  • added focused regression coverage for two write-capable actors interleaving input before Enter and disabled passthrough

Checks run locally:

  • pnpm run check
  • pnpm test

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 23, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@RomneyDa RomneyDa marked this pull request as ready for review May 23, 2026 21:34
@steipete steipete merged commit 247b598 into openclaw:main May 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants