feat: Phase 2 — persistence, boundary dispatcher, enhanced renderer#2
Merged
Conversation
GoldfishDB: - Add schema v8 with helios_settings, helios_lanes, helios_audit collections - Wire persistence adapter with upsertLane, writeAuditEntry, loadSettings - Settings loaded on bootstrap, lanes persisted after lifecycle commands Bridge: - Route commands through boundary dispatcher (local_control, tool_interop, a2a) - tool_interop and agent_delegation return stub errors (Phase 3) - Audit trail written on every lifecycle command Renderer: - Session tab shows persisted lanes table (id, transport, session, updated) - Right rail shows audit trail from GoldfishDB - Persisted data loaded on startup and refresh - New RPC endpoints: heliosGetLanes, heliosGetAudit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive persistence layer for Helios, enabling lanes and audit trails to be stored in the database. It updates the RPC bridge to integrate persistence operations, extends the bootstrap process to load settings, adds database schema definitions for Helios collections, and expands the renderer UI to display persisted lanes and audit entries. Changes
Sequence DiagramsequenceDiagram
participant Renderer as Renderer<br/>(Helios UI)
participant Main as Main Process<br/>(index.ts)
participant Bridge as RPC Bridge<br/>(bus-rpc-bridge.ts)
participant Persistence as Persistence<br/>(persistence.ts)
participant DB as Database<br/>(GoldfishDB)
Renderer->>Main: lane.create() via RPC
Main->>Bridge: Command received
Bridge->>Persistence: upsertLane()
Persistence->>DB: Store lane record
DB-->>Persistence: Confirm
Bridge->>Persistence: writeAuditEntry()
Persistence->>DB: Store audit entry
DB-->>Persistence: Confirm
Bridge-->>Main: Updated state
Main-->>Renderer: State broadcast
Renderer->>Main: heliosGetLanes() RPC
Main->>Persistence: getLanesForWorkspace()
Persistence->>DB: Query lanes
DB-->>Persistence: Lane records
Persistence-->>Main: Lanes array
Main-->>Renderer: Render lane table
Renderer->>Main: heliosGetAudit() RPC
Main->>Persistence: getRecentAudit(50)
Persistence->>DB: Query audit
DB-->>Persistence: Audit entries
Persistence-->>Main: Audit array
Main-->>Renderer: Render audit trail
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/helios/bridge/bus-rpc-bridge.ts`:
- Around line 102-109: The audit write using writeAuditEntry currently runs
unguarded and can cause handleRequest to reject if the DB write fails; wrap the
call to writeAuditEntry (the block building action: method, workspaceId, laneId,
sessionId from result?.session_id, and detail using
decision.boundary/decision.adapter and response.status) in a try/catch so any
thrown error is caught and swallowed or logged (via processLogger/errorLogger)
without propagating, ensuring the lifecycle command returns the original
successful result even if the audit write fails.
In `@src/helios/bridge/persistence.ts`:
- Around line 68-70: The current lookup only matches by laneId and can overwrite
lanes across workspaces; change the existence check to scope by workspaceId as
well. Update the query/find used around db.collection("helios_lanes").query()
and the existing assignment (existing = data.find(...)) to require both d.laneId
=== lane.laneId && d.workspaceId === lane.workspaceId (or add an equivalent
filter in the collection query) so upserts only affect records in the same
workspace; ensure any subsequent insert/update also sets/uses lane.workspaceId
consistently.
- Around line 129-140: getRecentAudit currently queries all rows from
db.collection("helios_audit") and returns a global top-N, causing
cross-workspace audit leakage; change getRecentAudit to accept a workspaceId
parameter (or require one) and filter the query results by workspaceId before
sorting and slicing, e.g. filter the array from
db.collection("helios_audit").query() for entries where workspaceId matches the
provided value, then sort by timestamp and slice to limit; update any callers of
getRecentAudit to pass the appropriate workspaceId so callers retrieve only that
workspace's audits.
In `@src/renderers/helios/index.ts`:
- Around line 191-200: Silent catches in loadPersistedData are hiding failures;
replace the empty catch blocks with error logging that includes context and the
caught error so developers can diagnose issues: when awaiting
electrobun.rpc?.request.heliosGetLanes() and
electrobun.rpc?.request.heliosGetAudit(), catch errors and log descriptive
messages referencing the operation (e.g., "failed to load lanes" and "failed to
load audit") along with the error object before continuing, and keep the
existing behavior of only assigning to persistedLanes and auditEntries when the
responses are arrays.
- Around line 319-324: The loop over auditEntries uses unguarded
entry.timestamp.slice and entry.detail.slice which will throw if those
properties are missing; update the loop handling in the block that iterates
auditEntries (the for (const entry of auditEntries.slice(0, 10)) loop) to
defensively access these fields—e.g., coerce to a safe string before slicing or
check typeof before calling slice (use something like const ts =
(entry.timestamp || '').slice(11,19) and const detail = (entry.detail ||
'').slice(0,40) or equivalent guarded checks) and then use ts and detail when
building the elements so missing values won’t cause exceptions.
- Around line 286-294: The loop over persistedLanes calls lane.laneId.slice(...)
and lane.lastUpdated.slice(...), which can throw if those fields are
null/undefined; update the rendering in src/renderers/helios/index.ts to
defensively access these properties (e.g., coerce to a safe string or use
nullish checks) before slicing and provide a safe fallback display (like "—" or
an empty string) so malformed RPC data won't cause a TypeError; apply this to
the code that builds row elements inside the persistedLanes iteration
(references: persistedLanes, lane.laneId, lane.lastUpdated, row.appendChild
calls).
- Around line 173-180: The PersistedLane type in this renderer is missing fields
returned by the backend; update the renderer's PersistedLane type (or import the
backend definition) so it includes the id and workspaceId fields that
heliosGetLanes returns (the backend type is defined in
src/helios/bridge/persistence.ts). Modify the PersistedLane declaration used in
this file (or replace it with an import of the backend PersistedLane type) to
include id: string and workspaceId: string (and ensure sessionId/terminalId
types still match).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/helios/bridge/bus-rpc-bridge.tssrc/helios/bridge/helios-main-bootstrap.tssrc/helios/bridge/persistence.tssrc/main/goldfishdb/db.tssrc/main/goldfishdb/schema/add_helios_collections_8.tssrc/main/index.tssrc/renderers/helios/index.htmlsrc/renderers/helios/index.tssrc/renderers/ivde/rpc.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (6)
src/renderers/helios/index.ts (2)
src/helios/bridge/persistence.ts (1)
PersistedLane(56-65)src/renderers/ivde/init.ts (1)
electrobun(662-662)
src/main/index.ts (1)
src/helios/bridge/persistence.ts (2)
getLanesForWorkspace(94-108)getRecentAudit(129-149)
src/helios/bridge/bus-rpc-bridge.ts (4)
src/helios/runtime/protocol/bus.ts (1)
InMemoryLocalBus(51-244)src/helios/runtime/protocol/boundary_adapter.ts (2)
createBoundaryDispatcher(90-138)getBoundaryDispatchDecision(77-88)src/helios/bridge/persistence.ts (2)
upsertLane(67-92)writeAuditEntry(112-127)src/main/workspaceWindows.ts (1)
broadcastToAllWindowsInWorkspace(32-44)
src/main/goldfishdb/db.ts (1)
src/main/goldfishdb/schema/add_helios_collections_8.ts (1)
schema8(15-140)
src/helios/bridge/persistence.ts (1)
src/helios/desktop/settings.ts (1)
DEFAULT_SETTINGS(12-15)
src/helios/bridge/helios-main-bootstrap.ts (2)
src/helios/runtime/protocol/bus.ts (1)
InMemoryLocalBus(51-244)src/helios/bridge/persistence.ts (2)
HeliosSettings(12-15)loadSettings(24-40)
🔇 Additional comments (2)
src/renderers/helios/index.ts (2)
166-169: LGTM!Extending
doRefreshStateto also reload persisted data is appropriate for ensuring the UI reflects the latest state from the database.
360-364: LGTM!The double-render pattern is appropriate here: it shows the UI shell immediately while persisted data loads asynchronously, then updates with the fetched data.
Comment on lines
+102
to
+109
| // Audit trail | ||
| writeAuditEntry({ | ||
| action: method, | ||
| workspaceId, | ||
| laneId, | ||
| sessionId: (result?.session_id as string) ?? null, | ||
| detail: `${method} via ${decision.boundary}/${decision.adapter} — ${response.status}`, | ||
| }); |
There was a problem hiding this comment.
Don’t let audit-write failures fail successful lifecycle commands.
Line 103 writes audit outside a guard. A DB write error can reject handleRequest even when the lifecycle command itself succeeded.
🔧 Proposed fix
- // Audit trail
- writeAuditEntry({
- action: method,
- workspaceId,
- laneId,
- sessionId: (result?.session_id as string) ?? null,
- detail: `${method} via ${decision.boundary}/${decision.adapter} — ${response.status}`,
- });
+ // Audit trail (best-effort; must not fail command path)
+ try {
+ writeAuditEntry({
+ action: method,
+ workspaceId,
+ laneId,
+ sessionId: (result?.session_id as string) ?? null,
+ detail: `${method} via ${decision.boundary}/${decision.adapter} — ${response.status}`,
+ });
+ } catch {
+ // Don't fail the command if audit persistence fails
+ }📝 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
| // Audit trail | |
| writeAuditEntry({ | |
| action: method, | |
| workspaceId, | |
| laneId, | |
| sessionId: (result?.session_id as string) ?? null, | |
| detail: `${method} via ${decision.boundary}/${decision.adapter} — ${response.status}`, | |
| }); | |
| // Audit trail (best-effort; must not fail command path) | |
| try { | |
| writeAuditEntry({ | |
| action: method, | |
| workspaceId, | |
| laneId, | |
| sessionId: (result?.session_id as string) ?? null, | |
| detail: `${method} via ${decision.boundary}/${decision.adapter} — ${response.status}`, | |
| }); | |
| } catch { | |
| // Don't fail the command if audit persistence fails | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/helios/bridge/bus-rpc-bridge.ts` around lines 102 - 109, The audit write
using writeAuditEntry currently runs unguarded and can cause handleRequest to
reject if the DB write fails; wrap the call to writeAuditEntry (the block
building action: method, workspaceId, laneId, sessionId from result?.session_id,
and detail using decision.boundary/decision.adapter and response.status) in a
try/catch so any thrown error is caught and swallowed or logged (via
processLogger/errorLogger) without propagating, ensuring the lifecycle command
returns the original successful result even if the audit write fails.
Comment on lines
+68
to
+70
| const { data } = db.collection("helios_lanes").query(); | ||
| const existing = data.find((d) => d.laneId === lane.laneId); | ||
|
|
There was a problem hiding this comment.
Scope lane upsert by workspaceId to prevent cross-workspace overwrite.
Line 69 matches existing rows by laneId only. If two workspaces reuse the same lane id, one workspace will overwrite the other.
🔧 Proposed fix
export function upsertLane(lane: Omit<PersistedLane, "id">): PersistedLane {
- const { data } = db.collection("helios_lanes").query();
- const existing = data.find((d) => d.laneId === lane.laneId);
+ const { data } = db.collection("helios_lanes").query({
+ where: (d) => d.workspaceId === lane.workspaceId && d.laneId === lane.laneId,
+ });
+ const existing = data[0];📝 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
| const { data } = db.collection("helios_lanes").query(); | |
| const existing = data.find((d) => d.laneId === lane.laneId); | |
| const { data } = db.collection("helios_lanes").query({ | |
| where: (d) => d.workspaceId === lane.workspaceId && d.laneId === lane.laneId, | |
| }); | |
| const existing = data[0]; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/helios/bridge/persistence.ts` around lines 68 - 70, The current lookup
only matches by laneId and can overwrite lanes across workspaces; change the
existence check to scope by workspaceId as well. Update the query/find used
around db.collection("helios_lanes").query() and the existing assignment
(existing = data.find(...)) to require both d.laneId === lane.laneId &&
d.workspaceId === lane.workspaceId (or add an equivalent filter in the
collection query) so upserts only affect records in the same workspace; ensure
any subsequent insert/update also sets/uses lane.workspaceId consistently.
Comment on lines
+129
to
+140
| export function getRecentAudit(limit = 50): Array<{ | ||
| timestamp: string; | ||
| action: string; | ||
| workspaceId: string; | ||
| laneId: string | null; | ||
| sessionId: string | null; | ||
| detail: string; | ||
| }; | ||
|
|
||
| /** Collection names for use with GoldfishDB */ | ||
| export const HELIOS_COLLECTIONS = { | ||
| settings: "helios_settings", | ||
| workspaces: "helios_workspaces", | ||
| lanes: "helios_lanes", | ||
| audit: "helios_audit", | ||
| } as const; | ||
| }> { | ||
| const { data } = db.collection("helios_audit").query(); | ||
| return data | ||
| .sort((a, b) => b.timestamp.localeCompare(a.timestamp)) | ||
| .slice(0, limit) |
There was a problem hiding this comment.
Filter audit reads by workspace to avoid cross-workspace audit leakage.
Lines 137-140 read/sort all audit rows and return a global top-50. That leaks other workspaces’ lifecycle activity.
🔧 Proposed fix
-export function getRecentAudit(limit = 50): Array<{
+export function getRecentAudit(workspaceId: string, limit = 50): Array<{
timestamp: string;
action: string;
workspaceId: string;
laneId: string | null;
sessionId: string | null;
detail: string;
}> {
- const { data } = db.collection("helios_audit").query();
- return data
+ const { data } = db.collection("helios_audit").query({
+ where: (d) => d.workspaceId === workspaceId,
+ });
+ return [...data]
.sort((a, b) => b.timestamp.localeCompare(a.timestamp))
.slice(0, limit)
.map((d) => ({
timestamp: d.timestamp,
action: d.action,
workspaceId: d.workspaceId,
laneId: d.laneId ?? null,
sessionId: d.sessionId ?? null,
detail: d.detail,
}));
}Also update caller:
- return getRecentAudit(50);
+ return getRecentAudit(workspaceId, 50);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/helios/bridge/persistence.ts` around lines 129 - 140, getRecentAudit
currently queries all rows from db.collection("helios_audit") and returns a
global top-N, causing cross-workspace audit leakage; change getRecentAudit to
accept a workspaceId parameter (or require one) and filter the query results by
workspaceId before sorting and slicing, e.g. filter the array from
db.collection("helios_audit").query() for entries where workspaceId matches the
provided value, then sort by timestamp and slice to limit; update any callers of
getRecentAudit to pass the appropriate workspaceId so callers retrieve only that
workspace's audits.
Comment on lines
+173
to
+180
| type PersistedLane = { | ||
| laneId: string; | ||
| state: string; | ||
| transport: string; | ||
| sessionId: string | null; | ||
| terminalId: string | null; | ||
| lastUpdated: string; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the renderer's PersistedLane type definition
cat -n src/renderers/helios/index.ts | sed -n '170,185p'Repository: KooshaPari/colab
Length of output: 472
🏁 Script executed:
# Check the backend PersistedLane type definition
cat -n src/helios/bridge/persistence.ts | sed -n '50,70p'Repository: KooshaPari/colab
Length of output: 687
🏁 Script executed:
# Search for heliosGetLanes RPC endpoint to understand data flow
rg -n 'heliosGetLanes' --type ts -B 2 -A 10Repository: KooshaPari/colab
Length of output: 2138
🏁 Script executed:
# Search for any type definitions or interfaces related to PersistedLane across codebase
rg -n 'type PersistedLane|interface PersistedLane' --type tsRepository: KooshaPari/colab
Length of output: 182
🏁 Script executed:
# Find getLanesForWorkspace implementation to see what it returns
rg -n 'function getLanesForWorkspace|const getLanesForWorkspace' --type ts -A 8Repository: KooshaPari/colab
Length of output: 716
🏁 Script executed:
# Also search for any return statements in getLanesForWorkspace
rg -n 'getLanesForWorkspace' --type ts -B 2 -A 12 | head -60Repository: KooshaPari/colab
Length of output: 2719
Renderer PersistedLane type is missing fields returned by the backend.
The heliosGetLanes RPC endpoint returns the full backend PersistedLane type from src/helios/bridge/persistence.ts (which includes id and workspaceId fields), but the renderer's type definition at lines 173-180 omits these fields. Update the type to match the actual data being received, or import the type from the backend definition to keep them in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderers/helios/index.ts` around lines 173 - 180, The PersistedLane type
in this renderer is missing fields returned by the backend; update the
renderer's PersistedLane type (or import the backend definition) so it includes
the id and workspaceId fields that heliosGetLanes returns (the backend type is
defined in src/helios/bridge/persistence.ts). Modify the PersistedLane
declaration used in this file (or replace it with an import of the backend
PersistedLane type) to include id: string and workspaceId: string (and ensure
sessionId/terminalId types still match).
Comment on lines
+191
to
+200
| async function loadPersistedData() { | ||
| try { | ||
| const lanes = await electrobun.rpc?.request.heliosGetLanes(); | ||
| if (Array.isArray(lanes)) persistedLanes = lanes; | ||
| } catch { /* ignore */ } | ||
| try { | ||
| const audit = await electrobun.rpc?.request.heliosGetAudit(); | ||
| if (Array.isArray(audit)) auditEntries = audit.slice(0, 20); | ||
| } catch { /* ignore */ } | ||
| } |
There was a problem hiding this comment.
Silent error swallowing hides failures.
Catching exceptions with empty blocks makes debugging difficult and provides no feedback when data fails to load. At minimum, log the error for developer visibility.
🛠️ Proposed fix
async function loadPersistedData() {
try {
const lanes = await electrobun.rpc?.request.heliosGetLanes();
if (Array.isArray(lanes)) persistedLanes = lanes;
- } catch { /* ignore */ }
+ } catch (e) {
+ console.warn("Failed to load persisted lanes:", e);
+ }
try {
const audit = await electrobun.rpc?.request.heliosGetAudit();
if (Array.isArray(audit)) auditEntries = audit.slice(0, 20);
- } catch { /* ignore */ }
+ } catch (e) {
+ console.warn("Failed to load audit entries:", e);
+ }
}📝 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 loadPersistedData() { | |
| try { | |
| const lanes = await electrobun.rpc?.request.heliosGetLanes(); | |
| if (Array.isArray(lanes)) persistedLanes = lanes; | |
| } catch { /* ignore */ } | |
| try { | |
| const audit = await electrobun.rpc?.request.heliosGetAudit(); | |
| if (Array.isArray(audit)) auditEntries = audit.slice(0, 20); | |
| } catch { /* ignore */ } | |
| } | |
| async function loadPersistedData() { | |
| try { | |
| const lanes = await electrobun.rpc?.request.heliosGetLanes(); | |
| if (Array.isArray(lanes)) persistedLanes = lanes; | |
| } catch (e) { | |
| console.warn("Failed to load persisted lanes:", e); | |
| } | |
| try { | |
| const audit = await electrobun.rpc?.request.heliosGetAudit(); | |
| if (Array.isArray(audit)) auditEntries = audit.slice(0, 20); | |
| } catch (e) { | |
| console.warn("Failed to load audit entries:", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderers/helios/index.ts` around lines 191 - 200, Silent catches in
loadPersistedData are hiding failures; replace the empty catch blocks with error
logging that includes context and the caught error so developers can diagnose
issues: when awaiting electrobun.rpc?.request.heliosGetLanes() and
electrobun.rpc?.request.heliosGetAudit(), catch errors and log descriptive
messages referencing the operation (e.g., "failed to load lanes" and "failed to
load audit") along with the error object before continuing, and keep the
existing behavior of only assigning to persistedLanes and auditEntries when the
responses are arrays.
Comment on lines
+286
to
+294
| for (const lane of persistedLanes) { | ||
| const row = el("div", "lane-row"); | ||
| row.appendChild(el("span", "lane-id", lane.laneId.slice(0, 12))); | ||
| row.appendChild(el("span", "lane-transport", lane.transport)); | ||
| row.appendChild(el("span", "lane-session", lane.sessionId ? lane.sessionId.slice(0, 12) : "—")); | ||
| row.appendChild(el("span", "lane-updated", lane.lastUpdated.slice(11, 19))); | ||
| table.appendChild(row); | ||
| } | ||
| center.appendChild(table); |
There was a problem hiding this comment.
Unguarded property access may throw on malformed data.
lane.laneId.slice() and lane.lastUpdated.slice() will throw TypeError if these properties are undefined or null. Since data comes from an external RPC source, add defensive checks.
🛠️ Proposed fix with defensive access
for (const lane of persistedLanes) {
const row = el("div", "lane-row");
- row.appendChild(el("span", "lane-id", lane.laneId.slice(0, 12)));
- row.appendChild(el("span", "lane-transport", lane.transport));
+ row.appendChild(el("span", "lane-id", lane.laneId?.slice(0, 12) ?? "—"));
+ row.appendChild(el("span", "lane-transport", lane.transport ?? "—"));
row.appendChild(el("span", "lane-session", lane.sessionId ? lane.sessionId.slice(0, 12) : "—"));
- row.appendChild(el("span", "lane-updated", lane.lastUpdated.slice(11, 19)));
+ row.appendChild(el("span", "lane-updated", lane.lastUpdated?.slice(11, 19) ?? "—"));
table.appendChild(row);
}📝 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
| for (const lane of persistedLanes) { | |
| const row = el("div", "lane-row"); | |
| row.appendChild(el("span", "lane-id", lane.laneId.slice(0, 12))); | |
| row.appendChild(el("span", "lane-transport", lane.transport)); | |
| row.appendChild(el("span", "lane-session", lane.sessionId ? lane.sessionId.slice(0, 12) : "—")); | |
| row.appendChild(el("span", "lane-updated", lane.lastUpdated.slice(11, 19))); | |
| table.appendChild(row); | |
| } | |
| center.appendChild(table); | |
| for (const lane of persistedLanes) { | |
| const row = el("div", "lane-row"); | |
| row.appendChild(el("span", "lane-id", lane.laneId?.slice(0, 12) ?? "—")); | |
| row.appendChild(el("span", "lane-transport", lane.transport ?? "—")); | |
| row.appendChild(el("span", "lane-session", lane.sessionId ? lane.sessionId.slice(0, 12) : "—")); | |
| row.appendChild(el("span", "lane-updated", lane.lastUpdated?.slice(11, 19) ?? "—")); | |
| table.appendChild(row); | |
| } | |
| center.appendChild(table); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderers/helios/index.ts` around lines 286 - 294, The loop over
persistedLanes calls lane.laneId.slice(...) and lane.lastUpdated.slice(...),
which can throw if those fields are null/undefined; update the rendering in
src/renderers/helios/index.ts to defensively access these properties (e.g.,
coerce to a safe string or use nullish checks) before slicing and provide a safe
fallback display (like "—" or an empty string) so malformed RPC data won't cause
a TypeError; apply this to the code that builds row elements inside the
persistedLanes iteration (references: persistedLanes, lane.laneId,
lane.lastUpdated, row.appendChild calls).
Comment on lines
+319
to
+324
| for (const entry of auditEntries.slice(0, 10)) { | ||
| const row = el("div", "log-entry"); | ||
| row.appendChild(el("span", "log-ts", entry.timestamp.slice(11, 19))); | ||
| row.appendChild(el("span", "log-label", `${entry.action}: ${entry.detail.slice(0, 40)}`)); | ||
| auditContainer.appendChild(row); | ||
| } |
There was a problem hiding this comment.
Same unguarded access pattern for audit entries.
entry.timestamp.slice() and entry.detail.slice() will throw if the values are missing. Apply the same defensive pattern as for persisted lanes.
🛠️ Proposed fix
for (const entry of auditEntries.slice(0, 10)) {
const row = el("div", "log-entry");
- row.appendChild(el("span", "log-ts", entry.timestamp.slice(11, 19)));
- row.appendChild(el("span", "log-label", `${entry.action}: ${entry.detail.slice(0, 40)}`));
+ row.appendChild(el("span", "log-ts", entry.timestamp?.slice(11, 19) ?? "—"));
+ row.appendChild(el("span", "log-label", `${entry.action ?? "?"}: ${entry.detail?.slice(0, 40) ?? ""}`));
auditContainer.appendChild(row);
}📝 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
| for (const entry of auditEntries.slice(0, 10)) { | |
| const row = el("div", "log-entry"); | |
| row.appendChild(el("span", "log-ts", entry.timestamp.slice(11, 19))); | |
| row.appendChild(el("span", "log-label", `${entry.action}: ${entry.detail.slice(0, 40)}`)); | |
| auditContainer.appendChild(row); | |
| } | |
| for (const entry of auditEntries.slice(0, 10)) { | |
| const row = el("div", "log-entry"); | |
| row.appendChild(el("span", "log-ts", entry.timestamp?.slice(11, 19) ?? "—")); | |
| row.appendChild(el("span", "log-label", `${entry.action ?? "?"}: ${entry.detail?.slice(0, 40) ?? ""}`)); | |
| auditContainer.appendChild(row); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderers/helios/index.ts` around lines 319 - 324, The loop over
auditEntries uses unguarded entry.timestamp.slice and entry.detail.slice which
will throw if those properties are missing; update the loop handling in the
block that iterates auditEntries (the for (const entry of auditEntries.slice(0,
10)) loop) to defensively access these fields—e.g., coerce to a safe string
before slicing or check typeof before calling slice (use something like const ts
= (entry.timestamp || '').slice(11,19) and const detail = (entry.detail ||
'').slice(0,40) or equivalent guarded checks) and then use ts and detail when
building the elements so missing values won’t cause exceptions.
This was referenced Feb 28, 2026
KooshaPari
added a commit
that referenced
this pull request
Mar 25, 2026
feat(config): polyglot-config-core contract scaffold
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
helios_settings,helios_lanes,helios_auditcollectionsheliosGetLanes,heliosGetAuditTest plan
HELIOS_SURFACE_EDITOR=false bun run build:devboots with helios rendererHELIOS_SURFACE_EDITOR=true) unaffected🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes