feat: add live messages panel - #5
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe PR adds persisted messages, a daemon ChangesMessaging flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPBridge
participant Daemon
participant MessageStore
participant Dashboard
MCPClient->>MCPBridge: call send_message
MCPBridge->>Daemon: POST /api/message
Daemon->>MessageStore: appendMessage
MessageStore-->>Daemon: stored message
Daemon-->>MCPBridge: message result
Dashboard->>Daemon: GET /api/state
Daemon->>MessageStore: readMessages(50)
MessageStore-->>Dashboard: persisted messages
Dashboard-->>Dashboard: render messages by kind
Possibly related PRs
Comment |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/store.js`:
- Around line 247-267: Update readMessages so state polling does not read and
parse the entire append-only messages file; retrieve only the newest max
records, preferably by tail-reading from the file or otherwise bounding the read
buffer and parsing work. Preserve reverse chronological ordering, malformed-line
skipping, and the max limit, and consider enforcing bounded file retention or
rotation if needed.
- Line 232: Update the generated id expression in the append-only store to use
at least 128 bits of cryptographic randomness, replacing the current four-byte
random value while preserving the existing m- prefix and hexadecimal format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 562f9859-6d2b-45b6-8cd8-ae12e8fbf3a3
📒 Files selected for processing (4)
bridge.jspublic/index.htmlsrc/daemon.jssrc/store.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/store.js
[warning] 246-246: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(paths.messages, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
public/index.html
[warning] 1447-1457: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: box.innerHTML = show.length ? show.map(m => {
const id = m?.id ?? '';
const kind = messageKind(m);
const fresh = pollN > 1 && seenMsg.get(id) === pollN;
return '
'
'' + esc(kind) + '' +
'' + esc(fmtRel(m?.ts)) + '' +
'new' +
'
}).join('') : hush('chat', msgFilter === 'all' ? 'No messages yet.' : 'Nothing matches this filter.')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🔇 Additional comments (6)
src/store.js (1)
18-18: LGTM!Also applies to: 397-398
src/daemon.js (2)
21-21: LGTM!Also applies to: 1422-1422, 1851-1856
1544-1544: 🩺 Stability & AvailabilityPersistence failures already return a JSON 500 via the server wrapper.
bridge.js (1)
304-317: LGTM!Also applies to: 367-376
public/index.html (2)
445-463: LGTM!Also applies to: 565-569, 688-688, 784-784, 922-925, 1031-1031, 1054-1054, 1222-1223, 1259-1259, 1658-1658, 1714-1714, 1731-1732
1447-1457: 🔒 Security & PrivacyNo issue:
escalready escapes HTML- and attribute-breaking characters.text,id, andtsare safe in thisinnerHTMLtemplate.> Likely an incorrect or invalid review comment.
| function appendMessage(partial = {}) { | ||
| const input = partial && typeof partial === 'object' ? partial : {}; | ||
| const message = { | ||
| id: `m-${crypto.randomBytes(4).toString('hex')}`, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Expand the generated message-ID space.
Four random bytes collide with meaningful probability as this append-only store grows; duplicate IDs merge dashboard freshness/signature identity. Use at least 128 bits.
Proposed fix
- id: `m-${crypto.randomBytes(4).toString('hex')}`,
+ id: `m-${crypto.randomBytes(16).toString('hex')}`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id: `m-${crypto.randomBytes(4).toString('hex')}`, | |
| id: `m-${crypto.randomBytes(16).toString('hex')}`, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/store.js` at line 232, Update the generated id expression in the
append-only store to use at least 128 bits of cryptographic randomness,
replacing the current four-byte random value while preserving the existing m-
prefix and hexadecimal format.
![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: e6e90495d4
ℹ️ 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".
| const max = Number.isInteger(limit) && limit > 0 ? limit : 50; | ||
|
|
||
| try { | ||
| const lines = fs.readFileSync(paths.messages, 'utf8').split(/\r?\n/); |
There was a problem hiding this comment.
readMessages(50) still synchronously reads and splits the entire append-only file before applying the limit. Because the visible dashboard calls /api/state every two seconds and that endpoint invokes this function, a long-running installation with many messages will repeatedly load an ever-growing file and block the daemon's event loop; use a bounded tail read like recentEvents() or rotate/truncate the log.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
|
|
||
| if (text.length > 2000) { |
There was a problem hiding this comment.
When a message contains non-BMP characters such as emoji, this check counts UTF-16 code units, while the MCP schema's maxLength: 2000 counts Unicode code points. Consequently, schema-valid inputs such as 1,001 emoji are rejected by the daemon as exceeding 2,000 characters; use a code-point-aware length check so the advertised tool contract and endpoint agree.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a “live messages” capability to AgentLoop so MCP clients can post progress updates/questions/results that appear on the dashboard.
Changes:
- Persist messages to a new
state/messages.ndjsonfile and exposeappendMessage()/readMessages()from the store. - Add a
POST /api/messageendpoint and include recent messages in/api/state. - Update the dashboard UI to render message “kinds” (info/question/results) and add an MCP tool (
send_message) in the bridge.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/store.js | Adds messages NDJSON storage plus read/append helpers. |
| src/daemon.js | Surfaces stored messages in daemon state and adds /api/message creation endpoint. |
| public/index.html | Updates dashboard rendering/styles for message kinds and enables the feature flag. |
| bridge.js | Adds send_message MCP tool to post messages into the daemon. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function readMessages(limit = 50) { | ||
| const max = Number.isInteger(limit) && limit > 0 ? limit : 50; | ||
|
|
||
| try { | ||
| const lines = fs.readFileSync(paths.messages, 'utf8').split(/\r?\n/); | ||
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/store.js`:
- Around line 255-270: Update the message-reading flow around the buffer
allocation and fs.readSync call to scan backward through the file in bounded
chunks, accumulating complete NDJSON records until the requested limit (max) is
reached or the beginning of the file is reached. Preserve record boundaries
across chunk joins, discard any partial leading record when more data remains,
and return the newest complete messages without limiting the scan to a single
maxMessageBytes tail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 598854c4-20ea-41ca-aba7-2dcd84ac4e5e
📒 Files selected for processing (2)
src/daemon.jssrc/store.js
📜 Review details
🔇 Additional comments (1)
src/daemon.js (1)
1532-1537: LGTM!
| const length = Math.min(size, maxMessageBytes); | ||
| const buffer = Buffer.alloc(length); | ||
| const descriptor = fs.openSync(paths.messages, 'r'); | ||
| let bytesRead; | ||
|
|
||
| try { | ||
| bytesRead = fs.readSync(descriptor, buffer, 0, length, size - length); | ||
| } finally { | ||
| fs.closeSync(descriptor); | ||
| } | ||
|
|
||
| const lines = buffer.subarray(0, bytesRead).toString('utf8').split(/\r?\n/); | ||
|
|
||
| if (size > length) { | ||
| lines.shift(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Read enough complete records to satisfy limit.
The fixed 16 KiB tail often contains far fewer than 50 valid messages—seven 2,000-character ASCII messages already exceed it. Since the daemon requests 50 and there is no pagination, persisted recent messages silently disappear from dashboard state. Scan backward in bounded chunks until max complete NDJSON records are found or BOF is reached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/store.js` around lines 255 - 270, Update the message-reading flow around
the buffer allocation and fs.readSync call to scan backward through the file in
bounded chunks, accumulating complete NDJSON records until the requested limit
(max) is reached or the beginning of the file is reached. Preserve record
boundaries across chunk joins, discard any partial leading record when more data
remains, and return the newest complete messages without limiting the scan to a
single maxMessageBytes tail.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3ab277e0b
ℹ️ 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".
| bridge: path.join(root, 'state', 'bridge.json'), | ||
| mcpToken: path.join(root, 'state', 'mcp-token'), | ||
| }; | ||
|
|
There was a problem hiding this comment.
The new bounded-read implementation fixes the earlier whole-file read, but this 16 KiB cap cannot satisfy daemonState()'s readMessages(50) request when messages approach the API's 2,000-character limit. For example, only about eight 2,000-byte ASCII messages fit, so after a burst the dashboard silently omits recent questions or results well before reaching its advertised 50-message limit; size the window for 50 maximum-size records or continue reading backward until 50 complete lines are collected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd49d67dc
ℹ️ 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".
Adds MCP-backed progress messages to the dashboard, with three review suggestions deferred as outside the agreed contract.