feat(web-ui): webhook notifications and live terminal panel#137
feat(web-ui): webhook notifications and live terminal panel#137
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe PR introduces webhook notifications for configuration changes, adds a live WebSocket-based terminal output panel, and enhances derived session conversion with native import workflows. It includes new webhook and WebSocket server modules, updated session conversion logic to support multi-file output, and expanded Web UI functionality for managing derived/native sessions. ChangesWebhook Notification System
Terminal WebSocket Panel
Derived Session & Native Import Workflow
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Server as Web UI Server
participant WS as WebSocket (Terminal)
participant ChildProc as Child Process
participant FS as Filesystem
User->>Server: Request terminal execution<br/>runTerminalCommand('node ...')
Server->>WS: TCP connect + WebSocket handshake
WS->>Server: 101 Switching Protocols
Server->>Server: Create frame reader, marshal stdio
Server->>ChildProc: spawn(cmd, whitelist check)
ChildProc->>ChildProc: Execute
ChildProc-->>Server: stdout/stderr data
Server->>WS: Send data frame (stream: 'stdout')
WS-->>User: Display line in terminal panel
ChildProc-->>Server: Exit code
Server->>WS: Send exit frame (code: 0)
Server->>WS: Send close frame
WS-->>User: Terminal closed
sequenceDiagram
participant User as User
participant WebUI as Web UI
participant API as API Server
participant WebhookURL as Webhook Endpoint
User->>WebUI: Enable webhook + set URL
WebUI->>API: POST /set-webhook-config
API->>API: Save config
API-->>WebUI: { enabled: true, url: '...' }
User->>WebUI: Change Claude provider
WebUI->>API: POST /set-provider
API->>API: Process change
API->>WebhookURL: POST { event: 'provider-switch', ... }
WebhookURL-->>API: 200 OK
WebUI->>API: GET /get-sessions (includes events)
API-->>WebUI: Updated sessions
User->>WebUI: View webhook in UI (test result shown)
sequenceDiagram
participant User as User
participant WebUI as Web UI
participant API as API Server
participant FS as Filesystem
rect rgba(200, 150, 255, 0.5)
Note over User,FS: Convert Codex → Claude (Derived)
end
User->>WebUI: Click convert (outputDir=derived)
WebUI->>API: POST /convert-session (outputDir=derived)
API->>FS: Compute derived output path
API->>FS: Write session.jsonl + session.meta.json
API-->>WebUI: { derived: true, nativePath: '...', ... }
rect rgba(100, 200, 150, 0.5)
Note over User,FS: Import Derived → Native
end
User->>WebUI: Click "Import to Native"
WebUI->>API: POST /import-derived-session (filePath)
API->>API: Validate derived source (codex/claude)
API->>FS: Resolve native target path
API->>FS: Check EEXIST (conflict check)
API->>FS: Write to native directory
API-->>WebUI: { success: true, path: '...', ... }
WebUI->>WebUI: Update sessions list + state
User->>WebUI: See native session in list (derived flag removed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR introduces three distinct feature systems (webhook, terminal, derived session workflows) affecting the CLI, WebSocket server, Web UI methods, UI panels, styling, and tests. The changes span heterogeneous areas: new WebSocket frame encoding logic, webhook HTTP client, session conversion metadata, import workflows with error handling, and localization. Reviewers must validate correct WebSocket framing, webhook payload structure and delivery, multi-file session output and metadata versioning, native/derived session path resolution and conflict detection, and UI state coherence across multiple new data properties and methods. The complexity is increased by cross-cutting concerns (session metadata propagation, API error handling, localization coverage) and the need to verify WebSocket security (command whitelisting) and session conflict prevention. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/app.js (1)
620-645:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
terminalSocketis never closed inbeforeUnmount— resource and subprocess leak.
stopTaskOrchestrationPolling(),_initialLoadTimer, and other resource handles are all cleaned up inbeforeUnmount, butterminalSocketis missing. If the terminal panel is running when the component unmounts (HMR, SPA navigation, test teardown), the WebSocket stays open, the server-side subprocess may remain alive, and no exit frame is dispatched to the backend.🔒 Proposed fix in
beforeUnmountthis.stopTaskOrchestrationPolling(); + if (this.terminalSocket) { + try { this.terminalSocket.close(); } catch (_) {} + this.terminalSocket = null; + } this.sessionPreviewScrollEl = null;🤖 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 `@web-ui/app.js` around lines 620 - 645, beforeUnmount currently cleans many resources but never closes the terminal WebSocket, leaving server subprocesses alive; update beforeUnmount to check for this. If this.terminalSocket exists, send the terminal-exit/close frame required by our terminal protocol (or at minimum call this.terminalSocket.close()), remove any terminal-specific event listeners, and set this.terminalSocket = null so the socket is cleaned up; ensure the send/close is done before nulling to guarantee the backend receives the exit frame.
🧹 Nitpick comments (3)
web-ui/partials/index/panel-config-claude.html (1)
143-170: 💤 Low valueAll webhook UI strings bypass
t()— breaks EN locale consistencyEvery other user-facing string in this panel is routed through
t(), but the entire webhook section uses hard-coded Chinese literals (lines 143, 146, 163, 165, 168–169). This prevents localization and is inconsistent with the rest of the file.♻️ Suggested approach
Add the missing keys to
web-ui/modules/i18n.dict.mjs(zh + en entries) and replace each hard-coded string:- <div class="config-template-hint">配置变更（provider 切换 / CLAUDE.md 编辑）外发到 Slack/飞书/自定义 webhook。</div> + <div class="config-template-hint">{{ t('webhook.hint') }}</div> <label class="webhook-row"> <input type="checkbox" v-model="webhookConfig.enabled" `@change`="saveWebhookSettings"> - <span>启用 webhook 通知</span> + <span>{{ t('webhook.enable') }}</span> </label> ... <button type="button" class="btn-mini" `@click`="testWebhook" :disabled="webhookTesting || !webhookConfig.url"> - {{ webhookTesting ? '测试中…' : '发送测试 ping' }} + {{ webhookTesting ? t('webhook.testing') : t('webhook.test') }} </button> - <button type="button" class="btn-mini" `@click`="loadWebhookSettings">重新加载</button> + <button type="button" class="btn-mini" `@click`="loadWebhookSettings">{{ t('webhook.reload') }}</button>🤖 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 `@web-ui/partials/index/panel-config-claude.html` around lines 143 - 170, The webhook UI strings are hard-coded in the template (references: webhookConfig, webhookEventOptions, webhookTesting, testWebhook, loadWebhookSettings, webhookTestResult) and must be wrapped with the i18n helper t(); add corresponding keys for each visible label/button/message to web-ui/modules/i18n.dict.mjs (both zh and en entries), then replace the literal Chinese strings in the template with t('your.key') calls (including checkbox label, placeholder, "启用 webhook 通知", "发送测试 ping"/"测试中…", "重新加载", and the success/error message fragments) so the panel uses localized text via t().web-ui/partials/index/panel-terminal.html (1)
32-34: 💤 Low valueRedundant
indexOfcall inside:class— minor simplification availableThe
v-ifon line 32 already ensures that whenterminalSearchQueryis truthy theindexOfresult is!== -1. The:classbinding therefore recalculates it unnecessarily; the ternary simplifies toterminalSearchQuery ? 'terminal-line-match' : ''.♻️ Proposed simplification
-<span v-if="!terminalSearchQuery || line.text.toLowerCase().indexOf(terminalSearchQuery.toLowerCase()) !== -1" - :class="['terminal-line', 'terminal-stream-' + line.stream, (terminalSearchQuery && line.text.toLowerCase().indexOf(terminalSearchQuery.toLowerCase()) !== -1) ? 'terminal-line-match' : '']">{{ line.text }} +<span v-if="!terminalSearchQuery || line.text.toLowerCase().indexOf(terminalSearchQuery.toLowerCase()) !== -1" + :class="['terminal-line', 'terminal-stream-' + line.stream, terminalSearchQuery ? 'terminal-line-match' : '']">{{ line.text }}🤖 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 `@web-ui/partials/index/panel-terminal.html` around lines 32 - 34, The class-binding redundantly recalculates indexOf even though the v-if already guarantees a match when terminalSearchQuery is truthy; in the template iterating over terminalLines (the <template v-for="(line, idx) in terminalLines"> and its v-if using terminalSearchQuery and line.text.indexOf), replace the ternary inside :class that checks indexOf with a simple check of terminalSearchQuery (e.g. terminalSearchQuery ? 'terminal-line-match' : '') so the 'terminal-line-match' class is applied when a search query exists without re-evaluating indexOf.tests/e2e/test-session-convert-derived.js (1)
141-149: ⚡ Quick winConsider asserting
imported.filePathis a native Codex session path.Line 144 only checks that
imported.filePathis non-empty and differs from the derived source path. A broken implementation that copies to an arbitrary location would satisfy this assertion. AddingisCodexSessionPath(tmpHome, imported.filePath)aligns the assertion with the checks used elsewhere in this helper.🧪 Suggested assertion addition
assert(imported.filePath && imported.filePath !== legacyDerivedCodexPath, 'import-derived-session should copy to native path'); +assert(isCodexSessionPath(tmpHome, imported.filePath), 'imported native codex session should reside in a codex sessions dir'); assert(fs.existsSync(imported.filePath), 'imported native codex session file missing');🤖 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 `@tests/e2e/test-session-convert-derived.js` around lines 141 - 149, Add an assertion that verifies the imported.filePath is a native Codex session path (not just non-empty/different) by calling the existing helper isCodexSessionPath(tmpHome, imported.filePath) after the import-derived-session call; ensure tmpHome is in scope or use the same temp home used elsewhere in this test, so the test asserts the import copied to a valid native Codex session location rather than any arbitrary path.
🤖 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 `@cli.js`:
- Around line 11149-11150: The upgrade handler is dropping the initial bytes
because the listener ignores the upgrade "head"; modify the upgrade flow so the
raw head is preserved and passed into the WebSocket handling: update
performWsHandshake(req, socket) to accept the head (performWsHandshake(req,
socket, head)) or ensure the head is pushed back onto the socket (e.g.,
socket.unshift(head)) before attaching the session, and then pass the head (or
use the pushed-back data) into attachTerminalSession(socket) or the WebSocket
server's handleUpgrade call so the client's first frame is not lost; locate and
update both performWsHandshake and attachTerminalSession call sites (also apply
similar changes in the other occurrences around the 10078-10130 range).
- Around line 11129-11148: Currently the code uses isLoopback to skip auth for
loopback sockets; change this so the token check always runs for the
`/ws/terminal` connection: always read process.env.CODEXMATE_HTTP_TOKEN, extract
token from req.headers.authorization or req.headers['x-codexmate-token'] (same
logic already in the diff) and compare it to expected, and only destroy the
socket and return on mismatch; remove or disable the branch that bypasses
validation when isLoopback is true. Optionally, if you want defense-in-depth,
also validate req.headers.origin against an allowlist and require a CSRF/session
token tied to the Web UI page load, but the immediate fix is to stop bypassing
auth for loopback by making the token validation unconditional.
- Around line 10055-10061: The allowlist check in isTerminalWsCommandAllowed
(and the duplicate at the other occurrence) uses path.basename so path-prefixed
binaries like "/tmp/node" bypass the whitelist; fix by extracting the command's
first token (split trimmed on whitespace) and immediately rejecting if that
token contains any path separators or is path-prefixed (e.g., starts with '.' or
has a dirname other than '.'), then compare the plain basename against
TERMINAL_WS_ALLOWED_COMMAND_BASENAMES; apply the same change to the other
function at 10096-10101 to ensure only bare command names (no paths) are
allowed.
- Around line 7144-7153: The code builds outputPath/metaPath using unsanitized
baseSessionId when outputDirMode === 'native', allowing path traversal; fix by
sanitizing or constraining baseSessionId before joining: normalize the session
id to a safe filename (e.g., strip path separators, reject '..', or use
path.basename/URL-encode) and then compute outputPath/metaPath via
path.resolve(outputDir, safeSessionId + '.jsonl') and path.resolve(... +
'.meta.json'); after resolving, verify both resolved paths start with the
resolved outputDir (e.g., resolvedOutputDir = path.resolve(outputDir) and check
startsWith) and return an error if validation fails, updating the logic around
derivedSessionId, outputPath, metaPath, and the exists checks accordingly.
- Around line 6452-6461: The resolveNativeSessionFilePath function currently
uses sessionId directly in path.join which allows path traversal; sanitize
sessionId (from the variable id) before using it by rejecting or normalizing any
input containing path separators or upward traversal (e.g., '..') and/or
reducing it to a safe basename or a validated token (only allow alphanumerics,
hyphen/underscore) and fall back to returning '' on invalid input; update the
logic around id and the two uses in the codex and projectDir path.join calls so
only the sanitized_safe_id is interpolated.
In `@cli/session-convert-args.js`:
- Around line 44-45: The CLI option parsing for --output-dir currently treats a
missing value or an empty value (from "--output-dir" with no following arg or
"--output-dir=") as a default "native"; update the parsing logic around
options.outputDir to explicitly reject empty values: when encountering
"--output-dir" ensure `next` exists and is non-empty before assigning to
options.outputDir (otherwise throw a usage error/exit non-zero), and when
arg.startsWith("--output-dir=") ensure arg.slice(13) is non-empty before
assigning (otherwise throw a usage error/exit non-zero); preserve existing
behavior when a valid non-empty value is provided.
In `@lib/cli-webhook.js`:
- Around line 19-21: normalizeConfig currently replaces an explicitly empty
cfg.events ([]) with ALLOWED_EVENTS; change the branch that handles
Array.isArray(cfg.events) so it preserves an empty array: if cfg.events.length
=== 0 set out.events = [] (preserve the explicit "no events" choice), otherwise
compute filtered = cfg.events.filter(e => ALLOWED_EVENTS.indexOf(e) !== -1) and
set out.events = filtered.length ? filtered : ALLOWED_EVENTS.slice(); reference
normalizeConfig, cfg.events, out.events, ALLOWED_EVENTS and the filtered
variable to locate the change.
In `@lib/cli-ws-server.js`:
- Around line 80-123: makeFrameReader currently ignores Ping frames and drops
fragmented messages; change its signature to accept a sendFrame/sendPong
callback (e.g., makeFrameReader(onMessage, onClose, sendFrame)) and implement
handling for ping (opcode 0x9) by immediately sending a Pong (opcode 0xA) with
the identical payload via sendFrame; also add fragmentation buffering: track an
ongoing fragment state (e.g., currentFragmentOpcode and fragmentBuffers) so that
when a non-FIN frame (FIN bit b0 & 0x80 === 0) with opcode 0x1/0x2 starts you
append payloads and treat continuation frames (opcode 0x0) as part of that
buffer, and only dispatch to onMessage when a frame with FIN set completes
(decode to UTF-8 for text opcode 0x1, return Buffer for binary opcode 0x2); keep
handling for close (0x8) and safely ignore pong (0xA).
In `@tests/unit/compact-layout-ui.test.mjs`:
- Line 67: The current assertion on the styles string uses a greedy regex and
the wrong breakpoint value, so it can match a .session-item rule from a
different media block; update the assert.match that references styles to use the
actual mobile breakpoint value used in the stylesheet (replace 720px with the
project breakpoint) and make the media-block match non-greedy (use [\s\S]*?
between the media query and the .session-item rules) so the regex ensures the
.session-item min-height/height/contain-intrinsic-size are inside that exact
`@media` (max-width: Xpx) block.
In `@web-ui/modules/app.methods.session-actions.mjs`:
- Around line 31-42: The two helpers disagree because
isSessionNativeAvailable(session) currently treats a missing
session.nativeAvailable as true; change the logic so native availability is only
true when session.nativeAvailable === true (i.e., treat missing/undefined as
false). Update isSessionNativeAvailable to return Boolean(session && typeof
session === 'object' && session.nativeAvailable === true) and ensure
isImportToNativeAvailable continues to require this.isDerivedSession(session)
and that session.nativeAvailable !== true and session.nativeImportAvailable !==
false; reference the functions isSessionNativeAvailable,
isImportToNativeAvailable, isDerivedSession and the fields
session.nativeAvailable and session.nativeImportAvailable when making the
change.
In `@web-ui/modules/app.methods.webhook-terminal.mjs`:
- Around line 8-15: The current assignment to this.webhookConfig.events treats
an empty saved array as falsy and replaces it with this.webhookEventOptions,
preventing round-tripping an intentionally empty selection; change the logic in
the webhook config initialization (where this.webhookConfig is set and
webhookEventOptions is referenced) so that if data.events is an actual array
(even if length === 0) you use data.events.slice(), and only fall back to
this.webhookEventOptions.slice() when data.events is not an array or is
undefined.
- Around line 126-128: The socket URL construction using window.location drops
custom host/port and breaks file:// fallbacks; change the logic in
web-ui/modules/app.methods.webhook-terminal.mjs to build the WebSocket URL from
the existing API_BASE (or fall back to the previous window.location logic)
instead of using window.location.host directly: parse API_BASE (or new
URL(API_BASE)) to determine protocol (map http->ws, https->wss), host, and port,
then append '/ws/terminal' to produce url; update the variables proto, host, and
url usage so the terminal connects to the same backend endpoint as the HTTP
client.
---
Outside diff comments:
In `@web-ui/app.js`:
- Around line 620-645: beforeUnmount currently cleans many resources but never
closes the terminal WebSocket, leaving server subprocesses alive; update
beforeUnmount to check for this. If this.terminalSocket exists, send the
terminal-exit/close frame required by our terminal protocol (or at minimum call
this.terminalSocket.close()), remove any terminal-specific event listeners, and
set this.terminalSocket = null so the socket is cleaned up; ensure the
send/close is done before nulling to guarantee the backend receives the exit
frame.
---
Nitpick comments:
In `@tests/e2e/test-session-convert-derived.js`:
- Around line 141-149: Add an assertion that verifies the imported.filePath is a
native Codex session path (not just non-empty/different) by calling the existing
helper isCodexSessionPath(tmpHome, imported.filePath) after the
import-derived-session call; ensure tmpHome is in scope or use the same temp
home used elsewhere in this test, so the test asserts the import copied to a
valid native Codex session location rather than any arbitrary path.
In `@web-ui/partials/index/panel-config-claude.html`:
- Around line 143-170: The webhook UI strings are hard-coded in the template
(references: webhookConfig, webhookEventOptions, webhookTesting, testWebhook,
loadWebhookSettings, webhookTestResult) and must be wrapped with the i18n helper
t(); add corresponding keys for each visible label/button/message to
web-ui/modules/i18n.dict.mjs (both zh and en entries), then replace the literal
Chinese strings in the template with t('your.key') calls (including checkbox
label, placeholder, "启用 webhook 通知", "发送测试 ping"/"测试中…", "重新加载", and the
success/error message fragments) so the panel uses localized text via t().
In `@web-ui/partials/index/panel-terminal.html`:
- Around line 32-34: The class-binding redundantly recalculates indexOf even
though the v-if already guarantees a match when terminalSearchQuery is truthy;
in the template iterating over terminalLines (the <template v-for="(line, idx)
in terminalLines"> and its v-if using terminalSearchQuery and
line.text.indexOf), replace the ternary inside :class that checks indexOf with a
simple check of terminalSearchQuery (e.g. terminalSearchQuery ?
'terminal-line-match' : '') so the 'terminal-line-match' class is applied when a
search query exists without re-evaluating indexOf.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4743cb6-0b05-47f0-a8d9-a5013394987f
📒 Files selected for processing (30)
cli.jscli/session-convert-args.jscli/session-convert.jslib/cli-webhook.jslib/cli-ws-server.jstests/e2e/run.jstests/e2e/test-session-convert-derived.jstests/e2e/test-terminal.jstests/e2e/test-webhook.jstests/unit/compact-layout-ui.test.mjstests/unit/session-convert.test.mjstests/unit/session-header-actions-layout.test.mjstests/unit/session-resume-command.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.methods.codex-config.mjsweb-ui/modules/app.methods.index.mjsweb-ui/modules/app.methods.session-actions.mjsweb-ui/modules/app.methods.webhook-terminal.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-config-claude.htmlweb-ui/partials/index/panel-sessions.htmlweb-ui/partials/index/panel-terminal.htmlweb-ui/session-helpers.mjsweb-ui/styles.cssweb-ui/styles/responsive.cssweb-ui/styles/sessions-list.cssweb-ui/styles/sessions-toolbar-trash.cssweb-ui/styles/terminal-webhook.css
📜 Review details
🧰 Additional context used
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-terminal.html
[error] 2-2: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (18)
web-ui/styles/responsive.css (1)
235-238: Consistent mobile sizing update for session items.Line [235]-Line [238] keeps
min-height,height, andcontain-intrinsic-sizealigned, which helps avoid layout jumps on small screens.web-ui/styles/sessions-list.css (1)
208-210: Baseline session-item size bump looks coherent.Line [208] and Line [210] are consistent and keep intrinsic sizing in sync with the visual min height.
web-ui/styles/terminal-webhook.css (1)
1-109: Terminal/webhook style surface is cleanly scoped and complete.The new classes are cohesive and avoid broad/global selector collisions.
web-ui/styles.css (1)
22-22: New terminal/webhook stylesheet is correctly registered.Line [22] ensures the feature styles are bundled with the main UI CSS.
web-ui/styles/sessions-toolbar-trash.css (1)
226-238: Derived-session badge styling is well-structured.Line [226]-Line [238] introduces a readable, nowrap badge style that should integrate smoothly with session metadata rows.
tests/unit/session-convert.test.mjs (1)
37-43: Meta-file assertions are a solid contract upgrade.These checks validate both file emission and metadata semantics for each conversion direction, which improves regression safety.
Also applies to: 70-76
tests/e2e/run.js (1)
164-167: 💤 Low valueTest ordering disagrees with the PR/AI summary.
The summary states webhook and terminal tests run "after test-task orchestration and before test-install-status," but here they're invoked after
testInstallStatus(line 164) and before thetestWebUi*block. The sequencing in code looks fine in itself; only flagging so the description/changelog can be reconciled with what actually executes.web-ui/index.html (1)
26-26: LGTM!The terminal panel include is placed correctly between panel-plugins and layout-footer, consistent with the existing include ordering convention.
web-ui/session-helpers.mjs (1)
428-445: LGTM!The new field hydration follows the same guarded-assignment pattern used for all other session fields above it. Using
typeof === 'boolean'for the boolean flags correctly preserves explicitfalse, and the truthiness guards for path/label fields are consistent with existing usage.tests/e2e/test-terminal.js (1)
73-125: LGTM!Clean structure for a raw-TCP WebSocket E2E test. The RFC6455 handshake validation, masked client frames, and
waitForFramepolling pattern are all correct. Good coverage of command whitelist rejection (test 2) and stderr routing (test 3).tests/unit/session-resume-command.test.mjs (1)
11-13: LGTM!The
DICTimport viapathToFileURLis consistent with thecreateSessionActionMethodsimport on lines 8–10. Good addition of comprehensive i18n key coverage and locale-aware tests.tests/e2e/test-webhook.js (1)
56-144: LGTM!Solid end-to-end coverage: config round-trip, test-ping delivery verification, provider-switch event validation with summary/timestamp checks, disabled-webhook and event-filter suppression. The
finallyblock correctly tears down both the webhook config and the recording server regardless of assertion failures.web-ui/partials/index/panel-sessions.html (1)
241-247: ⚡ Quick winNo issue found.
sessionImportingNativeis properly initialized as{}inapp.js(line 200), so the template access at line 245 is safe fromTypeError. The proposed safeguard is unnecessary.> Likely an incorrect or invalid review comment.web-ui/partials/index/panel-config-claude.html (1)
167-170: This branch is not dead code—remove this concernThe search results confirm that
webhookTestResult.statusis explicitly set to'saved'on line 37 ofapp.methods.webhook-terminal.mjswithin the save handler, not the test handler. The conditionwebhookTestResult.status === 'saved'is therefore reachable and will correctly display "已保存" after a successful save operation.> Likely an incorrect or invalid review comment.web-ui/app.js (2)
200-200: LGTM — consistent with the existing per-session tracking map pattern.
417-417:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply
markRawto prevent Vue from proxying the WebSocket object.When
this.terminalSocket = socketexecutes at line 136 inweb-ui/modules/app.methods.webhook-terminal.mjs, Vue 3's reactivity system wraps the new WebSocket in a Proxy. WebSocket is a non-plain object where proxy wrapping causes unnecessary tracking overhead and can trigger spurious re-render dependencies. UsemarkRaw()at the assignment to tell Vue "do not track this object."Fix at the assignment site
- this.terminalSocket = socket; + this.terminalSocket = Vue.markRaw(socket);Or, if importing from Vue:
+import { markRaw } from 'vue'; import { api, API_BASE } from './api.mjs'; ... - this.terminalSocket = socket; + this.terminalSocket = markRaw(socket);> Likely an incorrect or invalid review comment.tests/unit/web-ui-behavior-parity.test.mjs (2)
334-347: LGTM — parity baseline accurately mirrors the new data and method surface.All added keys (
webhookConfig,webhookEventOptions,webhookSaving,webhookTestResult,webhookTesting,terminalPanelOpen,terminalLines,terminalPaused,terminalSearchQuery,terminalCommandInput,terminalRunning,terminalSocket,terminalPendingBuffer,terminalMaxLines) align with the state added inweb-ui/app.js. Placement in the HEAD-only branch is correct since the non-HEAD baseline predates these fields.
456-467: LGTM — new method keys correctly enumerate the webhook/terminal public API surface.
| function resolveNativeSessionFilePath(source, sessionId, cwd = '') { | ||
| const normalizedSource = normalizeSessionDerivedTarget(source); | ||
| const id = typeof sessionId === 'string' ? sessionId.trim() : ''; | ||
| if (!normalizedSource || !id) return ''; | ||
| if (normalizedSource === 'codex') { | ||
| return path.join(getCodexSessionsDir(), `${id}.jsonl`); | ||
| } | ||
| const projectDir = resolveClaudeProjectDirForCwd(cwd || '') || path.join(getClaudeProjectsDir(), 'codexmate-derived'); | ||
| return path.join(projectDir, `${id}.jsonl`); | ||
| } |
There was a problem hiding this comment.
Sanitize sessionId before building native file paths
sessionId is used directly in path.join(...). If it contains ../ or separators, native-path resolution can escape the intended root and target arbitrary files.
Suggested fix
+function sanitizeSessionIdForPath(value) {
+ const raw = typeof value === 'string' ? value.trim() : '';
+ if (!raw) return '';
+ const safe = raw.replace(/[^a-zA-Z0-9._-]/g, '_').slice(0, 120);
+ return safe || '';
+}
+
function resolveNativeSessionFilePath(source, sessionId, cwd = '') {
const normalizedSource = normalizeSessionDerivedTarget(source);
- const id = typeof sessionId === 'string' ? sessionId.trim() : '';
+ const id = sanitizeSessionIdForPath(sessionId);
if (!normalizedSource || !id) return '';
if (normalizedSource === 'codex') {
- return path.join(getCodexSessionsDir(), `${id}.jsonl`);
+ const root = getCodexSessionsDir();
+ const candidate = path.resolve(path.join(root, `${id}.jsonl`));
+ return isPathInside(candidate, root) ? candidate : '';
}
const projectDir = resolveClaudeProjectDirForCwd(cwd || '') || path.join(getClaudeProjectsDir(), 'codexmate-derived');
- return path.join(projectDir, `${id}.jsonl`);
+ const candidate = path.resolve(path.join(projectDir, `${id}.jsonl`));
+ return isPathInside(candidate, projectDir) ? candidate : '';
}🤖 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 `@cli.js` around lines 6452 - 6461, The resolveNativeSessionFilePath function
currently uses sessionId directly in path.join which allows path traversal;
sanitize sessionId (from the variable id) before using it by rejecting or
normalizing any input containing path separators or upward traversal (e.g.,
'..') and/or reducing it to a safe basename or a validated token (only allow
alphanumerics, hyphen/underscore) and fall back to returning '' on invalid
input; update the logic around id and the two uses in the codex and projectDir
path.join calls so only the sanitized_safe_id is interpolated.
| let derivedSessionId = ''; | ||
| let outputPath = ''; | ||
| let metaPath = ''; | ||
| if (outputDirMode === 'native') { | ||
| derivedSessionId = baseSessionId; | ||
| outputPath = path.join(outputDir, `${derivedSessionId}.jsonl`); | ||
| metaPath = path.join(outputDir, `${derivedSessionId}.meta.json`); | ||
| if (fs.existsSync(outputPath) || fs.existsSync(metaPath)) { | ||
| return { error: 'Converted sessionId conflicts with an existing native session; please retry or choose derived output.' }; | ||
| } |
There was a problem hiding this comment.
outputDir=native path creation is traversal-prone
In native mode, baseSessionId is written into file paths without sanitization. A crafted ID can break out of outputDir and overwrite arbitrary paths.
Suggested fix
- if (outputDirMode === 'native') {
- derivedSessionId = baseSessionId;
- outputPath = path.join(outputDir, `${derivedSessionId}.jsonl`);
- metaPath = path.join(outputDir, `${derivedSessionId}.meta.json`);
+ if (outputDirMode === 'native') {
+ derivedSessionId = sanitizeSessionIdForPath(baseSessionId);
+ if (!derivedSessionId) {
+ return { error: 'Invalid sessionId for native output' };
+ }
+ outputPath = path.resolve(path.join(outputDir, `${derivedSessionId}.jsonl`));
+ metaPath = path.resolve(path.join(outputDir, `${derivedSessionId}.meta.json`));
+ if (!isPathInside(outputPath, outputDir) || !isPathInside(metaPath, outputDir)) {
+ return { error: 'Invalid native output path' };
+ }
if (fs.existsSync(outputPath) || fs.existsSync(metaPath)) {
return { error: 'Converted sessionId conflicts with an existing native session; please retry or choose derived output.' };
}🤖 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 `@cli.js` around lines 7144 - 7153, The code builds outputPath/metaPath using
unsanitized baseSessionId when outputDirMode === 'native', allowing path
traversal; fix by sanitizing or constraining baseSessionId before joining:
normalize the session id to a safe filename (e.g., strip path separators, reject
'..', or use path.basename/URL-encode) and then compute outputPath/metaPath via
path.resolve(outputDir, safeSessionId + '.jsonl') and path.resolve(... +
'.meta.json'); after resolving, verify both resolved paths start with the
resolved outputDir (e.g., resolvedOutputDir = path.resolve(outputDir) and check
startsWith) and return an error if validation fails, updating the logic around
derivedSessionId, outputPath, metaPath, and the exists checks accordingly.
| function isTerminalWsCommandAllowed(cmd) { | ||
| if (!cmd || typeof cmd !== 'string') return false; | ||
| const trimmed = cmd.trim(); | ||
| if (!trimmed) return false; | ||
| const base = path.basename(trimmed).toLowerCase(); | ||
| return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base); | ||
| } |
There was a problem hiding this comment.
Command allowlist can be bypassed via path-prefixed binaries
Using path.basename(cmd) means /tmp/node or ./codexmate passes the allowlist. That defeats the whitelist intent.
Suggested fix
function isTerminalWsCommandAllowed(cmd) {
if (!cmd || typeof cmd !== 'string') return false;
const trimmed = cmd.trim();
if (!trimmed) return false;
- const base = path.basename(trimmed).toLowerCase();
- return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base);
+ if (trimmed.includes('/') || trimmed.includes('\\')) return false;
+ const base = trimmed.toLowerCase();
+ return TERMINAL_WS_ALLOWED_COMMAND_BASENAMES.has(base);
}Also applies to: 10096-10101
🤖 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 `@cli.js` around lines 10055 - 10061, The allowlist check in
isTerminalWsCommandAllowed (and the duplicate at the other occurrence) uses
path.basename so path-prefixed binaries like "/tmp/node" bypass the whitelist;
fix by extracting the command's first token (split trimmed on whitespace) and
immediately rejecting if that token contains any path separators or is
path-prefixed (e.g., starts with '.' or has a dirname other than '.'), then
compare the plain basename against TERMINAL_WS_ALLOWED_COMMAND_BASENAMES; apply
the same change to the other function at 10096-10101 to ensure only bare command
names (no paths) are allowed.
| const remoteAddr = socket && socket.remoteAddress ? socket.remoteAddress : ''; | ||
| const isLoopback = !remoteAddr | ||
| || remoteAddr === '127.0.0.1' | ||
| || remoteAddr === '::1' | ||
| || remoteAddr === '::ffff:127.0.0.1'; | ||
| if (!isLoopback) { | ||
| const expected = typeof process.env.CODEXMATE_HTTP_TOKEN === 'string' | ||
| ? process.env.CODEXMATE_HTTP_TOKEN.trim() | ||
| : ''; | ||
| const headers = req && req.headers ? req.headers : {}; | ||
| const auth = typeof headers.authorization === 'string' ? headers.authorization.trim() : ''; | ||
| const match = auth ? auth.match(/^bearer\s+(.+)$/i) : null; | ||
| const token = match && match[1] | ||
| ? match[1].trim() | ||
| : (typeof headers['x-codexmate-token'] === 'string' ? String(headers['x-codexmate-token']).trim() : ''); | ||
| if (!expected || token !== expected) { | ||
| try { socket.destroy(); } catch (_) {} | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Loopback trust on /ws/terminal enables cross-site websocket command execution
The endpoint skips token checks for loopback clients. Any website opened in the user’s browser can initiate a WebSocket to 127.0.0.1 and send terminal commands (including node ...).
Require explicit auth for terminal WS even on loopback (and/or strict Origin validation plus CSRF-style session token tied to the Web UI page load).
🤖 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 `@cli.js` around lines 11129 - 11148, Currently the code uses isLoopback to
skip auth for loopback sockets; change this so the token check always runs for
the `/ws/terminal` connection: always read process.env.CODEXMATE_HTTP_TOKEN,
extract token from req.headers.authorization or req.headers['x-codexmate-token']
(same logic already in the diff) and compare it to expected, and only destroy
the socket and return on mismatch; remove or disable the branch that bypasses
validation when isLoopback is true. Optionally, if you want defense-in-depth,
also validate req.headers.origin against an allowlist and require a CSRF/session
token tied to the Web UI page load, but the immediate fix is to stop bypassing
auth for loopback by making the token validation unconditional.
| if (!performWsHandshake(req, socket)) return; | ||
| attachTerminalSession(socket); |
There was a problem hiding this comment.
Upgrade head bytes are dropped, which can lose the first WS frame
server.on('upgrade') ignores head; if the client’s first frame arrives with upgrade, the command can be lost.
Suggested fix
-function attachTerminalSession(socket) {
+function attachTerminalSession(socket, initialHead = null) {
let child = null;
@@
const reader = makeWsFrameReader((message) => {
@@
}, close);
socket.on('data', reader);
+ if (initialHead && initialHead.length > 0) {
+ reader(initialHead);
+ }
}
@@
- if (!performWsHandshake(req, socket)) return;
- attachTerminalSession(socket);
+ if (!performWsHandshake(req, socket)) return;
+ attachTerminalSession(socket, head);
});Also applies to: 10078-10130
🤖 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 `@cli.js` around lines 11149 - 11150, The upgrade handler is dropping the
initial bytes because the listener ignores the upgrade "head"; modify the
upgrade flow so the raw head is preserved and passed into the WebSocket
handling: update performWsHandshake(req, socket) to accept the head
(performWsHandshake(req, socket, head)) or ensure the head is pushed back onto
the socket (e.g., socket.unshift(head)) before attaching the session, and then
pass the head (or use the pushed-back data) into attachTerminalSession(socket)
or the WebSocket server's handleUpgrade call so the client's first frame is not
lost; locate and update both performWsHandshake and attachTerminalSession call
sites (also apply similar changes in the other occurrences around the
10078-10130 range).
| assert.match(styles, /\.session-item\s*\{[\s\S]*min-height:\s*80px;/); | ||
| assert.match(styles, /\.session-item\s*\{[\s\S]*min-height:\s*84px;/); | ||
| assert.match(styles, /\.session-item\s*\{[\s\S]*contain-intrinsic-size:\s*84px;/); | ||
| assert.match(styles, /@media\s*\(max-width:\s*720px\)\s*\{[\s\S]*\.session-item\s*\{[\s\S]*min-height:\s*79px;[\s\S]*height:\s*79px;[\s\S]*contain-intrinsic-size:\s*79px;/); |
There was a problem hiding this comment.
Tighten the media-query assertion to the actual mobile breakpoint.
Line [67] currently checks max-width: 720px with a broad cross-block regex, so it can pass by matching .session-item rules from a different media block. This weakens the guarantee the test is trying to provide.
Suggested test fix
- assert.match(styles, /@media\s*\(max-width:\s*720px\)\s*\{[\s\S]*\.session-item\s*\{[\s\S]*min-height:\s*79px;[\s\S]*height:\s*79px;[\s\S]*contain-intrinsic-size:\s*79px;/);
+ assert.match(styles, /@media\s*\(max-width:\s*540px\)\s*\{[\s\S]*\.session-item\s*\{[\s\S]*min-height:\s*79px;[\s\S]*height:\s*79px;[\s\S]*contain-intrinsic-size:\s*79px;/);📝 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.
| assert.match(styles, /@media\s*\(max-width:\s*720px\)\s*\{[\s\S]*\.session-item\s*\{[\s\S]*min-height:\s*79px;[\s\S]*height:\s*79px;[\s\S]*contain-intrinsic-size:\s*79px;/); | |
| assert.match(styles, /@media\s*\(max-width:\s*540px\)\s*\{[\s\S]*\.session-item\s*\{[\s\S]*min-height:\s*79px;[\s\S]*height:\s*79px;[\s\S]*contain-intrinsic-size:\s*79px;/); |
🤖 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 `@tests/unit/compact-layout-ui.test.mjs` at line 67, The current assertion on
the styles string uses a greedy regex and the wrong breakpoint value, so it can
match a .session-item rule from a different media block; update the assert.match
that references styles to use the actual mobile breakpoint value used in the
stylesheet (replace 720px with the project breakpoint) and make the media-block
match non-greedy (use [\s\S]*? between the media query and the .session-item
rules) so the regex ensures the .session-item
min-height/height/contain-intrinsic-size are inside that exact `@media`
(max-width: Xpx) block.
| isSessionNativeAvailable(session) { | ||
| if (!session || typeof session !== 'object') return false; | ||
| if (session.nativeAvailable === false) return false; | ||
| return true; | ||
| }, | ||
|
|
||
| isImportToNativeAvailable(session) { | ||
| if (!this.isDerivedSession(session)) return false; | ||
| if (session.nativeAvailable === true) return false; | ||
| if (session.nativeImportAvailable === false) return false; | ||
| const source = String(session.source || '').trim().toLowerCase(); | ||
| return source === 'codex' || source === 'claude'; |
There was a problem hiding this comment.
Don't treat missing nativeAvailable as “native”.
For derived sessions, isSessionNativeAvailable() returns true when the field is missing, while isImportToNativeAvailable() treats the same missing field as importable. That produces contradictory UI state: resume shows no warning, but the session still offers “Import to Native”. Default derived sessions to non-native unless the backend explicitly sets nativeAvailable === true.
Suggested fix
isSessionNativeAvailable(session) {
if (!session || typeof session !== 'object') return false;
- if (session.nativeAvailable === false) return false;
- return true;
+ if (session.nativeAvailable === true) return true;
+ if (this.isDerivedSession(session)) return false;
+ return session.nativeAvailable !== false;
},🤖 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 `@web-ui/modules/app.methods.session-actions.mjs` around lines 31 - 42, The two
helpers disagree because isSessionNativeAvailable(session) currently treats a
missing session.nativeAvailable as true; change the logic so native availability
is only true when session.nativeAvailable === true (i.e., treat
missing/undefined as false). Update isSessionNativeAvailable to return
Boolean(session && typeof session === 'object' && session.nativeAvailable ===
true) and ensure isImportToNativeAvailable continues to require
this.isDerivedSession(session) and that session.nativeAvailable !== true and
session.nativeImportAvailable !== false; reference the functions
isSessionNativeAvailable, isImportToNativeAvailable, isDerivedSession and the
fields session.nativeAvailable and session.nativeImportAvailable when making the
change.
| if (data && typeof data === 'object' && !data.error) { | ||
| this.webhookConfig = { | ||
| enabled: !!data.enabled, | ||
| url: typeof data.url === 'string' ? data.url : '', | ||
| events: Array.isArray(data.events) && data.events.length | ||
| ? data.events.slice() | ||
| : this.webhookEventOptions.slice() | ||
| }; |
There was a problem hiding this comment.
Keep an empty webhook event selection empty on reload.
If the saved config comes back with events: [], this branch expands it to every event again. That makes “enabled but no subscribed events” impossible to round-trip, so a refresh silently re-enables deliveries the user turned off.
Suggested fix
this.webhookConfig = {
enabled: !!data.enabled,
url: typeof data.url === 'string' ? data.url : '',
- events: Array.isArray(data.events) && data.events.length
- ? data.events.slice()
- : this.webhookEventOptions.slice()
+ events: Array.isArray(data.events)
+ ? data.events.slice()
+ : this.webhookEventOptions.slice()
};🤖 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 `@web-ui/modules/app.methods.webhook-terminal.mjs` around lines 8 - 15, The
current assignment to this.webhookConfig.events treats an empty saved array as
falsy and replaces it with this.webhookEventOptions, preventing round-tripping
an intentionally empty selection; change the logic in the webhook config
initialization (where this.webhookConfig is set and webhookEventOptions is
referenced) so that if data.events is an actual array (even if length === 0) you
use data.events.slice(), and only fall back to this.webhookEventOptions.slice()
when data.events is not an array or is undefined.
| _appendTerminalLine(stream, text) { | ||
| const lines = String(text || '').split(/\r?\n/); | ||
| for (let i = 0; i < lines.length; i++) { | ||
| const t = lines[i]; | ||
| if (i === lines.length - 1 && t === '') continue; | ||
| this.terminalLines.push({ stream: stream || 'stdout', text: t }); | ||
| } | ||
| const max = Number.isFinite(this.terminalMaxLines) ? this.terminalMaxLines : 2000; | ||
| if (this.terminalLines.length > max) { | ||
| this.terminalLines.splice(0, this.terminalLines.length - max); | ||
| } |
There was a problem hiding this comment.
Preserve partial terminal chunks between WebSocket messages.
Process output is chunked arbitrarily, not on line boundaries. Splitting every message immediately will turn partial writes into separate “lines”, which corrupts streamed output and search/highlight behavior. Keep a per-stream carry buffer and flush the remainder on exit/close.
| const proto = (typeof window !== 'undefined' && window.location && window.location.protocol === 'https:') ? 'wss:' : 'ws:'; | ||
| const host = (typeof window !== 'undefined' && window.location && window.location.host) ? window.location.host : 'localhost'; | ||
| const url = proto + '//' + host + '/ws/terminal'; |
There was a problem hiding this comment.
Build the terminal socket URL from API_BASE.
The HTTP client already falls back to http://localhost:3737 when the UI runs from a file:/null origin, but this code rebuilds the socket URL from window.location.host and drops that port/custom host. In those modes the terminal panel will target the wrong endpoint and never connect.
Suggested fix
- const proto = (typeof window !== 'undefined' && window.location && window.location.protocol === 'https:') ? 'wss:' : 'ws:';
- const host = (typeof window !== 'undefined' && window.location && window.location.host) ? window.location.host : 'localhost';
- const url = proto + '//' + host + '/ws/terminal';
+ const base = new URL(API_BASE);
+ base.protocol = base.protocol === 'https:' ? 'wss:' : 'ws:';
+ base.pathname = '/ws/terminal';
+ base.search = '';
+ base.hash = '';
+ const url = base.toString();📝 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.
| const proto = (typeof window !== 'undefined' && window.location && window.location.protocol === 'https:') ? 'wss:' : 'ws:'; | |
| const host = (typeof window !== 'undefined' && window.location && window.location.host) ? window.location.host : 'localhost'; | |
| const url = proto + '//' + host + '/ws/terminal'; | |
| const base = new URL(API_BASE); | |
| base.protocol = base.protocol === 'https:' ? 'wss:' : 'ws:'; | |
| base.pathname = '/ws/terminal'; | |
| base.search = ''; | |
| base.hash = ''; | |
| const url = base.toString(); |
🤖 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 `@web-ui/modules/app.methods.webhook-terminal.mjs` around lines 126 - 128, The
socket URL construction using window.location drops custom host/port and breaks
file:// fallbacks; change the logic in
web-ui/modules/app.methods.webhook-terminal.mjs to build the WebSocket URL from
the existing API_BASE (or fall back to the previous window.location logic)
instead of using window.location.host directly: parse API_BASE (or new
URL(API_BASE)) to determine protocol (map http->ws, https->wss), host, and port,
then append '/ws/terminal' to produce url; update the variables proto, host, and
url usage so the terminal connects to the same backend endpoint as the HTTP
client.
Closes #126: outbound webhook delivery on provider switch and CLAUDE.md edits, with enable toggle, URL field, event filter, and test ping. Persisted at ~/.codex/codexmate-webhook.json. Disabled or filtered events skip delivery. Closes #127: collapsible terminal panel streaming stdout/stderr from a curated command list (codexmate, node, echo) over a hand-rolled WebSocket on /ws/terminal. Supports run, kill, pause, resume, clear, and inline search highlight. Tests: e2e covers webhook delivery, disable, event filter, and test ping; ws terminal covers stdout, stderr, exit code, and command rejection. Unit parity baselines extended for the new data and method keys.
ymkiux
force-pushed
the
feat/webhook-notify-and-live-terminal
branch
from
9988be7 to
9cb97a8
Compare
1 participant
Summary
provider-switchandclaude-md-editevents, with enable toggle, URL field, event filter, and test ping. Persisted at~/.codex/codexmate-webhook.json. Disabled or filtered events skip delivery.codexmate,node,echo) over a hand-rolled WebSocket on/ws/terminal. Supports run, kill, pause, resume, clear, and inline search highlight.Implementation notes
lib/cli-webhook.js(config load/save/normalize, fire-and-forget POST with 5s timeout)lib/cli-ws-server.js(RFC6455 handshake, frame encode/decode, masked client frames for tests)cli.js:get-webhook-config,set-webhook-config,test-webhookapply-claude-configandapply-claude-md-filefirenotifyWebhookon successserver.on('upgrade')for/ws/terminalwith loopback / token-gated remote access and command whitelistpanel-plugins.htmlapp.methods.webhook-terminal.mjsregistered through the existing methods indexstyles/terminal-webhook.cssTests
tests/unit/web-ui-behavior-parity.test.mjs).tests/e2e/test-webhook.js:test-webhookdelivers POST withevent=provider-switchanddetails.test=trueapply-claude-configtriggers aprovider-switchpayload mentioning the provider name and ISO timestamptests/e2e/test-terminal.js:startedframesnot allowedVerification
npm run test:unit-> 524/524 passing locallynpm run test:e2e-> existing pre-PR Windows timeout intestInstallStatusreproduces on cleanmain; added tests are wired into the suite for CI to exercise on LinuxCompatibility / rollback
Summary by CodeRabbit
New Features
UI Enhancements