feat: dashboard - #2
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete AgentLoop browser command center with themed task views, polling, offline preview, task controls, messages, events, and transcript access. The daemon now returns structured dashboard state, tracks worker activity and completion reasons, persists logs, serves log data, and supports task cancellation. ChangesAgentLoop command center
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserDashboard
participant DaemonHTTPServer
participant LogStore
participant TaskWorker
BrowserDashboard->>DaemonHTTPServer: GET /api/state
DaemonHTTPServer-->>BrowserDashboard: Return structured dashboard state
BrowserDashboard->>DaemonHTTPServer: GET /api/log/task-id
DaemonHTTPServer->>LogStore: Read bounded task log
LogStore-->>DaemonHTTPServer: Return transcript lines
DaemonHTTPServer-->>BrowserDashboard: Return transcript data
BrowserDashboard->>DaemonHTTPServer: POST /api/cancel
DaemonHTTPServer->>TaskWorker: Terminate active worker
Possibly related PRs
Comment |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@public/index.html`:
- Around line 1262-1285: Update renderRecent and the corresponding expansion
rendering near the alternate recent-task view to include task.prompt and
result.reason when a task is expanded, while preserving the existing summary
display and collapsed behavior. Read the prompt from task.prompt and the
completion reason from result.reason, and render both with the existing escaping
and expanded-task state handling.
- Around line 1019-1043: Update poll to prevent overlapping invocations from
applying stale results: use a sequence token or equivalent serialization
covering both fetch success and fallback rendering, and discard any response
whose token is no longer current before mutating pollN, lastState, mockActive,
liveEverOk, offline status, or rendering. Apply the same protection to the
related invocation path around the additional poll logic.
- Around line 1109-1122: Remove or disable the message, loop, and answer
controls and their associated flows, including handleAnswer and the code around
the listed UI sections, unless the daemon exposes them through an explicit
capabilities contract. Ensure the frontend no longer POSTs to routes that
src/daemon.js rejects unconditionally; if retained, gate each control and
request on the corresponding capability.
- Around line 671-733: Update the drawer and newPanel dialog lifecycle,
including the corresponding code around their open/close handlers, so closed
panels are removed from the accessibility and keyboard trees using native dialog
behavior or synchronized inert/aria-hidden state. When either panel opens, move
focus into it and contain Tab/Shift+Tab navigation within that panel; restore
focus to the triggering control on close and preserve equivalent behavior for
the additional dialog section noted in the comment.
- Around line 1468-1492: Update openDrawer to track the currently opened task
and ignore fetch results or errors that no longer match that task. Before
applying either the success or catch UI updates, verify the response belongs to
the current drawer task ID, or abort the previous request when opening a new
task, while preserving the existing transcript and fallback behavior for the
active task.
- Around line 1607-1623: Update the `#taskForm` submit handler to disable its
submit button immediately before awaiting post('/api/dispatch', body), keep it
disabled through success and error handling, and re-enable it when the request
settles so repeated submissions cannot enqueue duplicate tasks.
In `@src/daemon.js`:
- Around line 236-260: Update the result construction around cancelled,
workerExitedNonzero, invalidLoopResult, details.forceFailed, and
details.timedOut so every failed completion path persists a distinct
result.reason. Preserve the existing cancellation and nonzero-exit reasons, and
add corresponding reasons for timeout, forced failure, and invalid LOOP_RESULT
outcomes so dashboardRecentTask() can expose them.
- Around line 236-243: Update the status classification and related event/count
handling around cancelled, workerExitedNonzero, invalidLoopResult, and the
corresponding logic at the alternate range so user cancellations produce the
distinct `cancelled` status. Ensure cancelled tasks do not emit `fail` or
increment the daemon’s failed count, while preserving failure handling for force
failures, timeouts, nonzero exits, and invalid loop results.
In `@src/store.js`:
- Around line 165-172: Update appendLogLine to bound per-task log growth and
remove synchronous filesystem work from the worker-output path. Add
retention/rotation or enforce a per-task byte cap, and replace
ensureDirs()/appendFileSync() with queued asynchronous writes or a write stream
while preserving task-specific log paths and line formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6513b117-336e-466b-9170-a24f0e37c829
📒 Files selected for processing (3)
public/index.htmlsrc/daemon.jssrc/store.js
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
src/daemon.js (1)
src/prompts.js (1)
taskPrompt(13-15)
🪛 ast-grep (0.44.1)
src/store.js
[warning] 176-176: 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(logPath(id), 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
public/index.html
[error] 776-776: React's useState should not be directly called
Context: setTimeout(() => { el.textContent = ''; }, 4000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1030-1030: React's useState should not be directly called
Context: setOffline(!(s.daemon && s.daemon.alive))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1034-1034: React's useState should not be directly called
Context: setOffline(true)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1039-1039: React's useState should not be directly called
Context: setOffline(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1133-1133: React's useState should not be directly called
Context: setTimeout(() => el.classList.remove('ping'), 1600)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1225-1233: React's useState should not be directly called
Context: setTimeout(() => {
if ((armed.get(id) || 0) <= Date.now()) {
armed.delete(id);
if (btn.isConnected && !btn.disabled) {
btn.classList.remove('armed');
btn.textContent = 'Cancel';
}
}
}, 3100)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1529-1529: React's useState should not be directly called
Context: setTheme('tokyo-night')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1645-1645: React's useState should not be directly called
Context: setInterval(poll, 2000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 1646-1646: React's useState should not be directly called
Context: setInterval(tick, 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 738-740: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: String(s ?? '').replace(/[&<>"']/g, c => (
{'&':'&','<':'<','>':'>','"':'"',"'":'''}[c]
))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 812-818: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: menu.innerHTML = '
'' +
'' +
'' + esc(name) + '' +
'&
#10003;' +''
).join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1076-1076: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#slDaemon').innerHTML = left
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1078-1078: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#slDone').innerHTML = done === null ? '' : '' + done + ' done all-time'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1091-1106: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#blockedList').innerHTML = arr.map(x => {
const id = esc(x?.id);
return '
'
'waiting' +
'' + esc(x?.title || 'untitled') + '' +
'' + id + '' +
'
'
' + esc(x?.question || '(no question text)') + '
' +'
'<textarea id="ans-' + id + '" placeholder="Type your answer" aria-label="Answer for ' + esc(x?.title || id) + '"></textarea>' +
'Send answer' +
'
'
'';
}).join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1165-1185: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#runningList').innerHTML = arr.length ? arr.map(t => {
const id = esc(t?.id);
const isLoop = t?.type === 'loop';
const engine = t?.engine || 'codex';
return '
'
'' + (isLoop ? 'loop' : 'task') + '' +
'' + esc(t?.title || 'untitled') + '' +
'' + id + '' +
'Cancel' +
'
'
'' + esc(engine) + ':' + esc(t?.model || 'default') + '' +
'-' +
'' + (num(t?.toolCalls) ?? 0) + ' tool calls' +
(isLoop ? 'cycle ' + (num(t?.cycle) ?? '?') + '/' + (num(t?.maxCycles) ?? '?') + '' : '') +
'
(isLoop ? segbarHtml(t) : '') +
'
'';
}).join('') : hush('moon', 'All quiet - dispatch a task or start a loop.')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1242-1249: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#queueList').innerHTML = arr.length ? arr.map(t =>
'
'' + esc(t?.title || 'untitled') + '' +
'' + esc(t?.type === 'loop' ? 'loop' : (t?.model || 'task')) + '' +
'' + esc(t?.id) + '' +
'' + fmtRel(t?.createdAt) + '' +
'
).join('') : hush('inbox', 'Queue is clear.')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1269-1285: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#recentList').innerHTML = arr.length ? arr.map(t => {
const id = esc(t?.id);
const st = t?.status ?? t?.result?.status;
const sum = t?.summary ?? t?.result?.summary ?? '';
const [pc, pt] = statusPill(st);
return '
'' + esc(pt) + '' +
'
'
'
'
'
'
'
'
'
}).join('') : hush('check', 'Nothing finished yet.')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1320-1335: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: box.innerHTML = show.length ? show.map(m => {
const id = m?.id ?? '';
const role = roleOf(m?.from);
const fresh = pollN > 1 && seenMsg.get(id) === pollN;
const linked = !!(m?.taskId && blockedIds.has(m.taskId));
const wid = role === 'worker' ? (String(m?.from || '').split(':')[1] || '') : '';
return '<article class="bubble r-' + role + (fresh ? ' fresh' : '') + (linked ? ' linked' : '') + '"' +
' data-mid="' + esc(id) + '"' + (linked ? ' data-task="' + esc(m.taskId) + '" title="Jump to the question"' : '') + '>' +
'
'' + role + '' +
(wid ? '' + esc(wid) + '' : '') +
'' + fmtRel(m?.ts) + '' +
'new' +
(linked ? 'waiting for your answer' : '') +
'
}).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)
[warning] 1351-1355: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: $('#eventList').innerHTML = arr.length ? arr.map(e =>
'
'' + esc(e?.kind || 'info') + '' +
'' + esc(e?.text) + '
).join('') : '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1480-1480: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: body.innerHTML = lines.length ? renderTranscript(lines) : '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1486-1486: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: body.innerHTML = renderTranscript(MOCK_LOG)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 1645-1645: Avoid using the initial state variable in setState
Context: setInterval(poll, 2000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1646-1646: Avoid using the initial state variable in setState
Context: setInterval(tick, 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
src/daemon.js
[error] 371-375: React's useState should not be directly called
Context: setTimeout(() => {
timedOut = true;
captureLine(Worker timed out after ${timeoutMinutes} minutes.);
terminateWorker(child);
}, timeoutMinutes * 60 * 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 446-446: 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(resultPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🪛 HTMLHint (1.9.2)
public/index.html
[warning] 595-595: The type attribute must be present on elements.
(button-type-require)
[warning] 610-610: The type attribute must be present on
elements.(button-type-require)
[warning] 631-631: The type attribute must be present on
elements.(button-type-require)
[warning] 632-632: The type attribute must be present on
elements.(button-type-require)
[warning] 633-633: The type attribute must be present on
elements.(button-type-require)
[warning] 651-651: The type attribute must be present on
elements.(button-type-require)
[warning] 676-676: The type attribute must be present on
elements.(button-type-require)
[warning] 686-686: The type attribute must be present on
elements.(button-type-require)
[warning] 639-639: No matching [ label ] tag found.
(input-requires-label)
🔇 Additional comments (5)
src/store.js (3)
16-16: LGTM!Also applies to: 54-54, 319-320
161-167: 🔒 Security & PrivacyNo path traversal issue here.
/api/log/:idrejects anything outside[A-Za-z0-9_-]+, and task IDs are generated internally ast-<hex>, sologPath()never receives an untrusted value.> Likely an incorrect or invalid review comment.
175-183: 🚀 Performance & ScalabilityNeed bounded reads at the I/O boundary
readFileSync(...).split(...)still loads the entire log before slicing the last 200 lines. Iflimitcan come from user input, also guard it to a finite integer; otherwise keep the comment focused on the full-file read path.public/index.html (1)
2-580: LGTM!Also applies to: 581-616, 735-1017, 1045-1108, 1124-1261, 1289-1467, 1514-1557, 1575-1598, 1649-1654
src/daemon.js (1)
12-12: LGTM!Also applies to: 43-101, 327-382, 406-406, 444-528, 595-707
![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: 56aae137b3
ℹ️ 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".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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/daemon.js`:
- Around line 245-248: Update the reason-selection logic in finish() so
details.timedOut is evaluated before details.reason, ensuring a worker timeout
produces timed_out even when an earlier stdin failure supplied
worker_input_failed. Apply the same precedence change to the corresponding logic
in the additionally referenced section, while preserving existing reasons for
non-timeout outcomes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f1fde6b-631c-479e-b662-7903848504ab
📒 Files selected for processing (2)
public/index.htmlsrc/daemon.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
public/index.html
[warning] 1496-1496: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: body.innerHTML = lines.length ? renderTranscript(lines) : '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🔇 Additional comments (2)
public/index.html (1)
398-400: LGTM!Also applies to: 716-716, 740-740, 1014-1014, 1115-1115, 1273-1292, 1482-1501, 1512-1512, 1547-1549, 1622-1622, 1631-1648
src/daemon.js (1)
241-244: LGTM!Also applies to: 249-290, 327-353, 371-397, 427-427, 491-494
There was a problem hiding this comment.
Pull request overview
This PR updates AgentLoop’s local daemon + dashboard to a production-ready, single-file UI and expands the daemon/state contract to support per-task transcript logs and cancellation.
Changes:
- Added per-task NDJSON transcript logging in the store and exposed it via a new
/api/log/:idendpoint. - Implemented task cancellation and expanded daemon state payload to match the new dashboard’s nested contract.
- Introduced a new single-file dashboard (
public/index.html) with mock fallback + theme persistence.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/store.js | Adds state/logs directory plus append/read helpers for per-task NDJSON transcripts. |
| src/daemon.js | Records worker output to per-task logs, adds /api/log/:id + /api/cancel, and reshapes /api/state. |
| public/index.html | New single-file dashboard implementing the updated state contract, transcript drawer, and theme persistence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/daemon.js (1)
12-12: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCap stored log files on write
src/store.js::appendLogLine()appends every worker line to disk, andreadLogLines()only limits what the API returns. Add per-task or global rotation/byte limits so a noisy worker can’t fill local disk.🤖 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/daemon.js` at line 12, Update appendLogLine in src/store.js to enforce a per-task or global log rotation/size limit while writing worker output, rather than relying only on readLogLines truncation. Reuse the existing maxLogLines configuration where appropriate, and ensure older log content is discarded or rotated before continued appends so noisy workers cannot exhaust local disk.
🤖 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/daemon.js`:
- Around line 265-266: Update the timeout handling around spawnWorker() and
fallbackSummary() to compute the normalized task timeout once using the same
Math.max(1, Number(...) || 45) behavior, then reuse that value for both the
worker timer and timeout summary. Ensure invalid, missing, zero, or negative
configuration values produce matching durations in the dashboard and actual
wait.
---
Outside diff comments:
In `@src/daemon.js`:
- Line 12: Update appendLogLine in src/store.js to enforce a per-task or global
log rotation/size limit while writing worker output, rather than relying only on
readLogLines truncation. Reuse the existing maxLogLines configuration where
appropriate, and ensure older log content is discarded or rotated before
continued appends so noisy workers cannot exhaust local disk.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9833c4db-7d62-49dd-b8f9-4996817e6a4a
📒 Files selected for processing (1)
src/daemon.js
📜 Review details
🔇 Additional comments (1)
src/daemon.js (1)
43-101: LGTM!Also applies to: 236-264, 272-301, 329-342, 351-355, 356-372, 375-400, 422-429, 461-548, 586-597, 615-624, 635-701, 713-732
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d7f143fb
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/daemon.js`:
- Around line 543-545: Update the cancel branch in the event-building logic to
explicitly set the cancellation-specific kind, matching the distinct event kind
used by the dashboard for cancelled tasks, while preserving the existing text
and value fields.
- Line 697: Guard the store.appendEvent call in the async dispatch handler so
synchronous write failures are caught and handled without leaving the request
unresolved or crashing the daemon. Preserve the already-successful task enqueue
behavior, and ensure the handler sends an appropriate client response when
appendEvent fails.
- Around line 550-575: Update recentEvents() to avoid readFileSync loading the
entire unbounded events log on each poll. Implement a bounded tail read that
retrieves only enough recent lines or bytes to identify the last 30 events,
while preserving reverse chronological order, malformed-line skipping,
dashboardEvent transformation, and [] on read failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 28c09e97-a780-4c5d-b120-60818076a139
📒 Files selected for processing (1)
src/daemon.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/daemon.js
[error] 392-396: React's useState should not be directly called
Context: setTimeout(() => {
timedOut = true;
captureLine(Worker timed out after ${timeoutMin} minutes.);
terminateWorker(child);
}, timeoutMin * 60 * 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 551-551: 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(store.paths.events, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🔇 Additional comments (1)
src/daemon.js (1)
217-224: LGTM!Also applies to: 392-397
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d675ecd4f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/daemon.js (1)
517-549: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCancel events still map to generic
infokind — regression of previously fixed issue.Line 545 explicitly sets
kind: 'info'forcancelevents, matching the exact bug flagged in an earlier review round (cancelled events losing their distinct styling because they fall back toinfo), which was reportedly fixed in commitbefca60by usingkind: 'cancel'. In this rewrittendashboardEvent(), that fix appears to have been lost — every other terminal branch (queue,start,done,fail) gets a dedicated kind, butcancelis back to the indistinguishable default.🐛 Proposed fix
if (event.type === 'cancel') { - return { ...value, kind: 'info', text: `${task} cancelled${reason}` }; + return { ...value, kind: 'cancel', text: `${task} cancelled${reason}` }; }🤖 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/daemon.js` around lines 517 - 549, Update the cancel branch in dashboardEvent so cancelled events use the dedicated “cancel” kind instead of “info”; preserve the existing cancellation text and reason handling.
🤖 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.
Duplicate comments:
In `@src/daemon.js`:
- Around line 517-549: Update the cancel branch in dashboardEvent so cancelled
events use the dedicated “cancel” kind instead of “info”; preserve the existing
cancellation text and reason handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a5634fc-dd98-488f-9098-ed9da8397a2d
📒 Files selected for processing (2)
src/daemon.jssrc/store.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/daemon.js
[error] 393-397: React's useState should not be directly called
Context: setTimeout(() => {
timedOut = true;
captureLine(Worker timed out after ${timeoutMin} minutes.);
terminateWorker(child);
}, timeoutMin * 60 * 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🔇 Additional comments (4)
src/daemon.js (3)
218-226: LGTM!Also applies to: 393-398
551-600: LGTM!
602-641: LGTM!Also applies to: 691-725
src/store.js (1)
189-203: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: befca60373
ℹ️ 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".
What changed
Verification
node --check src\daemon.jsandnode --check src\store.js.Review fixes applied