Add Sentinel Agents system, sidebar redesign, memory leak fixes#207
Open
motherskitchenblr2 wants to merge 3 commits into
Open
Add Sentinel Agents system, sidebar redesign, memory leak fixes#207motherskitchenblr2 wants to merge 3 commits into
motherskitchenblr2 wants to merge 3 commits into
Conversation
- New Sentinel Agents backend (router, API client, UI panel) - Sidebar redesigned as persistent vertical icon bar (collapsed=60px) - Memory leak fixes: node-pty, globby, worker pool, router/SSE cleanup - New icons: ShieldScan, ShieldCheck, Database - Brand colors for supabase/sentinel tabs, dark mode sidebar CSS
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds three new routes and a Sentinel monitoring UI/API flow, while also tightening subprocess handling, sidebar collapse behavior, and deploy/PWA packaging metadata. ChangesSentinel Feature + New Routes
Deployment and PWA packaging
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
PR Summary by QodoAdd Sentinels monitoring panel, new sidebar icon rail, and backend leak safeguards
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
Code Review by Qodo
1. sidebar-scroll-position doc outdated
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
71
to
+83
| const layoutStyle = computed(() => { | ||
| if (isMobile.value || props.isSidebarCollapsed) { | ||
| if (isMobile.value) { | ||
| return { | ||
| '--sidebar-width': '0px', | ||
| '--layout-columns': 'minmax(0, 1fr)', | ||
| } | ||
| } | ||
| if (props.isSidebarCollapsed) { | ||
| return { | ||
| '--sidebar-width': `${COLLAPSED_SIDEBAR_WIDTH}px`, | ||
| '--layout-columns': 'var(--sidebar-width) 0px minmax(0, 1fr)', | ||
| } | ||
| } |
There was a problem hiding this comment.
1. sidebar-scroll-position doc outdated 📘 Rule violation ⚙ Maintainability
The sidebar collapse behavior was changed to a persistent collapsed sidebar (60px wide), but the existing manual test doc still describes “closing and reopening” the sidebar. This violates the requirement to update domain manual test documentation under tests/<domain>/ after feature work, risking incorrect/repeatability of manual verification.
Agent Prompt
## Issue description
The sidebar collapse UX changed (collapsed sidebar remains visible at a fixed width), but manual test documentation still reflects the old collapse/reopen behavior.
## Issue Context
This PR introduces a persistent collapsed sidebar layout and adds new sidebar/panel surfaces. Per compliance requirements, the relevant domain manual test docs under `tests/<domain>/` must be updated to keep verification steps accurate and discoverable.
## Fix Focus Areas
- tests/projects-sidebar-new-chat/sidebar-scroll-position-survives-collapse.md[1-27]
- tests/projects-sidebar-new-chat/index.md[7-26]
- tests/template.md[1-200]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1304
to
+1305
| const SocketSecurityPanel = defineAsyncComponent(() => import('./components/content/SocketSecurityPanel.vue')) | ||
| const SupabasePanel = defineAsyncComponent(() => import('./components/content/SupabasePanel.vue')) |
There was a problem hiding this comment.
2. Missing socket/supabase panels 🐞 Bug ≡ Correctness
App.vue references SocketSecurityPanel and SupabasePanel modules that do not exist in the branch checkout, causing client build-time module resolution failures. This blocks the UI from compiling when those async components are imported.
Agent Prompt
### Issue description
`src/App.vue` lazy-loads `./components/content/SocketSecurityPanel.vue` and `./components/content/SupabasePanel.vue`, but those files are not present in the branch. This will fail module resolution during the frontend build.
### Issue Context
The PR adds routes and sidebar entries for Socket Security and Supabase, and App.vue attempts to render panels for them.
### Fix Focus Areas
- Create the missing components (or remove the imports until implemented):
- src/App.vue[1299-1306]
- src/components/content/SocketSecurityPanel.vue[1-200]
- src/components/content/SupabasePanel.vue[1-200]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+15
to
+16
| import { handleSocketSecurityRoutes } from './socketSecurityRouter.js' | ||
| import { handleSupabaseRoutes } from './supabaseRouter.js' |
There was a problem hiding this comment.
3. Missing socket/supabase routers 🐞 Bug ≡ Correctness
codexAppServerBridge.ts imports and invokes handleSocketSecurityRoutes and handleSupabaseRoutes, but the corresponding router modules are not present in the branch checkout, causing server build/startup failures. Any attempt to start the backend will fail module resolution for these imports.
Agent Prompt
### Issue description
`src/server/codexAppServerBridge.ts` imports `./socketSecurityRouter.js` and `./supabaseRouter.js` and calls their handlers in the middleware chain, but those modules are absent in the branch. This will break TypeScript compilation and/or runtime module loading.
### Issue Context
The PR introduces new API surfaces for Socket Security and Supabase in the UI and wires backend routing, but only `sentinelRouter.ts` is added.
### Fix Focus Areas
- Implement the missing routers (or remove wiring until ready):
- src/server/codexAppServerBridge.ts[14-17]
- src/server/codexAppServerBridge.ts[7877-7883]
- src/server/socketSecurityRouter.ts[1-250]
- src/server/supabaseRouter.ts[1-250]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+4731
to
4734
| if (route.name === 'home' || route.name === 'skills' || route.name === 'automations' || route.name === 'socket-security') { | ||
| if (selectedThreadId.value !== '') { | ||
| await selectThread('') | ||
| } |
There was a problem hiding this comment.
4. Thread not cleared routes 🐞 Bug ≡ Correctness
syncThreadSelectionWithRoute() only clears the selected thread for home/skills/automations/socket-security, but not for the newly added supabase/sentinels non-thread routes. This leaves stale selected-thread state active while on those panels.
Agent Prompt
### Issue description
`syncThreadSelectionWithRoute()` treats some routes as non-thread views and clears `selectedThreadId`, but it omits the newly added `supabase` and `sentinels` routes. As a result, navigating to these routes can keep a previously selected thread active in state.
### Issue Context
The router defines `supabase` and `sentinels` as top-level routes (like `skills`/`automations`), and App.vue renders dedicated panels for them.
### Fix Focus Areas
- Extend the non-thread route checks in both initialization and route-sync:
- src/App.vue[4701-4708]
- src/App.vue[4720-4736]
- src/router/index.ts[30-44]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+7095
to
+7103
| const key = this.turnKey(next) | ||
| const retryCount = (this.queueRetryCountByTurnKey.get(key) ?? 0) + 1 | ||
| if (retryCount > BackendQueueProcessor.MAX_QUEUE_RETRIES) { | ||
| this.queueRetryCountByTurnKey.delete(key) | ||
| } else { | ||
| this.queueRetryCountByTurnKey.set(key, retryCount) | ||
| await this.restoreQueuedTurn(next) | ||
| this.scheduleThreadQueueDrain(threadId) | ||
| } |
There was a problem hiding this comment.
5. Queued turns dropped silently 🐞 Bug ☼ Reliability
BackendQueueProcessor removes a queued turn from persisted storage and, after exceeding MAX_QUEUE_RETRIES, stops restoring it without recording a terminal failure. This can permanently drop queued user work with no error surfaced.
Agent Prompt
### Issue description
`processThreadQueue()` pops the next queued turn from persisted state. If `startQueuedTurn()` fails repeatedly, once `retryCount > MAX_QUEUE_RETRIES` the code deletes the retry counter and does not call `restoreQueuedTurn(next)`, leaving the popped item permanently removed.
### Issue Context
`popNextQueuedTurn()` updates the persisted queue by removing the message. `restoreQueuedTurn()` is the only mechanism shown to put it back.
### Fix Focus Areas
- Ensure a turn is not lost when retries are exhausted (e.g., restore it, move to a dead-letter queue, mark failed, and log/notify):
- src/server/codexAppServerBridge.ts[7086-7103]
- src/server/codexAppServerBridge.ts[7132-7147]
- src/server/codexAppServerBridge.ts[7150-7160]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+374
to
+383
| async function readJsonBody(req: IncomingMessage): Promise<Record<string, unknown> | null> { | ||
| const rawBody = await new Promise<string>((resolve, reject) => { | ||
| let body = '' | ||
| req.setEncoding('utf8') | ||
| req.on('data', (chunk: string) => { body += chunk }) | ||
| req.on('end', () => resolve(body)) | ||
| req.on('error', reject) | ||
| }) | ||
| return asRecord(rawBody.length > 0 ? JSON.parse(rawBody) : {}) | ||
| } |
There was a problem hiding this comment.
6. Sentinel json parsing unsafe 🐞 Bug ⛨ Security
sentinelRouter readJsonBody() parses JSON without handling parse errors locally and accumulates request bodies without any size limit. Malformed JSON will return 500 instead of 400, and large bodies can cause memory pressure/DoS.
Agent Prompt
### Issue description
`readJsonBody()` concatenates the entire request body into a string and calls `JSON.parse` without a try/catch. This allows unbounded memory growth and causes malformed JSON to throw into the route-level catch, producing a 500.
### Issue Context
`handleSentinelRoutes()` expects `readJsonBody()` to return `null` to produce a 400 for invalid bodies, but parse errors bypass that path.
### Fix Focus Areas
- Add a max byte limit (reject with 413/400) and catch JSON.parse errors to return `null` (or a structured error):
- src/server/sentinelRouter.ts[254-355]
- src/server/sentinelRouter.ts[374-383]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
6495
to
+6498
| this.readBuffer += chunk | ||
| if (this.readBuffer.length > READ_BUFFER_MAX_BYTES) { | ||
| this.readBuffer = '' | ||
| } |
There was a problem hiding this comment.
7. Stdout buffer truncation hangs 🐞 Bug ☼ Reliability
AppServerProcess clears readBuffer when it exceeds READ_BUFFER_MAX_BYTES, which can drop a JSON-RPC response line and leave the corresponding pending RPC promise unresolved indefinitely. This can stall features relying on app-server RPCs without any recovery.
Agent Prompt
### Issue description
When `readBuffer` exceeds the configured limit, the code resets it to an empty string. If the buffer contained an in-flight JSON-RPC response (especially a single very large line without a newline), that response is dropped and any matching entry in `this.pending` will never be resolved/rejected.
### Issue Context
`handleLine()` is the only place where pending requests are resolved/rejected, and it requires newline-delimited complete JSON.
### Fix Focus Areas
- Preserve the tail after the last newline, and/or detect an oversized single line and force a controlled failure (reject all pending + restart process):
- src/server/codexAppServerBridge.ts[6493-6507]
- src/server/codexAppServerBridge.ts[6545-6564]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.vue (1)
4731-4735: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear thread selection for
supabaseandsentinelshere too.The new guards elsewhere in this file already treat those routes as non-thread views, but this branch only resets selection for
socket-security. Navigating to Supabase or Sentinels keeps the previous thread selected in shared state and can leave thread-specific state/polling alive behind the dashboard.Suggested fix
- if (route.name === 'home' || route.name === 'skills' || route.name === 'automations' || route.name === 'socket-security') { + if ( + route.name === 'home' + || route.name === 'skills' + || route.name === 'automations' + || route.name === 'socket-security' + || route.name === 'supabase' + || route.name === 'sentinels' + ) { if (selectedThreadId.value !== '') { await selectThread('') }🤖 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/App.vue` around lines 4731 - 4735, The non-thread-route reset branch in the App.vue route watcher only clears selection for home, skills, automations, and socket-security, so Supabase and Sentinels can keep a stale thread selected. Update the route-name guard in this same watcher to include the supabase and sentinels route names alongside socket-security, and keep using selectThread('') when selectedThreadId.value is not empty. Make sure this branch stays consistent with the other non-thread view checks in App.vue so shared thread state is cleared whenever those dashboards are entered.
🧹 Nitpick comments (1)
src/server/reviewGit.ts (1)
196-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffTimeout-wrapped spawn pattern is now duplicated four times.
The
settled+setTimeout(SIGKILL)+error/closeguard block is repeated here inrunCommandResultandrunCommandCaptureRaw, and again incodexAppServerBridge.ts(runComposioJsonLines 2790-2810,runCommandCaptureRawLines 4263-4298). Extracting a sharedspawnWithTimeout(command, args, options, timeoutMs)helper would remove the divergence risk (e.g., one copy forgettingtimer.unref()or asettledcheck). Deferable since all copies are currently correct.🤖 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/server/reviewGit.ts` around lines 196 - 218, The timeout-wrapped spawn logic is duplicated across multiple command runners, including runCommandResult and runCommandCaptureRaw in reviewGit.ts and the matching helpers in codexAppServerBridge.ts. Refactor the shared settled/timer/error/close handling into a single spawnWithTimeout(command, args, options, timeoutMs) helper, then update each caller to use it so the SIGKILL, timer.unref(), and settled guards stay consistent in one place.
🤖 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/components/content/SentinelsPanel.vue`:
- Around line 180-205: Prevent overlapping poll cycles in the SentinelsPanel
polling flow by ensuring refresh does not start a new /status + /alerts request
while a previous one is still running. Update the polling logic around refresh,
fetchStatus, and fetchAlerts so only one cycle can be in flight at a time, and
replace the raw setInterval(refresh, 5000) behavior with a guarded or
self-scheduling approach that waits for completion before scheduling the next
poll. Also apply the same protection to the related polling block noted in the
component so stale responses cannot race newer UI state.
- Around line 749-767: The `.sentinels-severity--*` styles in
`SentinelsPanel.vue` are being reused for both the LED and the badge, which
causes the later badge rules to override the LED colors. Split the severity
styling into separate selector groups for the LED and badge elements, and update
the relevant markup/classes so each uses its own scoped severity class names.
Keep the existing severity variants (`low`, `medium`, `high`, `critical`) but
ensure the LED keeps the solid dot color while the badge retains its own tint
and animation styling.
- Around line 208-215: The onToggleMasterMode handler in SentinelsPanel.vue is
incorrectly mutating each agent’s local mode after calling
sentinelSetMasterMode, which fabricates per-agent state and wipes real
overrides. Update onToggleMasterMode so it only updates masterMode.value after
the /sentinel/mode call succeeds, and remove the loop that assigns agent.mode =
nextMode; keep agent-specific mode changes confined to the dedicated agent mode
flow.
In `@src/server/codexAppServerBridge.ts`:
- Around line 7094-7103: The queued-turn retry path in codexAppServerBridge’s
queue drain logic silently drops a turn after exceeding
BackendQueueProcessor.MAX_QUEUE_RETRIES. Update the catch block around
popNextQueuedTurn/restoreQueuedTurn so that when retryCount exceeds the max and
queueRetryCountByTurnKey is cleared, you also emit a clear log or observability
event identifying the abandoned turn (using turnKey(next) / next / threadId
context) before giving up, while keeping the existing retry-and-reschedule
behavior for the non-terminal path.
In `@src/server/sentinelRouter.ts`:
- Around line 260-274: The live GET handlers in sentinelRouter for /status and
/alerts currently call setJson without any cache directive, so add explicit
no-store caching headers to both responses to prevent stale polling data. Update
the response path in the GET branches that use manager.getMasterMode(),
manager.getAllAgents(), manager.getMetrics(),
manager.getUnacknowledgedAlertCount(), and manager.getAlerts() so they set
Cache-Control: no-store (or equivalent) before sending JSON, and apply the same
fix to the other referenced route block as well.
- Around line 351-354: The POST handlers in sentinelRouter.ts are treating
malformed JSON and oversized request bodies as 500s because readJsonBody() and
the outer catch() do not distinguish client input errors from server faults.
Update the request parsing path used by the affected endpoints (including the
handlers around the identified blocks) to enforce an explicit maximum body size
before buffering, and have JSON.parse/readJsonBody failures return 400 for
invalid JSON or 413 for payloads that exceed the limit. Keep the fix localized
to the parsing helpers and the endpoint catch blocks so the existing router
methods continue to use the same validation flow.
---
Outside diff comments:
In `@src/App.vue`:
- Around line 4731-4735: The non-thread-route reset branch in the App.vue route
watcher only clears selection for home, skills, automations, and
socket-security, so Supabase and Sentinels can keep a stale thread selected.
Update the route-name guard in this same watcher to include the supabase and
sentinels route names alongside socket-security, and keep using selectThread('')
when selectedThreadId.value is not empty. Make sure this branch stays consistent
with the other non-thread view checks in App.vue so shared thread state is
cleared whenever those dashboards are entered.
---
Nitpick comments:
In `@src/server/reviewGit.ts`:
- Around line 196-218: The timeout-wrapped spawn logic is duplicated across
multiple command runners, including runCommandResult and runCommandCaptureRaw in
reviewGit.ts and the matching helpers in codexAppServerBridge.ts. Refactor the
shared settled/timer/error/close handling into a single
spawnWithTimeout(command, args, options, timeoutMs) helper, then update each
caller to use it so the SIGKILL, timer.unref(), and settled guards stay
consistent in one place.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 01a9fa8a-85de-4ae6-a98b-65e988917648
📒 Files selected for processing (15)
package.jsonsrc/App.vuesrc/api/sentinel.tssrc/components/content/SentinelsPanel.vuesrc/components/icons/IconTablerDatabase.vuesrc/components/icons/IconTablerShieldCheck.vuesrc/components/icons/IconTablerShieldScan.vuesrc/components/layout/DesktopLayout.vuesrc/composables/useDesktopState.tssrc/router/index.tssrc/server/codexAppServerBridge.tssrc/server/reviewGit.tssrc/server/sentinelRouter.tssrc/server/telegramThreadBridge.tstsconfig.json
Comment on lines
+180
to
+205
| let pollTimer: ReturnType<typeof setInterval> | null = null | ||
|
|
||
| async function fetchStatus(): Promise<void> { | ||
| try { | ||
| const status = await sentinelGetStatus() | ||
| masterMode.value = status.masterMode | ||
| agents.value = status.agents | ||
| metrics.value = status.metrics | ||
| agentsOnline.value = status.metrics.agentsOnline | ||
| agentsTotal.value = status.metrics.agentsTotal | ||
| unacknowledgedAlertCount.value = status.unacknowledgedAlerts | ||
| } catch { | ||
| // silently retry on next poll | ||
| } | ||
| } | ||
|
|
||
| async function fetchAlerts(): Promise<void> { | ||
| try { | ||
| alerts.value = await sentinelGetAlerts(100) | ||
| } catch { | ||
| // silently retry on next poll | ||
| } | ||
| } | ||
|
|
||
| async function refresh(): Promise<void> { | ||
| await Promise.all([fetchStatus(), fetchAlerts()]) |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Prevent overlapping poll cycles.
setInterval(refresh, 5000) schedules a new poll even when the previous /status + /alerts fetches are still in flight. On a slow backend that stacks duplicate requests and lets older responses race newer state back into the UI.
Suggested fix
-let pollTimer: ReturnType<typeof setInterval> | null = null
+let pollTimer: ReturnType<typeof setTimeout> | null = null
+let isRefreshing = false
async function refresh(): Promise<void> {
+ if (isRefreshing) return
+ isRefreshing = true
- await Promise.all([fetchStatus(), fetchAlerts()])
+ try {
+ await Promise.all([fetchStatus(), fetchAlerts()])
+ } finally {
+ isRefreshing = false
+ }
}
onMounted(() => {
- refresh()
- pollTimer = setInterval(refresh, 5000)
+ const poll = async () => {
+ await refresh()
+ pollTimer = setTimeout(() => {
+ void poll()
+ }, 5000)
+ }
+ void poll()
})
onUnmounted(() => {
if (pollTimer) {
- clearInterval(pollTimer)
+ clearTimeout(pollTimer)
pollTimer = null
}
})Also applies to: 287-295
🤖 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/components/content/SentinelsPanel.vue` around lines 180 - 205, Prevent
overlapping poll cycles in the SentinelsPanel polling flow by ensuring refresh
does not start a new /status + /alerts request while a previous one is still
running. Update the polling logic around refresh, fetchStatus, and fetchAlerts
so only one cycle can be in flight at a time, and replace the raw
setInterval(refresh, 5000) behavior with a guarded or self-scheduling approach
that waits for completion before scheduling the next poll. Also apply the same
protection to the related polling block noted in the component so stale
responses cannot race newer UI state.
Comment on lines
+208
to
+215
| async function onToggleMasterMode(): Promise<void> { | ||
| const nextMode = masterMode.value === 'auto' ? 'manual' : 'auto' | ||
| try { | ||
| await sentinelSetMasterMode(nextMode) | ||
| masterMode.value = nextMode | ||
| for (const agent of agents.value) { | ||
| agent.mode = nextMode | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't overwrite per-agent modes after changing the master mode.
The /codex-api/sentinel/mode endpoint only changes masterMode; agent mode changes have their own /agent/mode endpoint. Writing agent.mode = nextMode here fabricates client state and temporarily erases real per-agent overrides until the next poll.
Suggested fix
async function onToggleMasterMode(): Promise<void> {
const nextMode = masterMode.value === 'auto' ? 'manual' : 'auto'
try {
await sentinelSetMasterMode(nextMode)
masterMode.value = nextMode
- for (const agent of agents.value) {
- agent.mode = nextMode
- }
+ await fetchStatus()
} catch {
// revert on failure handled by next poll
}
}📝 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.
Suggested change
| async function onToggleMasterMode(): Promise<void> { | |
| const nextMode = masterMode.value === 'auto' ? 'manual' : 'auto' | |
| try { | |
| await sentinelSetMasterMode(nextMode) | |
| masterMode.value = nextMode | |
| for (const agent of agents.value) { | |
| agent.mode = nextMode | |
| } | |
| async function onToggleMasterMode(): Promise<void> { | |
| const nextMode = masterMode.value === 'auto' ? 'manual' : 'auto' | |
| try { | |
| await sentinelSetMasterMode(nextMode) | |
| masterMode.value = nextMode | |
| await fetchStatus() | |
| } catch { | |
| // revert on failure handled by next poll | |
| } | |
| } |
🤖 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/components/content/SentinelsPanel.vue` around lines 208 - 215, The
onToggleMasterMode handler in SentinelsPanel.vue is incorrectly mutating each
agent’s local mode after calling sentinelSetMasterMode, which fabricates
per-agent state and wipes real overrides. Update onToggleMasterMode so it only
updates masterMode.value after the /sentinel/mode call succeeds, and remove the
loop that assigns agent.mode = nextMode; keep agent-specific mode changes
confined to the dedicated agent mode flow.
Comment on lines
+749
to
+767
| .sentinels-severity--low { | ||
| background: #22c55e; | ||
| box-shadow: 0 0 4px #22c55e; | ||
| } | ||
|
|
||
| .sentinels-severity--medium { | ||
| background: #f59e0b; | ||
| box-shadow: 0 0 4px #f59e0b; | ||
| } | ||
|
|
||
| .sentinels-severity--high { | ||
| background: #f97316; | ||
| box-shadow: 0 0 6px #f97316; | ||
| } | ||
|
|
||
| .sentinels-severity--critical { | ||
| background: #ef4444; | ||
| box-shadow: 0 0 8px #ef4444; | ||
| animation: pulse-alert 1.5s infinite; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the severity classes to the LED and badge separately.
.sentinels-severity--* is declared twice. The later badge block overrides the earlier LED background rules, so the dot inherits the badge tint instead of the intended solid severity color.
Suggested fix
-.sentinels-severity--low {
+.sentinels-alert-led.sentinels-severity--low {
background: `#22c55e`;
box-shadow: 0 0 4px `#22c55e`;
}
-.sentinels-severity--medium {
+.sentinels-alert-led.sentinels-severity--medium {
background: `#f59e0b`;
box-shadow: 0 0 4px `#f59e0b`;
}
-.sentinels-severity--high {
+.sentinels-alert-led.sentinels-severity--high {
background: `#f97316`;
box-shadow: 0 0 6px `#f97316`;
}
-.sentinels-severity--critical {
+.sentinels-alert-led.sentinels-severity--critical {
background: `#ef4444`;
box-shadow: 0 0 8px `#ef4444`;
animation: pulse-alert 1.5s infinite;
}
-.sentinels-severity--low {
+.sentinels-alert-severity.sentinels-severity--low {
background: color-mix(in srgb, `#22c55e` 15%, transparent);
color: `#22c55e`;
}Also applies to: 796-814
🤖 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/components/content/SentinelsPanel.vue` around lines 749 - 767, The
`.sentinels-severity--*` styles in `SentinelsPanel.vue` are being reused for
both the LED and the badge, which causes the later badge rules to override the
LED colors. Split the severity styling into separate selector groups for the LED
and badge elements, and update the relevant markup/classes so each uses its own
scoped severity class names. Keep the existing severity variants (`low`,
`medium`, `high`, `critical`) but ensure the LED keeps the solid dot color while
the badge retains its own tint and animation styling.
Comment on lines
7094
to
+7103
| } catch { | ||
| await this.restoreQueuedTurn(next) | ||
| this.scheduleThreadQueueDrain(threadId) | ||
| const key = this.turnKey(next) | ||
| const retryCount = (this.queueRetryCountByTurnKey.get(key) ?? 0) + 1 | ||
| if (retryCount > BackendQueueProcessor.MAX_QUEUE_RETRIES) { | ||
| this.queueRetryCountByTurnKey.delete(key) | ||
| } else { | ||
| this.queueRetryCountByTurnKey.set(key, retryCount) | ||
| await this.restoreQueuedTurn(next) | ||
| this.scheduleThreadQueueDrain(threadId) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Queued turn is silently discarded after exhausting retries.
On the failure path, once retryCount > MAX_QUEUE_RETRIES the retry key is deleted but restoreQueuedTurn(next) is not called — the turn was already popped via popNextQueuedTurn, so the user's queued message is permanently dropped with no log or surfaced error. The retry cap itself is sound (avoids poison-pill loops), but the permanent loss is invisible to operators and users.
Consider logging when a turn is abandoned so the drop is observable.
🪵 Suggested observability for abandoned turns
} catch {
const key = this.turnKey(next)
const retryCount = (this.queueRetryCountByTurnKey.get(key) ?? 0) + 1
if (retryCount > BackendQueueProcessor.MAX_QUEUE_RETRIES) {
this.queueRetryCountByTurnKey.delete(key)
+ console.warn('[backend-queue] Dropping queued turn after max retries', {
+ threadId: next.threadId,
+ messageId: next.message.id,
+ retries: BackendQueueProcessor.MAX_QUEUE_RETRIES,
+ })
} else {Want me to wire this into a structured logger instead, or open an issue to track surfacing the failure to the client?
📝 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.
Suggested change
| } catch { | |
| await this.restoreQueuedTurn(next) | |
| this.scheduleThreadQueueDrain(threadId) | |
| const key = this.turnKey(next) | |
| const retryCount = (this.queueRetryCountByTurnKey.get(key) ?? 0) + 1 | |
| if (retryCount > BackendQueueProcessor.MAX_QUEUE_RETRIES) { | |
| this.queueRetryCountByTurnKey.delete(key) | |
| } else { | |
| this.queueRetryCountByTurnKey.set(key, retryCount) | |
| await this.restoreQueuedTurn(next) | |
| this.scheduleThreadQueueDrain(threadId) | |
| } | |
| } catch { | |
| const key = this.turnKey(next) | |
| const retryCount = (this.queueRetryCountByTurnKey.get(key) ?? 0) + 1 | |
| if (retryCount > BackendQueueProcessor.MAX_QUEUE_RETRIES) { | |
| this.queueRetryCountByTurnKey.delete(key) | |
| console.warn('[backend-queue] Dropping queued turn after max retries', { | |
| threadId: next.threadId, | |
| messageId: next.message.id, | |
| retries: BackendQueueProcessor.MAX_QUEUE_RETRIES, | |
| }) | |
| } else { | |
| this.queueRetryCountByTurnKey.set(key, retryCount) | |
| await this.restoreQueuedTurn(next) | |
| this.scheduleThreadQueueDrain(threadId) | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, spawnSync, type ChildProcessWithoutNullStreams } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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/server/codexAppServerBridge.ts` around lines 7094 - 7103, The queued-turn
retry path in codexAppServerBridge’s queue drain logic silently drops a turn
after exceeding BackendQueueProcessor.MAX_QUEUE_RETRIES. Update the catch block
around popNextQueuedTurn/restoreQueuedTurn so that when retryCount exceeds the
max and queueRetryCountByTurnKey is cleared, you also emit a clear log or
observability event identifying the abandoned turn (using turnKey(next) / next /
threadId context) before giving up, while keeping the existing
retry-and-reschedule behavior for the non-terminal path.
Comment on lines
+260
to
+274
| if (req.method === 'GET' && path === '/status') { | ||
| setJson(res, 200, { | ||
| masterMode: manager.getMasterMode(), | ||
| agents: manager.getAllAgents(), | ||
| metrics: manager.getMetrics(), | ||
| unacknowledgedAlerts: manager.getUnacknowledgedAlertCount(), | ||
| }) | ||
| return true | ||
| } | ||
|
|
||
| if (req.method === 'GET' && path === '/alerts') { | ||
| const limit = parseInt(url.searchParams.get('limit') || '50', 10) | ||
| const unacknowledgedOnly = url.searchParams.get('unacknowledged') === 'true' | ||
| setJson(res, 200, manager.getAlerts(limit, unacknowledgedOnly)) | ||
| return true |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mark live sentinel responses as non-cacheable.
/status and /alerts are live state endpoints, but setJson() does not send any cache directive. That leaves the polling dashboard exposed to stale snapshots from browser/intermediary caching. Add Cache-Control: no-store (or equivalent) on these responses. As per coding guidelines, **/*.{js,ts,tsx} routing/API changes should explicitly check cache invalidation risk.
Also applies to: 368-371
🤖 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/server/sentinelRouter.ts` around lines 260 - 274, The live GET handlers
in sentinelRouter for /status and /alerts currently call setJson without any
cache directive, so add explicit no-store caching headers to both responses to
prevent stale polling data. Update the response path in the GET branches that
use manager.getMasterMode(), manager.getAllAgents(), manager.getMetrics(),
manager.getUnacknowledgedAlertCount(), and manager.getAlerts() so they set
Cache-Control: no-store (or equivalent) before sending JSON, and apply the same
fix to the other referenced route block as well.
Source: Coding guidelines
Comment on lines
+351
to
+354
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error) | ||
| setJson(res, 500, { error: `Sentinel error: ${message}` }) | ||
| return true |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound request bodies and downgrade malformed JSON to client errors.
readJsonBody() buffers the full body with no size cap, and JSON.parse() failures fall through to the outer catch, which currently returns a 500. For these POST endpoints that creates an avoidable memory-pressure path and misclassifies bad input as a server fault. Enforce a max body size and map parse failures to 400/413 instead. As per coding guidelines, **/*.{js,ts,tsx} API changes should explicitly check large payloads.
Suggested fix
export async function handleSentinelRoutes(req: IncomingMessage, res: ServerResponse, url: URL): Promise<boolean> {
@@
} catch (error) {
const message = error instanceof Error ? error.message : String(error)
- setJson(res, 500, { error: `Sentinel error: ${message}` })
+ if (message === 'INVALID_JSON_BODY') {
+ setJson(res, 400, { error: 'Invalid JSON body' })
+ return true
+ }
+ if (message === 'REQUEST_BODY_TOO_LARGE') {
+ setJson(res, 413, { error: 'Request body too large' })
+ return true
+ }
+ setJson(res, 500, { error: `Sentinel error: ${message}` })
return true
}
}
@@
async function readJsonBody(req: IncomingMessage): Promise<Record<string, unknown> | null> {
+ const MAX_BODY_BYTES = 1_048_576
const rawBody = await new Promise<string>((resolve, reject) => {
let body = ''
+ let size = 0
req.setEncoding('utf8')
- req.on('data', (chunk: string) => { body += chunk })
+ req.on('data', (chunk: string) => {
+ size += Buffer.byteLength(chunk)
+ if (size > MAX_BODY_BYTES) {
+ reject(new Error('REQUEST_BODY_TOO_LARGE'))
+ req.destroy()
+ return
+ }
+ body += chunk
+ })
req.on('end', () => resolve(body))
req.on('error', reject)
})
- return asRecord(rawBody.length > 0 ? JSON.parse(rawBody) : {})
+ try {
+ return asRecord(rawBody.length > 0 ? JSON.parse(rawBody) : {})
+ } catch {
+ throw new Error('INVALID_JSON_BODY')
+ }
}Also applies to: 374-382
🤖 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/server/sentinelRouter.ts` around lines 351 - 354, The POST handlers in
sentinelRouter.ts are treating malformed JSON and oversized request bodies as
500s because readJsonBody() and the outer catch() do not distinguish client
input errors from server faults. Update the request parsing path used by the
affected endpoints (including the handlers around the identified blocks) to
enforce an explicit maximum body size before buffering, and have
JSON.parse/readJsonBody failures return 400 for invalid JSON or 413 for payloads
that exceed the limit. Keep the fix localized to the parsing helpers and the
endpoint catch blocks so the existing router methods continue to use the same
validation flow.
Source: Coding guidelines
- deploy.sh: one-click Linux deployment (auto-installs deps, builds, starts) - android-termux.sh: Android Termux deployment script - apk-builder.sh: APK builder (Bubblewrap TWA + AndroidJS methods) - Updated PWA manifest with Android-specific meta/display fields - Added mobile-web-app-capable and 512x512 icon link to index.html
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
public/manifest.webmanifest (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEmpty
screenshotsarray has no effect.Rich install UI (richer PWA install prompts) requires actual screenshot entries with
src/sizes/form_factor; an empty array is a no-op placeholder.🤖 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 `@public/manifest.webmanifest` at line 40, The manifest currently includes an empty screenshots array, which has no effect on rich PWA install UI. Update the manifest’s screenshots entry to include one or more real screenshot objects with the required src, sizes, and form_factor fields, or remove the screenshots property entirely if you do not have valid assets. Use the manifest structure around the screenshots field to locate and replace the placeholder.deploy.sh (1)
64-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
caseon$SHELL.Both
zshandbashbranches perform the identical export, and line 68 unconditionally re-exports the same value anyway, so thecaseadds no behavior.♻️ Proposed simplification
- case "$SHELL" in - */zsh) export PATH="$PNPM_HOME:$PATH" ;; - */bash) export PATH="$PNPM_HOME:$PATH" ;; - esac - PATH="$PNPM_HOME:$PATH" + export PATH="$PNPM_HOME:$PATH"🤖 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 `@deploy.sh` around lines 64 - 68, The PATH setup in the deploy script is redundant because the $SHELL case in the shell-init block performs the same export for both zsh and bash, and the following PATH assignment already applies unconditionally. Simplify the logic by removing the unnecessary case statement and keeping a single PATH update in the deploy script’s environment setup so the behavior stays the same but the duplicated branches are eliminated.android-termux.sh (1)
18-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
ARCH_SUFFIXis computed but never used.The architecture is detected and validated (good — unsupported arches exit early), but
ARCH_SUFFIXitself is never referenced afterward, so the case statement only serves as a validation gate. Consider dropping the unused variable or using it if arch-specific logic was intended.🤖 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 `@android-termux.sh` around lines 18 - 25, The ARCH_SUFFIX value in the architecture detection block is computed but never used, so either remove the unused ARCH_SUFFIX assignment from the uname/case logic or wire it into the later download/install flow in android-termux.sh if arch-specific behavior was intended. Keep the existing validation and error path in the architecture check, but ensure the script only retains ARCH_SUFFIX if a later step actually consumes it.
🤖 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 `@android-termux.sh`:
- Line 47: The default repository used by the android-termux.sh bootstrap script
is pointing at the wrong upstream, which can clone an unintended fork. Update
the REPO_URL default in the script so it matches the project’s real GitHub
repository as advertised by the CLI banner in src/cli/index.ts, and keep the
fallback aligned with the canonical source while preserving the ability to
override it via the first argument.
- Around line 34-37: The Termux bootstrap install step is treating pnpm as a
pkg-managed package, which will fail and stop the script under set -euo
pipefail. Update the install flow in android-termux.sh so nodejs-lts is
installed first, then install pnpm separately via npm install -g pnpm or
corepack enable, keeping the rest of the package installation intact.
In `@apk-builder.sh`:
- Around line 65-104: The build_androidjs() helper accepts app_name but never
uses it, so wire that parameter into the generated AndroidJS app configuration.
Update the package.json and/or index.js generation in build_androidjs() so
app_name is reflected in a meaningful place such as product metadata or the
BrowserWindow title, and ensure callers passing a custom name see it applied.
- Line 59: The APK copy step in apk-builder.sh can fail silently because the cp
command redirects errors away, yet the script still continues to report success.
Update the build flow around the cp invocation that creates codex-web-twa.apk so
it checks whether the expected source APK actually exists, handles alternate
bubblewrap output names if needed, and fails or emits an error when the copy
does not happen. Make sure the success message is only printed after the copy in
the APK build path has truly succeeded.
- Around line 30-62: `build_bubblewrap` accepts `app_name` and `package_name`
but never uses them, so custom values passed by callers are ignored. Update the
`npx `@bubblewrap/cli` init` invocation in `build_bubblewrap` to forward the app
name and package identifier from those parameters, and keep the existing
defaults intact when arguments are omitted. Make sure the `build_bubblewrap`
parameter names remain aligned with the CLI options so the generated TWA
reflects the caller-provided metadata.
In `@deploy.sh`:
- Around line 32-34: Piping the NodeSource setup script directly into sudo bash
in deploy.sh is unsafe because it executes remote content without verification.
Update the Node installation flow in the node check block to first download the
setup script, verify its integrity/signature before running it, and only then
execute it with elevated privileges. Keep the fix localized to the command
sequence around the node installation step so the existing apt install flow
remains unchanged.
- Around line 61-62: The pnpm installation step in the deploy script is piping a
remote curl response directly into sh without any verification. Replace this in
the install flow around the "Installing pnpm..." step with a safer approach that
downloads the installer first and validates it before execution, or use a
trusted package manager/source with integrity checking. Keep the change
localized to the pnpm install block in deploy.sh.
---
Nitpick comments:
In `@android-termux.sh`:
- Around line 18-25: The ARCH_SUFFIX value in the architecture detection block
is computed but never used, so either remove the unused ARCH_SUFFIX assignment
from the uname/case logic or wire it into the later download/install flow in
android-termux.sh if arch-specific behavior was intended. Keep the existing
validation and error path in the architecture check, but ensure the script only
retains ARCH_SUFFIX if a later step actually consumes it.
In `@deploy.sh`:
- Around line 64-68: The PATH setup in the deploy script is redundant because
the $SHELL case in the shell-init block performs the same export for both zsh
and bash, and the following PATH assignment already applies unconditionally.
Simplify the logic by removing the unnecessary case statement and keeping a
single PATH update in the deploy script’s environment setup so the behavior
stays the same but the duplicated branches are eliminated.
In `@public/manifest.webmanifest`:
- Line 40: The manifest currently includes an empty screenshots array, which has
no effect on rich PWA install UI. Update the manifest’s screenshots entry to
include one or more real screenshot objects with the required src, sizes, and
form_factor fields, or remove the screenshots property entirely if you do not
have valid assets. Use the manifest structure around the screenshots field to
locate and replace the placeholder.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9591208-8f72-4d4c-a790-2f4c601afdee
📒 Files selected for processing (5)
android-termux.shapk-builder.shdeploy.shindex.htmlpublic/manifest.webmanifest
✅ Files skipped from review due to trivial changes (1)
- index.html
- All 5 feature tabs (Skills, Automations, Security, Supabase, Sentinels) grouped under a collapsible 'Advanced' sidebar section header - Sidebar collapsed mode unchanged: all 5 icons still show - Added isAdvancedExpanded state (default expanded) - Added responsive CSS for SentinelsPanel (mobile-friendly layout) - Added sidebar-section-header CSS with dark mode support
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
All Correction Reviewed and Code Rabbit was successfully fixed the Issues.
Commented in CodeRabbit Change Stack
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Approved
Commented in CodeRabbit Change Stack
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Commented in CodeRabbit Change Stack
Comment on lines
+13
to
+54
| detect_pkg_manager() { | ||
| for pm in apt dnf yum pacman zypper apk; do | ||
| command -v "$pm" &>/dev/null && { echo "$pm"; return; } | ||
| done | ||
| echo "" | ||
| } | ||
|
|
||
| install_deps() { | ||
| local pm; pm=$(detect_pkg_manager) | ||
| if [ -z "$pm" ]; then | ||
| warn "No supported package manager found. Install Node.js 18+ and pnpm manually." | ||
| return | ||
| fi | ||
|
|
||
| info "Detected package manager: $pm" | ||
| case "$pm" in | ||
| apt) | ||
| sudo apt update -qq | ||
| sudo apt install -y -qq curl git openssl jq | ||
| if ! command -v node &>/dev/null; then | ||
| curl -fsSL https://deb.nodesource.com/setup_22.x | sudo bash - | ||
| sudo apt install -y -qq nodejs | ||
| fi | ||
| ;; | ||
| dnf) | ||
| sudo dnf install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| yum) | ||
| sudo yum install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| pacman) | ||
| sudo pacman -Sy --noconfirm curl git openssl jq nodejs npm | ||
| ;; | ||
| zypper) | ||
| sudo zypper --non-interactive install curl git openssl jq nodejs npm | ||
| ;; | ||
| apk) | ||
| apk add curl git openssl jq nodejs npm | ||
| ;; | ||
| esac | ||
| ok "System dependencies installed" | ||
| } |
Comment on lines
+13
to
+54
| detect_pkg_manager() { | ||
| for pm in apt dnf yum pacman zypper apk; do | ||
| command -v "$pm" &>/dev/null && { echo "$pm"; return; } | ||
| done | ||
| echo "" | ||
| } | ||
|
|
||
| install_deps() { | ||
| local pm; pm=$(detect_pkg_manager) | ||
| if [ -z "$pm" ]; then | ||
| warn "No supported package manager found. Install Node.js 18+ and pnpm manually." | ||
| return | ||
| fi | ||
|
|
||
| info "Detected package manager: $pm" | ||
| case "$pm" in | ||
| apt) | ||
| sudo apt update -qq | ||
| sudo apt install -y -qq curl git openssl jq | ||
| if ! command -v node &>/dev/null; then | ||
| curl -fsSL https://deb.nodesource.com/setup_22.x | sudo bash - | ||
| sudo apt install -y -qq nodejs | ||
| fi | ||
| ;; | ||
| dnf) | ||
| sudo dnf install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| yum) | ||
| sudo yum install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| pacman) | ||
| sudo pacman -Sy --noconfirm curl git openssl jq nodejs npm | ||
| ;; | ||
| zypper) | ||
| sudo zypper --non-interactive install curl git openssl jq nodejs npm | ||
| ;; | ||
| apk) | ||
| apk add curl git openssl jq nodejs npm | ||
| ;; | ||
| esac | ||
| ok "System dependencies installed" | ||
| } |
Comment on lines
+54
to
+65
| "pnpm": { | ||
| "onlyBuiltDependencies": [ | ||
| "@firebase/util", | ||
| "esbuild", | ||
| "node-pty", | ||
| "protobufjs" | ||
| ] | ||
| }, | ||
| "dependencies": { | ||
| "@composio/client": "0.1.0-alpha.66", | ||
| "@socketsecurity/sdk": "^4.0.1", | ||
| "@supabase/supabase-js": "^2.108.2", |
Comment on lines
+13
to
+54
| detect_pkg_manager() { | ||
| for pm in apt dnf yum pacman zypper apk; do | ||
| command -v "$pm" &>/dev/null && { echo "$pm"; return; } | ||
| done | ||
| echo "" | ||
| } | ||
|
|
||
| install_deps() { | ||
| local pm; pm=$(detect_pkg_manager) | ||
| if [ -z "$pm" ]; then | ||
| warn "No supported package manager found. Install Node.js 18+ and pnpm manually." | ||
| return | ||
| fi | ||
|
|
||
| info "Detected package manager: $pm" | ||
| case "$pm" in | ||
| apt) | ||
| sudo apt update -qq | ||
| sudo apt install -y -qq curl git openssl jq | ||
| if ! command -v node &>/dev/null; then | ||
| curl -fsSL https://deb.nodesource.com/setup_22.x | sudo bash - | ||
| sudo apt install -y -qq nodejs | ||
| fi | ||
| ;; | ||
| dnf) | ||
| sudo dnf install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| yum) | ||
| sudo yum install -y curl git openssl jq nodejs npm | ||
| ;; | ||
| pacman) | ||
| sudo pacman -Sy --noconfirm curl git openssl jq nodejs npm | ||
| ;; | ||
| zypper) | ||
| sudo zypper --non-interactive install curl git openssl jq nodejs npm | ||
| ;; | ||
| apk) | ||
| apk add curl git openssl jq nodejs npm | ||
| ;; | ||
| esac | ||
| ok "System dependencies installed" | ||
| } |
Comment on lines
+144
to
+179
| # ── Usage ── | ||
| echo "" | ||
| echo -e "${CYAN}╔══════════════════════════════════════════════════════════╗${NC}" | ||
| echo -e "${CYAN}║ Codex Web — APK Builder ║${NC}" | ||
| echo -e "${CYAN}╚══════════════════════════════════════════════════════════╝${NC}" | ||
| echo "" | ||
| echo "Methods available:" | ||
| echo " 1. bubblewrap — Google TWA (requires Java JDK 11+)" | ||
| echo " 2. androidjs — Node.js WebView wrapper" | ||
| echo " 3. auto — Try androidjs first, fallback to bubblewrap" | ||
| echo " 4. local — Build pointing at localhost:18923" | ||
| echo "" | ||
|
|
||
| METHOD="${1:-auto}" | ||
| case "$METHOD" in | ||
| bubblewrap|1) | ||
| build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" | ||
| ;; | ||
| androidjs|2) | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" | ||
| ;; | ||
| auto|3|"") | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" \ | ||
| || build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" \ | ||
| || { err "All methods failed"; exit 1; } | ||
| ;; | ||
| local|4) | ||
| build_from_local | ||
| ;; | ||
| *) | ||
| echo "Usage: $0 [method] [url] [app-name] [package]" | ||
| echo " method: bubblewrap | androidjs | auto (default) | local" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
Comment on lines
+144
to
+179
| # ── Usage ── | ||
| echo "" | ||
| echo -e "${CYAN}╔══════════════════════════════════════════════════════════╗${NC}" | ||
| echo -e "${CYAN}║ Codex Web — APK Builder ║${NC}" | ||
| echo -e "${CYAN}╚══════════════════════════════════════════════════════════╝${NC}" | ||
| echo "" | ||
| echo "Methods available:" | ||
| echo " 1. bubblewrap — Google TWA (requires Java JDK 11+)" | ||
| echo " 2. androidjs — Node.js WebView wrapper" | ||
| echo " 3. auto — Try androidjs first, fallback to bubblewrap" | ||
| echo " 4. local — Build pointing at localhost:18923" | ||
| echo "" | ||
|
|
||
| METHOD="${1:-auto}" | ||
| case "$METHOD" in | ||
| bubblewrap|1) | ||
| build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" | ||
| ;; | ||
| androidjs|2) | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" | ||
| ;; | ||
| auto|3|"") | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" \ | ||
| || build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" \ | ||
| || { err "All methods failed"; exit 1; } | ||
| ;; | ||
| local|4) | ||
| build_from_local | ||
| ;; | ||
| *) | ||
| echo "Usage: $0 [method] [url] [app-name] [package]" | ||
| echo " method: bubblewrap | androidjs | auto (default) | local" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
Comment on lines
+144
to
+179
| # ── Usage ── | ||
| echo "" | ||
| echo -e "${CYAN}╔══════════════════════════════════════════════════════════╗${NC}" | ||
| echo -e "${CYAN}║ Codex Web — APK Builder ║${NC}" | ||
| echo -e "${CYAN}╚══════════════════════════════════════════════════════════╝${NC}" | ||
| echo "" | ||
| echo "Methods available:" | ||
| echo " 1. bubblewrap — Google TWA (requires Java JDK 11+)" | ||
| echo " 2. androidjs — Node.js WebView wrapper" | ||
| echo " 3. auto — Try androidjs first, fallback to bubblewrap" | ||
| echo " 4. local — Build pointing at localhost:18923" | ||
| echo "" | ||
|
|
||
| METHOD="${1:-auto}" | ||
| case "$METHOD" in | ||
| bubblewrap|1) | ||
| build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" | ||
| ;; | ||
| androidjs|2) | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" | ||
| ;; | ||
| auto|3|"") | ||
| build_androidjs "${2:-http://localhost:18923}" "${3:-Codex Web}" \ | ||
| || build_bubblewrap "${2:-http://localhost:18923}" "${3:-Codex Web}" "${4:-com.codexweb.app}" \ | ||
| || { err "All methods failed"; exit 1; } | ||
| ;; | ||
| local|4) | ||
| build_from_local | ||
| ;; | ||
| *) | ||
| echo "Usage: $0 [method] [url] [app-name] [package]" | ||
| echo " method: bubblewrap | androidjs | auto (default) | local" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
Comment on lines
+75
to
+83
| build_app() { | ||
| info "Installing project dependencies..." | ||
| pnpm install | ||
| ok "Dependencies installed" | ||
|
|
||
| info "Building frontend + CLI..." | ||
| pnpm run build | ||
| ok "Build complete (frontend + CLI)" | ||
| } |
Comment on lines
+75
to
+83
| build_app() { | ||
| info "Installing project dependencies..." | ||
| pnpm install | ||
| ok "Dependencies installed" | ||
|
|
||
| info "Building frontend + CLI..." | ||
| pnpm run build | ||
| ok "Build complete (frontend + CLI)" | ||
| } |
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Submition can be done
Commented in CodeRabbit Change Stack
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Commented in CodeRabbit Change Stack
Author
There was a problem hiding this comment.
Ajor Security issues fixed
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Done.
Commented in CodeRabbit Change Stack
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Proceed to PR
Commented in CodeRabbit Change Stack
motherskitchenblr2
left a comment
motherskitchenblr2
left a comment
Author
There was a problem hiding this comment.
Commented in CodeRabbit Change Stack
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit