Beast immobile agent session: Hangar agent path, plans, topology - #152
Closed
Coldaine wants to merge 13 commits into
Closed
Beast immobile agent session: Hangar agent path, plans, topology#152Coldaine wants to merge 13 commits into
Coldaine wants to merge 13 commits into
Conversation
Three fixes to the /cockpit transport layer landed in #142. **Unstable useSyncExternalStore arguments.** Every hook passed inline closures for `subscribe`, `getSnapshot`, and `getServerSnapshot`. React re-subscribes whenever the `subscribe` identity changes, so each of the eight hooks tore down and rebuilt its listener registration on every render — at 10 Hz scan plus ~16 FPS imagery that is a lot of churn on the exact path the design says must stay cheap. The arguments are now hoisted to module scope so their identity is stable for the process lifetime. **Stale telemetry survived a disconnect.** `disconnect()` reset only `connectionState`. Every other slice kept its last value, so a dropped socket left the last-known voltage, odometry, IMU, clearance, and scan rendered as if live — the page was honest in the connection banner and lying in every readout below it. All slices now reset to their server snapshot and notify on disconnect, so a drop reads as absence. It also clears `onopen` alongside the other handlers, which was leaking a callback into a socket we had abandoned. **Fabricated state of charge.** `/ugv/voltage` handling synthesized a `percentage` field as `(v / 12.6) * 100`. That is not state of charge — the 3S pack's discharge curve is nowhere near linear, and 0 V is not 0%. No component ever rendered it: SafetyStrip derives its own bar from real volts over the 8.8-12.6 V window, and TelemetryRow labels the readout "real volts only". It was an invented number waiting to be surfaced. Removed, and the tests now assert the field is absent. Also: `Number.isFinite` in place of `isNaN`, so an Infinity range or odom value is rejected rather than passed through; `String.slice` for the service call id in place of the deprecated `substr`; and `transition-transform hover:` in place of `group group-hover:` on the RGB tile, where `group-hover:` on the group element itself never fired. The scan crop stays at 45-134.5 degrees. That is the published blind sector, bins 60-179 of the mirrored scan; 225-315 is the pre-mirror raw value and does not describe what /scan actually carries. npm run test:run 387/387 - lint 0 errors - typecheck clean - build clean Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rendering /cockpit against an unreachable robot surfaced two captions that outlived the synthetic state-of-charge removed in the previous commit. `SafetyStrip` printed a flat "Ok" under the pack bus whenever `isLowVoltage` was false. That predicate is `voltage > 0 && voltage < 10.5`, so a disconnected socket — where `voltage` is 0 — took the false branch and the strip reassured the operator about a pack it had no reading from, directly under a gauge already showing "— V". Now reads "NO READING" when there is no measurement. Both captions also advertised the deleted formula: "SOC%: FAKE (V/12.6)" in the strip and "SOC% FAKE — HIDDEN" on the honesty rail. Nothing computes V/12.6 any more, and the remaining 12.6 is only the top of the voltage bar's axis. They now say what is true — state of charge is not derived from volts at all. Verified in the browser against an unreachable robot: the strip reads "— V / NO READING", every other slice renders as absence, and the console is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…surfaces - TwinCanvas: SVG Legend (Junctions & Ownership badge), wire junctions (⊕) for multi-terminal nets - Port: Pi (π) vs Orin (Σ) host ownership badge indicators - UnitCard & Hangar Hub: Compute badges (Workstation / Edge AI), staggered grid animations, hover/tap micro-interactions - Cockpit: Interactive motion press feedback, active camera target reticles, pulsing estop warning outline - Quartermaster & Tech Tree: Upgrade budget progress bars and capability dependency visual indicators - Missions & Items: Radial SVG objective completion rings and smooth AnimatePresence filtering
…tection, simplify e-stop
…pology, and always-commit-and-PR rule. Land the /agent UI and ROS bridge tools, consolidate immobile execution plans, and encode the commit-and-PR workflow so session work ships as PRs instead of dirty trees. Co-authored-by: Cursor <cursoragent@cursor.com>
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:XXL
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded a BEAST agent command path with streamed chat, approval-gated motion tools, ROS bridge actions, and safety-aware status handling. Updated cockpit controls and E-STOP behavior. Added BEAST operating plans, topology and flash documentation, UI enhancements, and archived-plan references. ChangesBEAST agent command path
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant AgentClient
participant AgentChatRoute
participant BeastRosClient
participant Nav2
Operator->>AgentClient: enter command
AgentClient->>AgentChatRoute: submit messages
AgentChatRoute->>BeastRosClient: create model and tools
AgentChatRoute-->>AgentClient: stream planner response
AgentClient->>AgentChatRoute: approve motion tool
AgentChatRoute->>BeastRosClient: dispatch bounded action
BeastRosClient->>Nav2: send navigation goal
Nav2-->>BeastRosClient: feedback or result
BeastRosClient-->>AgentChatRoute: return tool result
AgentChatRoute-->>AgentClient: stream execution state
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+39
to
+51
| <div className="border border-dim/50 rounded-lg p-4 mb-4"> | ||
| <h2 className="font-mono text-[11px] font-bold uppercase tracking-widest text-cyan mb-2">Bay Command Panel</h2> | ||
| <div className="flex flex-wrap gap-6 font-mono text-xs text-ink-dim"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="uppercase tracking-wider">Cooling Capacity:</span> | ||
| <span className="text-emerald-400">85%</span> | ||
| </div> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="uppercase tracking-wider">Power Headroom:</span> | ||
| <span className="text-amber-400">1.2kW</span> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Suggestion: The command panel displays hard-coded 85% cooling capacity and 1.2kW power headroom for every selected bay, although these values are not derived from b or its units and are not fields in the bay data contract. Navigating between bays therefore presents fabricated bay-specific operational telemetry; derive the values from actual data or render them as unavailable. [logic error]
Severity Level: Major ⚠️
- ⚠️ Every bay page displays fabricated cooling telemetry.
- ⚠️ Every bay page displays fabricated power headroom.
- ⚠️ Operators may mistake static placeholders for bay-specific status.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/bay/[id]/page.tsx
**Line:** 39:51
**Comment:**
*Logic Error: The command panel displays hard-coded `85%` cooling capacity and `1.2kW` power headroom for every selected bay, although these values are not derived from `b` or its units and are not fields in the bay data contract. Navigating between bays therefore presents fabricated bay-specific operational telemetry; derive the values from actual data or render them as unavailable.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+86
to
105
| {status.isCharging === true ? ( | ||
| <span className="font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5 text-amber-400 text-glow-amber"> | ||
| <ShieldAlert className="h-4 w-4 animate-pulse" /> CHARGING LOCK | ||
| </span> | ||
| ) : status.allowMotion ? ( | ||
| <span | ||
| className={clsx( | ||
| 'font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5', | ||
| status.stale ? 'text-ink-dim line-through' : 'text-emerald-400 text-glow-emerald', | ||
| )} | ||
| > | ||
| <ShieldCheck className="h-4 w-4" /> ARMED | ||
| ) : status.isEthernetConnected === true ? ( | ||
| <span className="font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5 text-amber-400 text-glow-amber"> | ||
| <ShieldAlert className="h-4 w-4 animate-pulse" /> ETHERNET LOCK | ||
| </span> | ||
| ) : ( | ||
| <span | ||
| className={clsx( | ||
| 'font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5', | ||
| status.stale ? 'text-ink-dim line-through' : 'text-amber-400 text-glow-amber', | ||
| )} | ||
| > | ||
| <ShieldAlert className="h-4 w-4 animate-pulse" /> LOCKED | ||
| <span className="font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5 text-emerald-400 text-glow-emerald"> | ||
| <ShieldCheck className="h-4 w-4" /> ARMED | ||
| </span> | ||
| )} | ||
| <span className="font-mono text-[10px] text-ink-dim truncate mt-1"> | ||
| {status.allowMotion === null | ||
| ? '/ugv/allow_motion not deployed' | ||
| : status.allowMotion | ||
| ? 'Live operation active' | ||
| : 're-gate: beast-paces Ph.2 pending'} | ||
| {status.isCharging === true | ||
| ? 'Plugged into power' | ||
| : status.isEthernetConnected === true | ||
| ? 'Ethernet cable attached' | ||
| : 'Untethered · Battery & Wi-Fi'} | ||
| </span> |
There was a problem hiding this comment.
Suggestion: When both telemetry fields are null because no diagnostic message has arrived, this branch renders ARMED and Untethered · Battery & Wi-Fi. That contradicts the component's unknown-state contract and can falsely reassure the operator that motion is safe. Render an unknown/locked state unless the required telemetry has explicitly reported a safe condition. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Cockpit safety strip falsely reports ARMED during unknown telemetry.
- ❌ Missing tether data is displayed as battery-and-Wi-Fi operation.
- ⚠️ Operators can misread unknown physical-lock status as safe.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/components/cockpit/SafetyStrip.tsx
**Line:** 86:105
**Comment:**
*Incorrect Condition Logic: When both telemetry fields are `null` because no diagnostic message has arrived, this branch renders `ARMED` and `Untethered · Battery & Wi-Fi`. That contradicts the component's unknown-state contract and can falsely reassure the operator that motion is safe. Render an unknown/locked state unless the required telemetry has explicitly reported a safe condition.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+26
to
+28
| if (status.allowMotion === true) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Suggestion: The gate allows motion whenever the bridge is connected and allowMotion is true, even when the status explicitly reports watchdogArmed === false or watchdogFired === true. A fired or inactive watchdog is a known safety-lock condition, so motion tools can still dispatch actions in that state. Require the watchdog safety fields to be known and healthy before returning null. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Agent motion tools dispatch during watchdog safety faults.
- ⚠️ Nav2 reports goals dispatched despite unsafe watchdog state.
- ⚠️ Physical enforcement may stop motion, but server feedback is misleading.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/server/beast/motion-gate.ts
**Line:** 26:28
**Comment:**
*Incomplete Implementation: The gate allows motion whenever the bridge is connected and `allowMotion` is true, even when the status explicitly reports `watchdogArmed === false` or `watchdogFired === true`. A fired or inactive watchdog is a known safety-lock condition, so motion tools can still dispatch actions in that state. Require the watchdog safety fields to be known and healthy before returning `null`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+46
to
+49
| const status = await bridge.getStatus(); | ||
| const gated = gateMotionIntent(status); | ||
| if (gated) return gated; | ||
| return bridge.driveOnHeading(input); |
There was a problem hiding this comment.
Suggestion: The motion path gates only on allowMotion; it still dispatches an action when the status reports watchdogArmed: false or watchdogFired: true (and those fields are explicitly part of BeastRobotStatus). This allows agent motion intents to be sent while the robot watchdog has disabled or faulted motion. Require a healthy, armed, non-fired watchdog before dispatching, treating unknown watchdog state as gated. [security]
Severity Level: Major ⚠️
- ❌ Agent motion goals dispatch during watchdog fault states.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/server/beast/tools.ts
**Line:** 46:49
**Comment:**
*Security: The motion path gates only on `allowMotion`; it still dispatches an action when the status reports `watchdogArmed: false` or `watchdogFired: true` (and those fields are explicitly part of `BeastRobotStatus`). This allows agent motion intents to be sent while the robot watchdog has disabled or faulted motion. Require a healthy, armed, non-fired watchdog before dispatching, treating unknown watchdog state as gated.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+21
to
+31
| export async function POST(req: Request) { | ||
| if (!isHangarAgentEnabled()) { | ||
| return Response.json( | ||
| { | ||
| ok: false, | ||
| error: 'Hangar agent path disabled', | ||
| hint: 'Set HANGAR_AGENT_ENABLED=true to enable /api/agent/chat', | ||
| }, | ||
| { status: 503 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Suggestion: When enabled, this motion-capable route has no authentication, session/origin validation, rate limit, or request-size limit. Any caller that can reach the application can submit arbitrary chat messages and approval responses to invoke ROS tools, bypassing the UI as the only operator boundary. Enforce the same authenticated operator authorization and request limits at this server route. [security]
Severity Level: Critical 🚨
- ❌ Direct API access can reach motion-capable agent workflows.
- ❌ ROS action commands are not protected by route authorization.
- ⚠️ Abuse can consume model and bridge resources.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/agent/chat/route.ts
**Line:** 21:31
**Comment:**
*Security: When enabled, this motion-capable route has no authentication, session/origin validation, rate limit, or request-size limit. Any caller that can reach the application can submit arbitrary chat messages and approval responses to invoke ROS tools, bypassing the UI as the only operator boundary. Enforce the same authenticated operator authorization and request limits at this server route.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+92
to
+99
| {items.map((it, i) => { | ||
| const b = bay(it.bay); | ||
| const us = it.price?.us ?? null; | ||
| const imp = it.price?.import ?? null; | ||
| const relUnits = (it.relatedUnits ?? []).map(unit).filter(Boolean); | ||
| const relMissions = (it.relatedMissions ?? []).map(mission).filter(Boolean); | ||
| const relCaps = (it.relatedCapabilities ?? []).map(capability).filter(Boolean); | ||
| const relInsights = (it.relatedInsights ?? []).map(insight).filter(Boolean); |
There was a problem hiding this comment.
Suggestion: The filter buttons update activeFilter and display an active state, but the catalog still renders items.map(...) for every selection. Selecting Owned, On Order, Wishlist, or Components therefore gives visual feedback without changing the displayed items. Derive a filtered collection from activeFilter and render that collection. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Items page filters provide no catalog filtering.
- ⚠️ Users cannot isolate owned or ordered inventory.
- ⚠️ Components selection has no observable effect.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/items/page.tsx
**Line:** 92:99
**Comment:**
*Incorrect Condition Logic: The filter buttons update `activeFilter` and display an active state, but the catalog still renders `items.map(...)` for every selection. Selecting Owned, On Order, Wishlist, or Components therefore gives visual feedback without changing the displayed items. Derive a filtered collection from `activeFilter` and render that collection.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+158
to
+163
| const cm = mission(g.key); | ||
| const costConstraint = cm?.constraints?.find(c => c.unit === '$'); | ||
| const hypotheticalBudget = costConstraint?.budget ?? 250; | ||
| const cost = g[source]; | ||
| const pct = Math.min(100, Math.max(0, (cost / hypotheticalBudget) * 100)); | ||
| const over = cost > hypotheticalBudget; |
There was a problem hiding this comment.
Suggestion: Groups without a mission or without a dollar constraint are displayed with a fabricated $250 budget and a calculated percentage. The unassigned group is explicitly possible in upgradePath, and missions may have no dollar constraint, so the visualization presents an invented constraint as authoritative. Show an unknown budget state instead of using this fallback. [data type]
Severity Level: Major ⚠️
- ⚠️ Unassigned upgrade paths show fabricated budget limits.
- ⚠️ Missing mission constraints appear as $250 budgets.
- ⚠️ Over-budget indicators can mislead purchasing decisions.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/quartermaster/page.tsx
**Line:** 158:163
**Comment:**
*Data Type: Groups without a mission or without a dollar constraint are displayed with a fabricated `$250` budget and a calculated percentage. The unassigned group is explicitly possible in `upgradePath`, and missions may have no dollar constraint, so the visualization presents an invented constraint as authoritative. Show an unknown budget state instead of using this fallback.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
1177
to
1185
| if (typeof window === 'undefined') return false; | ||
| const live = !!socket && socket.readyState === WebSocket.OPEN; | ||
|
|
||
| if (engaged) { | ||
| // Another tab owns the lock; two heartbeats fighting is the hazard this | ||
| // guards against. Only ENGAGE is gated on being the writer — a release | ||
| // must always be able to drop this tab's own intent. | ||
| if (!getEstopState().writer) return false; | ||
| // No socket means no way to reach the mux. Refusing here is what keeps | ||
| // the UI honest: we never latch a state we could not transmit. | ||
| if (!live) return false; | ||
| const armed = startEstopHeartbeat(); | ||
| if (!armed) return false; | ||
| operatorEngaged = true; | ||
| setEstopState({ engaged: true, engagedAt: getEstopState().engagedAt ?? Date.now() }); | ||
| operatorEngaged = engaged; | ||
| setEstopState({ engaged, engagedAt: engaged ? Date.now() : null, writer: true }); | ||
| if (socket && socket.readyState === WebSocket.OPEN) { | ||
| publishEstopLock(engaged); | ||
| return true; | ||
| } | ||
|
|
||
| // A refused RELEASE must still drop local intent, or the next reconnect | ||
| // would re-assert a lock the operator already cleared. | ||
| operatorEngaged = false; | ||
| if (!live) { | ||
| stopEstopTimers(); | ||
| setEstopState({ engaged: false, engagedAt: null }); | ||
| return false; | ||
| } | ||
| stopEstopHeartbeat(); | ||
| const sent = publishEstopLock(false); | ||
| estopReleaseSends = sent ? 1 : 0; | ||
| if (!estopReleaseTimer) { | ||
| estopReleaseTimer = setInterval(() => { | ||
| if (estopReleaseSends >= ESTOP_RELEASE_SENDS || !publishEstopLock(false)) { | ||
| stopEstopRelease(); | ||
| return; | ||
| } | ||
| estopReleaseSends += 1; | ||
| }, ESTOP_RELEASE_INTERVAL_MS); | ||
| } | ||
| setEstopState({ engaged: false, releasing: true, engagedAt: null }); | ||
| return sent; | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
Suggestion: The setter latches operatorEngaged and publishes only one message, but it never calls startEstopHeartbeat() when engaging or stops the heartbeat and sends the required release burst when releasing. The robot-side lease can therefore expire after the initial true frame, and releasing does not publish false at all. Restore the documented engage/release timer lifecycle here. [logic error]
Severity Level: Critical 🚨
- ❌ Engaged e-stop leases can expire during normal operation.
- ❌ Safety lock release lacks the documented burst.
- ⚠️ Cockpit state can diverge from robot lock state.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/ros/client.ts
**Line:** 1177:1185
**Comment:**
*Logic Error: The setter latches `operatorEngaged` and publishes only one message, but it never calls `startEstopHeartbeat()` when engaging or stops the heartbeat and sends the required release burst when releasing. The robot-side lease can therefore expire after the initial `true` frame, and releasing does not publish `false` at all. Restore the documented engage/release timer lifecycle here.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+186
to
+198
| async start(): Promise<void> { | ||
| if (this.disposed) return; | ||
| if (!this.url) { | ||
| this.state = 'unconfigured'; | ||
| return; | ||
| } | ||
| if (this.ros?.isConnected) { | ||
| this.state = 'connected'; | ||
| return; | ||
| } | ||
| await this.ensureRoslib(); | ||
| this.openSocket(); | ||
| } |
There was a problem hiding this comment.
Suggestion: start() only avoids opening a socket when an existing socket is already connected. Concurrent requests while the client is still connecting can all pass this check and call openSocket(), repeatedly closing and replacing the in-flight connection. Guard the connecting state or serialize connection startup so repeated calls remain idempotent as the comment promises. [race condition]
Severity Level: Major ⚠️
- ⚠️ Concurrent agent requests can restart bridge connection setup.
- ⚠️ In-flight ROS connections may be discarded.
- ⚠️ Initial status and motion requests can fail intermittently.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/server/beast/ros-singleton.ts
**Line:** 186:198
**Comment:**
*Race Condition: `start()` only avoids opening a socket when an existing socket is already connected. Concurrent requests while the client is still connecting can all pass this check and call `openSocket()`, repeatedly closing and replacing the in-flight connection. Guard the connecting state or serialize connection startup so repeated calls remain idempotent as the comment promises.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+329
to
+337
| if (this.ros) { | ||
| this.detachRosListeners(this.ros); | ||
| try { | ||
| this.ros.close(); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| this.ros = null; | ||
| } |
There was a problem hiding this comment.
Suggestion: The action clients are retained across rosbridge reconnects. After onClose, a new Ros instance is created, but this.actions is not cleared, so ensureActions() leaves the old action handles in place and subsequent goals or cancellations target the closed connection. Clear the action handles whenever the underlying ROS connection is torn down or recreate them for each new connection. [stale reference]
Severity Level: Major ⚠️
- ❌ Motion commands can fail after bridge reconnects.
- ❌ Stop cancellation can target stale action handles.
- ⚠️ Recovery requires restarting the server process.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/server/beast/ros-singleton.ts
**Line:** 329:337
**Comment:**
*Stale Reference: The action clients are retained across rosbridge reconnects. After `onClose`, a new `Ros` instance is created, but `this.actions` is not cleared, so `ensureActions()` leaves the old action handles in place and subsequent goals or cancellations target the closed connection. Clear the action handles whenever the underlying ROS connection is torn down or recreate them for each new connection.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Pull request overview
This PR introduces a new Hangar “/agent” command surface and server-side Beast agent stack (model + tools + motion gating) intended for motion-locked/immobile bench sessions, while also consolidating and archiving Beast plans/docs and polishing several cockpit/Hangar UI elements.
Changes:
- Added BEAST agent backend primitives (model config, system prompt, tool schemas, motion gate, tool set) plus
/api/agent/chatstreaming route and/agentUI. - Updated cockpit ROS client/status handling (new charging/ethernet lock signals) and adjusted cockpit UI behaviors/animations.
- Reorganized Beast documentation: new control-topology doc, new plan set (Set 1–5) + immobile execution work order, and archived superseded plan drafts with corpus/seed updates.
Reviewed changes
Copilot reviewed 70 out of 76 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/beast/types.ts | New Beast agent/tooling types + limits |
| src/server/beast/tools.ts | Agent ToolSet with approval metadata |
| src/server/beast/schemas.ts | Zod schemas for motion tools |
| src/server/beast/prompts.ts | System prompt for Beast agent |
| src/server/beast/motion-gate.ts | Motion-intent refusal logic |
| src/server/beast/model.ts | OpenAI-compatible model wiring + env gating |
| src/lib/ros/estop-store.ts | Simplified e-stop store shape |
| src/lib/ros/client.ts | Cockpit status fields + estop/service updates |
| src/components/UnitCard.tsx | Compute-bay chip + hover/tap motion |
| src/components/Shell.tsx | Adds /agent nav + mobile-nav tweaks |
| src/components/cockpit/SafetyStrip.tsx | E-stop + motion-state presentation changes |
| src/components/cockpit/OpticsWall.tsx | Visual polish + clearance comment tweak |
| src/components/cockpit/HonestyRail.tsx | Updated honesty messaging |
| src/components/cockpit/CommandRail.tsx | Drive gating + tap animations |
| src/components/board/TwinCanvas.tsx | Adds legend + junction markers |
| src/components/board/Port.tsx | Ownership badge on ports |
| src/app/tech-tree/page.tsx | Layout tweak + dependency styling |
| src/app/quartermaster/page.tsx | Pipeline diagram + animated budget bars |
| src/app/page.tsx | Staggered UnitCard grid animation wrapper |
| src/app/missions/page.tsx | Objective progress ring UI |
| src/app/items/page.tsx | Filter-chip UI + AnimatePresence list |
| src/app/cockpit/CockpitClient.tsx | Simplifies unmount behavior |
| src/app/bay/[id]/page.tsx | Adds bay “command panel” block |
| src/app/api/agent/chat/route.ts | New streaming agent chat API route |
| src/app/agent/page.tsx | New /agent page wrapper |
| src/app/agent/AgentClient.tsx | New agent chat UI with tool approvals |
| src/tests/ros-singleton.test.ts | New server-side rosbridge singleton tests |
| src/tests/ros-client.test.ts | Updates for changed estop/voltage behavior |
| src/tests/command-rail.test.tsx | New CommandRail lifecycle tests |
| src/tests/cockpit-client.test.tsx | Updates for cockpit unmount behavior |
| src/tests/briefings-parity.test.ts | Updates archived plan repoPath constant |
| src/tests/agent-tools.test.ts | New agent tool gating/approval tests |
| src/tests/agent-model.test.ts | New agent model env tests |
| README.md | Adds Beast ops/topology pointers + repo map |
| public/beast-ups-i2c-wiring.svg | New UPS→Jetson I2C wiring diagram asset |
| package.json | Adds AI SDK + roslib dependencies |
| docs/plans/README.md | New accepted plan index + archived section |
| docs/plans/beast-command-deck-drafts/twist_mux.yaml | Removed draft (archived/cleaned up) |
| docs/plans/beast-command-deck-drafts/teleop_joy_operator.yaml | Removed draft (archived/cleaned up) |
| docs/plans/beast-command-deck-drafts/README.md | Removed draft (archived/cleaned up) |
| docs/plans/beast-command-deck-drafts/foxglove_bridge.launch.py | Removed draft (archived/cleaned up) |
| docs/plans/beast-command-deck-drafts/cockpit_robot.launch.py | Removed draft (archived/cleaned up) |
| docs/plans/archived/README.md | New archive index + extraction targets |
| docs/plans/archived/2026-07-31-beast-command-deck-spec.md | Updated references post-absorption |
| docs/plans/archived/2026-07-30-wiring-model-completion.md | Moved wiring-model plan into archive path |
| docs/plans/2026-08-02-beast-immobile-execution.md | New immobile execution work order |
| docs/plans/2026-08-02-beast-agent-pr5-hygiene.md | New Set 5 hygiene plan |
| docs/plans/2026-08-02-beast-agent-pr4-agent-command.md | New Set 4 command-path plan |
| docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md | New Set 3 lidar/slam/nav plan |
| docs/plans/2026-08-02-beast-agent-pr2-power-telemetry.md | New Set 2 power plan |
| docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md | New Set 1 safety-spine plan |
| docs/plans/2026-08-01-beast-cockpit-future-roadmap.md | Removed old roadmap from plans |
| docs/plans/2026-07-31-beast-command-deck-plan.md | Removed superseded plan |
| docs/plans/2026-07-11-beast-nvme-storage-implementation.md | Removed old storage plan |
| docs/hardware-library.md | Points to archived wiring plan path |
| docs/beast-control-topology.md | New cross-repo authority/topology doc |
| docs/beast-cockpit-future-roadmap.md | Thin pointer to Datacore idea bank |
| db/hangar/seed.sql | Updates briefing repoPath for archived plan |
| db/hangar/research-corpus-registry.ts | Updates corpus source path |
| db/hangar/research-corpus-manifest.json | Updates manifest repoPath |
| beast_status.ps1 | New Windows SSH helper script |
| beast_probe.ps1 | New Windows SSH probe script |
| AGENTS.md | Adds “always commit + PR” + repo split reminder |
| .cursor/rules/always-commit-and-pr.mdc | New Cursor rule enforcing commit/PR workflow |
| .claude/launch.json | Adds Claude launch config |
| _ppp/worksheet.md | Worksheet for wiring diagram provenance |
| .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md | Adds kilo plan artifact |
| .playwright-mcp/page-2026-08-02T16-22-03-718Z.yml | Playwright MCP artifact reference |
| .playwright-mcp/page-2026-07-31T17-56-40-364Z.yml | Playwright MCP artifact reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1176
to
+1180
| setEstopLock(engaged: boolean): boolean { | ||
| if (typeof window === 'undefined') return false; | ||
| const live = !!socket && socket.readyState === WebSocket.OPEN; | ||
|
|
||
| if (engaged) { | ||
| // Another tab owns the lock; two heartbeats fighting is the hazard this | ||
| // guards against. Only ENGAGE is gated on being the writer — a release | ||
| // must always be able to drop this tab's own intent. | ||
| if (!getEstopState().writer) return false; | ||
| // No socket means no way to reach the mux. Refusing here is what keeps | ||
| // the UI honest: we never latch a state we could not transmit. | ||
| if (!live) return false; | ||
| const armed = startEstopHeartbeat(); | ||
| if (!armed) return false; | ||
| operatorEngaged = true; | ||
| setEstopState({ engaged: true, engagedAt: getEstopState().engagedAt ?? Date.now() }); | ||
| operatorEngaged = engaged; | ||
| setEstopState({ engaged, engagedAt: engaged ? Date.now() : null, writer: true }); | ||
| if (socket && socket.readyState === WebSocket.OPEN) { |
Comment on lines
+95
to
97
| <span className="font-mono text-lg font-bold tracking-wide flex items-center gap-1.5 mt-0.5 text-emerald-400 text-glow-emerald"> | ||
| <ShieldCheck className="h-4 w-4" /> ARMED | ||
| </span> |
Comment on lines
+71
to
74
| // ── MOTION GATE: PHYSICAL TETHER & CHARGING LOCK ───────────────────────── | ||
| // Motion is disabled ONLY if the robot is charging or plugged into Ethernet | ||
| // (to prevent tearing cables out), or if software E-STOP is engaged. | ||
| const driveGateReason: string | null = !connected |
Comment on lines
+21
to
+31
| export async function POST(req: Request) { | ||
| if (!isHangarAgentEnabled()) { | ||
| return Response.json( | ||
| { | ||
| ok: false, | ||
| error: 'Hangar agent path disabled', | ||
| hint: 'Set HANGAR_AGENT_ENABLED=true to enable /api/agent/chat', | ||
| }, | ||
| { status: 503 }, | ||
| ); | ||
| } |
Comment on lines
+91
to
+93
| <AnimatePresence> | ||
| {items.map((it, i) => { | ||
| const b = bay(it.bay); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 56
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/cockpit/OpticsWall.tsx (1)
65-74: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBound-test the new clearance thresholds.
Add coverage for the new critical and warning boundaries at
0.159m,0.160m, and0.280m.🤖 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/cockpit/OpticsWall.tsx` around lines 65 - 74, Update the tests for the clearanceStatus logic in OpticsWall to cover values of 0.159m, 0.160m, and 0.280m, asserting that 0.159m is CRITICAL while 0.160m and 0.280m are not classified as CRITICAL or WARNING according to the existing strict threshold comparisons.src/lib/ros/client.ts (1)
1346-1354: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe
powerbranch overwrites metrics parsed fromsystem_metrics.Lines 1351-1354 assign
wifiRssi,diskFree,cpuTempandgpuTempunconditionally. Apowerdiagnostic that carries onlychargingtherefore resets all four fields tonull, including values that asystem_metricsentry set earlier in the samediagArray.forEachpass. Lines 1347-1349 already use the!== undefinedguard pattern; apply it to the remaining fields.🐛 Proposed fix
} else if (d.name === 'system_metrics' || d.name === 'power') { if (values.charging !== undefined) next.isCharging = safeBool(values.charging); if (values.ethernet !== undefined || values.ethernet_connected !== undefined) { next.isEthernetConnected = safeBool(values.ethernet_connected ?? values.ethernet); } - next.wifiRssi = safeNumber(values.wifi_rssi); - next.diskFree = values.disk_free || null; - next.cpuTemp = safeNumber(values.cpu_temp); - next.gpuTemp = safeNumber(values.gpu_temp); + if (values.wifi_rssi !== undefined) next.wifiRssi = safeNumber(values.wifi_rssi); + if (values.disk_free !== undefined) next.diskFree = values.disk_free || null; + if (values.cpu_temp !== undefined) next.cpuTemp = safeNumber(values.cpu_temp); + if (values.gpu_temp !== undefined) next.gpuTemp = safeNumber(values.gpu_temp); }🤖 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/lib/ros/client.ts` around lines 1346 - 1354, Update the power/system_metrics handling in the diagnostic iteration to assign wifiRssi, diskFree, cpuTemp, and gpuTemp only when their corresponding values are not undefined, preserving previously parsed system_metrics values when a power diagnostic omits them. Follow the existing guarded assignment pattern used for charging and Ethernet fields.
🤖 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 @.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md:
- Around line 138-140: Update the Wi-Fi reachability statement in the plan to
label the claim as historical and include the actual last-verified date, August
2, 2026. Preserve the existing unreachable details and LAN dependency for
robot-side phases.
- Around line 98-99: Update the example launch command to pass a declared
use_localplan value supported by nav.launch.py, such as dwa or teb, instead of
rpp; only retain rpp if nav.launch.py is updated to declare and wire it through.
In `@AGENTS.md`:
- Around line 21-30: Update the “Two repos for BEAST-01” section in AGENTS.md to
retain only the BEAST-specific process warning and replace its duplicated
repository table and topology details with links to README.md and
docs/beast-control-topology.md. Keep repository paths, roles, and location
guidance owned by README.md.
- Around line 13-19: Update the “Always commit and open a PR” guidance to
explicitly allow skipping the requirement when the user says not to commit, not
to push, or not to open a PR. Keep the existing cross-repository PR instructions
and reference to the enforcement rule unchanged.
In `@beast_probe.ps1`:
- Line 8: Replace StrictHostKeyChecking=no with StrictHostKeyChecking=yes in the
SSH commands at beast_probe.ps1 lines 8, beast_status.ps1 lines 5 and 11, and
ensure valid known_hosts entries are available for the robot hosts.
- Around line 8-12: Check $LASTEXITCODE immediately after each ssh invocation in
beast_probe.ps1 lines 8-12 and beast_status.ps1 lines 4-13, and treat any
non-zero value as failure so the corresponding catch/failure reporting runs
instead of printing SUCCESS or omitting SERVICE CHECK FAILED/PARAM CHECK FAILED.
Preserve the existing success output only for zero exit codes.
In `@beast_status.ps1`:
- Line 11: Update the SSH command in beast_status.ps1 to target the current
BEAST-01 SSH host and query the live, re-probed ROS 2 node name for
allow_motion, using the node name documented in docs/beast-ops.md (/ugv_bringup)
if confirmed. Replace the existing /bringup argument while preserving the ROS
environment setup and output behavior.
- Around line 1-5: Update beast_status.ps1 lines 1-13 to use the Quick connect
source via ssh beast-01 instead of the hard-coded key, IP address, and SSH
options; keep direct fallbacks documented in the Quick connect block. For
beast_probe.ps1 lines 1-3, either retain mDNS/Tailscale and direct fallback
discovery only if intentional, or derive its targets from the same Quick connect
source.
In `@db/hangar/research-corpus-manifest.json`:
- Line 25: Synchronize the corpus path declarations: in
db/hangar/research-corpus-manifest.json at lines 25-25, keep
src/data/datacore-corpus.ts mapped to the archived path; in
db/hangar/research-corpus-registry.ts at lines 99-99, add or update parity
coverage validating the manifest, registry, and runtime corpus paths.
In `@db/hangar/seed.sql`:
- Line 2005: Update the seeded briefing content associated with
$b_wiring_model_completion$ so its embedded body_markdown references use the
archived document consistently: change the stale ACTIVE status and old
active-path link to match
docs/plans/archived/2026-07-30-wiring-model-completion.md, while preserving the
updated repo_path.
In `@docs/beast-control-topology.md`:
- Line 105: Update the Quick connect link in the allow_motion/publisher counts
reference to use the heading’s complete generated anchor, including the Wave 3
session close suffix, so the link resolves correctly.
In `@docs/beast-ops.md`:
- Around line 1852-1866: Rewrite the heartbeat test section in the documented
procedure to remove the ESP32 three-second stale-command watchdog and instead
describe the supervised Jetson cmd_vel_timeout crawl-and-kill check. Require
verification of the documented stop result, or explicitly instruct the operator
to wait for motion lock before enabling motion, while preserving the existing
safety constraints and command flow.
- Around line 648-651: Update the WSS transport row in the documentation so it
is not labeled as current; mark it unavailable based on the existing dated
verification, unless live SSH verification confirms and supports updating the
Quick connect block and transport status.
- Around line 1820-1827: Update the bringup_lidar.launch.py command to use the
current live USB serial device path /dev/ttyACM0 instead of the retired
/dev/ttyTHS1 path, keeping the remaining environment setup and launch arguments
unchanged.
In `@docs/hardware-library.md`:
- Line 97: Remove the temporary archived-plan link from the Hardware Library
documentation and replace it with the stable CAD-purpose reference in the owner
document or the replacement live work order, preserving useful navigation
without referencing the deletable archive file.
In `@docs/plans/2026-08-02-beast-agent-architecture.md`:
- Line 217: The safety-status topic contract is inconsistent across the plans.
In docs/plans/2026-08-02-beast-agent-architecture.md:217, declare the canonical
producer topic and message type; in
docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md:44-45, replace
“compatible” wording with the exact wire contract; in
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md:104-105, bind the agent
UI to that canonical topic; and in
docs/plans/2026-08-02-beast-immobile-execution.md:21-23, record the same topic
or explicitly document an adapter.
- Around line 292-295: Standardize the disarmed-goal contract across all cited
documents: locked goals must run without actuation and end in the explicit
ABORTED terminal state. Update docs/plans/2026-08-02-beast-agent-architecture.md
lines 292-295 and 350-357, docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md
lines 63-66, docs/plans/2026-08-02-beast-agent-pr4-agent-command.md lines 76-81,
and docs/plans/2026-08-02-beast-immobile-execution.md line 24 to reflect this
behavior, while keeping only armed Nav2 goals excluded from scope.
- Around line 24-32: The bounded-skill contract must require verified costmap
collision checking before motion is available. In
docs/plans/2026-08-02-beast-agent-architecture.md:24-32, make the costmap path
an explicit prerequisite for bounded skills; in
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md:73-79, remove the
blind-primitives option or mark motion tools unavailable until collision
checking is verified.
- Around line 37-47: Align the rosbridge allowlist documentation across the
master and commissioning plans: in
docs/plans/2026-08-02-beast-agent-architecture.md lines 37-47,
docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md lines 28-35,
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md lines 83-95, and
docs/plans/2026-08-02-beast-immobile-execution.md lines 237-241, replace the
blanket “no services/actions” guidance with the exact topics_glob/services_glob
entries required for the Nav2 goal, cancel, feedback, and result flow. Keep the
idle-session failure rule restrictive so no additional services or actions are
exposed until explicitly needed.
In `@docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md`:
- Around line 58-62: Update the EstopLock release transition in the state
diagram so operator release and re-arm first re-evaluate EthernetLock,
ChargingLock, and any other active interlocks before permitting Armed; route
back into the applicable lock state when a condition remains active rather than
transitioning directly to Armed.
- Around line 42-45: Update the charging interlock and arming flow described
under “Charging” so charging_active telemetry must be present and fresh before
arming is allowed. Treat an absent or stale charging_active topic as a motion
lock rather than fail-open, while preserving the existing default-disarmed guard
and publishing the corresponding lock reason through /cockpit/status-compatible
diagnostics.
In `@docs/plans/2026-08-02-beast-agent-pr2-power-telemetry.md`:
- Around line 36-41: Choose either beast_power or ugv_bringup as the sole
publisher of /ugv/voltage, remove the competing publisher or relay, and document
the ownership decision in the PR-2b plan. Update relevant tests to assert that
exactly one publisher provides the BatteryState topic and prevent competing
charging-state samples.
In `@docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md`:
- Around line 51-56: Align the motion limits across the documented plans: in
docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md lines 51-56, explicitly
apply the ≤0.15 m/s limit to all motion-bearing work; in
.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines 85-87, replace the
0.26 m/s mapping limit or document it as a separately tested teleoperation-only
limit.
- Around line 45-49: The motion-gate references are undefined and must use the
named Set 1 re-gate with dated proof. In
docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md lines 45-49, replace the
“Phase 0.5” dependency with the Set 1 crawl-and-kill re-gate and its dated
proof; in .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines 80-84,
require that same gate before setting allow_motion:=true.
In `@docs/plans/2026-08-02-beast-agent-pr5-hygiene.md`:
- Around line 43-46: The Vizanti migration documentation must provide a
replacement launch path before removing ugv_web_app. In
docs/plans/2026-08-02-beast-agent-pr5-hygiene.md lines 43-46, update the
reference sweep to require documenting and validating the replacement before
deletion; in .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines
17-20, document launching Vizanti directly and the required bridge
configuration.
In `@docs/plans/2026-08-02-beast-immobile-execution.md`:
- Around line 33-40: Update the Packaging status table to replace every “(fill
after gh pr create)” entry with its actual GitHub PR URL after confirming the
corresponding work was committed, pushed, and opened. If any repository lacks a
completed PR, mark that work unfinished rather than leaving placeholder records.
- Around line 154-157: Update the Set 1a test plan to remove any instruction to
flip or test allow_motion:=true during a hard-ban session. Keep this session
limited to verifying the locked false gate with motors inert, and move any
armed-path validation to the next-session gate as directed by the surrounding
plan.
In `@docs/plans/archived/2026-07-30-wiring-model-completion.md`:
- Line 3: Update the plan’s Status metadata from ACTIVE to PARKED or ARCHIVED,
and explicitly state that a new work order is required before any wiring phases
can be executed.
In `@docs/plans/archived/README.md`:
- Line 14: Update the archived README row for
2026-07-31-beast-command-deck-spec.md to reflect the current status of
docs/beast-control-topology.md: mark the topology extraction complete if the
document landed, or replace the future-work note with the exact remaining
content required before deletion.
In `@src/__tests__/agent-model.test.ts`:
- Around line 26-31: Extend the test “gates the hangar agent behind
HANGAR_AGENT_ENABLED” to verify that isHangarAgentEnabled() returns true for the
exact value '1' and false for a rejected variant such as 'TRUE', while
preserving the existing empty and 'true' assertions.
In `@src/__tests__/agent-tools.test.ts`:
- Around line 128-133: Add a test alongside the existing gateMotionIntent
coverage in agent-tools.test.ts that passes status values with connection set to
disconnected and error while motion is allowed, and assert each result has
status bridge_unavailable. Keep the unconfigured test unchanged and target the
separate non-connected branch in gateMotionIntent.
In `@src/__tests__/cockpit-client.test.tsx`:
- Line 78: Replace the unconditional unmount disconnect test with E-STOP
coverage: in the CockpitClient unmount test, engage E-STOP before calling
unmount(), then assert the socket remains connected or retained and is not
disconnected. Preserve the existing test setup and use the component’s
established E-STOP and socket/disconnect symbols.
In `@src/__tests__/command-rail.test.tsx`:
- Around line 112-124: Add a test alongside the charging case that sets
mocks.isEthernetConnected to true, renders CommandRail, advances timers, clears
publish calls, and sends a non-repeating “w” keydown. Assert motion is not
published and the “motion locked — Ethernet tether connected” message is
rendered, using the existing mock and test setup.
- Around line 60-67: Update the beforeEach setup in command-rail tests to reset
both mocks.isCharging and mocks.isEthernetConnected to their default values,
preventing state leakage between tests while preserving the existing mock
resets.
In `@src/__tests__/estop-election.test.ts`:
- Around line 10-15: Update the E-STOP tests around setEstopLock to assert its
return value instead of relying only on getEstopState().engaged. Keep the
no-socket case aligned with the intended failure behavior, and add a separate
connected case using a mocked open socket that verifies the published frame and
confirms engaged is true.
In `@src/app/agent/AgentClient.tsx`:
- Around line 282-292: Add a stable, explicit aria-label to the chat input
element in AgentClient, independent of the conditional placeholder text. Keep
the existing value, disabled state, placeholder, and styling behavior unchanged.
In `@src/app/api/agent/chat/route.ts`:
- Around line 56-58: Update the fire-and-forget call in the route’s bridge
startup flow to attach a rejection handler to bridge.start(), preventing
failures from the awaited roslib import from becoming unhandled promise
rejections while preserving the existing non-blocking behavior.
- Around line 45-54: Update the request parsing in the agent chat route to
validate body.messages with the existing Zod UIMessage schema before assigning
or passing it to convertToModelMessages and streamText, returning the
established 400 response for invalid message objects. Also add the same operator
authentication or explicit operator-token check used by other protected routes
before allowing agent tool invocation, while preserving the HANGAR_AGENT_ENABLED
gate.
In `@src/app/bay/`[id]/page.tsx:
- Around line 39-52: Update the Bay Command Panel metrics in the page component
to use a defined data source tied to the selected bay and its current condition
instead of hardcoded “85%” and “1.2kW” values; if no such source exists,
explicitly label both metrics as static design values.
In `@src/app/items/page.tsx`:
- Around line 52-76: The activeFilter state currently changes only button
styling; update the items catalog rendering to apply predicates for each
supported value in mockFilters before mapping entries, while preserving the
existing unfiltered behavior for the default filter. Use the activeFilter and
items symbols in the page’s catalog rendering path, or remove any filter
controls whose values cannot be implemented.
In `@src/app/quartermaster/page.tsx`:
- Around line 106-134: Replace the Tailwind-class parsing used for the pipeline
status dot in the ACQUISITION_PIPELINE_STATUSES map with an explicit color value
sourced from WISHLIST_STATUS_META or a status-to-color mapping. Update the
relevant metadata or mapping and the dot className so watching retains its
intended indicator color without deriving colors from st.cls.
- Around line 157-185: Update the budget display in the upgradePath map near
costConstraint and hypotheticalBudget so groups without a dollar constraint do
not use or display the $250 fallback. Keep the budget optional: show “Budget
unavailable” and omit the progress bar when costConstraint is absent, while
preserving the existing cost, percentage, and over-budget behavior for groups
with a real dollar budget.
In `@src/components/board/Port.tsx`:
- Around line 39-48: Update the ownership inference in Port.tsx to use a shared
wiring-derived mapping for driver-board ports whose host varies by build, rather
than maintaining terminal-ID exceptions locally. Extend the wiring model in
src/data/wiring.ts with the mapping and reuse it when selecting the π or Σ
badge, preserving existing unit and terminal behavior for other ports.
In `@src/components/board/TwinCanvas.tsx`:
- Around line 151-164: Move the legend `<g>` containing the `LEGEND`, `Junction
(⊕)`, and `Ownership (π/Σ)` elements out of the transformed board/world group
and render it as a sibling group at the SVG root level. Preserve its existing
translate position and styling while ensuring pan-and-zoom transforms apply only
to board content.
In `@src/components/cockpit/CommandRail.tsx`:
- Around line 71-84: The cockpit must honor the robot-reported arming state and
represent unknown tether data correctly. In
src/components/cockpit/CommandRail.tsx lines 71-84, add an status.allowMotion
=== false gate to driveGateReason so motion is disabled when the robot reports
disarmed. In src/components/cockpit/SafetyStrip.tsx lines 86-104, render Unknown
when both isCharging and isEthernetConnected are null, and retain an
allowMotion-based indication in the motion-state block instead of always
displaying ARMED.
- Around line 71-84: Update the driveGateReason chain in CommandRail to consult
status.allowMotion and lock motion whenever the robot’s arming flag is not true,
preserving the existing connectivity, E-STOP, charging, Ethernet, and dead-topic
checks. Keep the resulting gate behavior consistent with the agent motion-gate
path and ensure the commanding indicator cannot activate when the robot rejects
motion.
In `@src/components/cockpit/HonestyRail.tsx`:
- Around line 15-19: Update the E-STOP chip title in HonestyRail to explicitly
state that the latch is software-only and depends on the rosbridge link, while
preserving the existing teleoperation context and rendered chip structure.
In `@src/components/cockpit/OpticsWall.tsx`:
- Around line 139-148: Update the animated reticles in OpticsWall, including the
motion.svg blocks gated by rgb.active and depth.active, to respect the user’s
reduced-motion preference. Use Framer Motion’s reduced-motion support or omit
the opacity animation and transition when reduced motion is enabled, while
preserving the existing reticle rendering and animation for other users.
In `@src/components/cockpit/SafetyStrip.tsx`:
- Around line 59-63: Update the motion.section animation in SafetyStrip so
clearing estopEngaged explicitly animates borderColor back to the resting
`#404040` value instead of using an empty target. First confirm `#404040` matches
the border-rim token, then preserve the existing pulsing keyframes and timing
while ensuring the inline colour resets when E-STOP is disengaged.
In `@src/lib/ros/client.ts`:
- Around line 1176-1185: Update src/lib/ros/client.ts lines 1176-1185 in
setEstopLock so the socket-open check and successful publishEstopLock occur
before setEstopState; return false without mutating the store when no frame is
sent, while preserving the successful state update. Update
src/__tests__/estop-election.test.ts lines 10-15 to verify the closed-socket
failure leaves engaged false, and add a separate open-mock-socket case covering
the successful path.
- Around line 1151-1161: Update callService to return a boolean like publish:
return false when the socket is unavailable and return true only after the
service request is successfully sent, so callers can detect whether it left the
browser. Preserve the existing request payload while replacing the random call
ID generation with the established opId scheme used by publish, including
service identity as required.
In `@src/server/beast/ros-singleton.ts`:
- Around line 359-367: Update scheduleReconnect to use an exponentially
increasing reconnect delay with a defined maximum cap instead of always using
reconnectMs. Track the current backoff across failed attempts, and reset it to
the initial reconnectMs when the successful connection event is handled.
Preserve the existing disposal, intentional-close, timer cleanup, and
socket-opening guards.
- Around line 145-155: Remove the unreachable inner condition and its state
assignment from the onClose method. Keep the outer early-return guard for
intentionalClose, disposed, or missing url, and preserve the disconnected state
update and scheduleReconnect flow for active unexpected closures.
- Around line 186-198: Update start() in the connection lifecycle to return
without reopening when this.state is 'connecting' or a reconnect timer is
pending, in addition to the existing connected guard. Preserve the current
unconfigured and connected handling, and ensure openSocket() only runs when no
connection attempt or scheduled reconnect is active.
- Around line 270-276: Update the backUp method’s runAction('backup') goal so
target is a geometry_msgs/Point object with the requested backup distance on the
appropriate axis, matching the shape used by driveOnHeading; preserve the
existing speed and time_allowance calculations.
In `@src/server/beast/tools.ts`:
- Around line 81-85: Update toolNeedsApproval to avoid eagerly constructing
createAgentTools(noopBridge()) when tools is omitted; derive approval from
MOTION_TOOL_NAMES in that case, while preserving metadata-based checks for an
explicitly provided ToolSet. Remove noopBridge if it becomes unused and is not
imported by tests.
---
Outside diff comments:
In `@src/components/cockpit/OpticsWall.tsx`:
- Around line 65-74: Update the tests for the clearanceStatus logic in
OpticsWall to cover values of 0.159m, 0.160m, and 0.280m, asserting that 0.159m
is CRITICAL while 0.160m and 0.280m are not classified as CRITICAL or WARNING
according to the existing strict threshold comparisons.
In `@src/lib/ros/client.ts`:
- Around line 1346-1354: Update the power/system_metrics handling in the
diagnostic iteration to assign wifiRssi, diskFree, cpuTemp, and gpuTemp only
when their corresponding values are not undefined, preserving previously parsed
system_metrics values when a power diagnostic omits them. Follow the existing
guarded assignment pattern used for charging and Ethernet fields.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9795bce-ae25-48ee-ae7d-bd5435c57c8a
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/beast-ups-i2c-wiring.svgis excluded by!**/*.svgups-module-3s-header.pngis excluded by!**/*.png
📒 Files selected for processing (73)
.claude/launch.json.cursor/rules/always-commit-and-pr.mdc.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md.playwright-mcp/page-2026-07-31T17-56-40-364Z.yml.playwright-mcp/page-2026-08-02T16-22-03-718Z.ymlAGENTS.mdREADME.md_ppp/worksheet.mdbeast_probe.ps1beast_status.ps1db/hangar/research-corpus-manifest.jsondb/hangar/research-corpus-registry.tsdb/hangar/seed.sqldocs/beast-cockpit-future-roadmap.mddocs/beast-control-topology.mddocs/beast-ops.mddocs/hardware-library.mddocs/plans/2026-07-11-beast-nvme-storage-implementation.mddocs/plans/2026-07-31-beast-command-deck-plan.mddocs/plans/2026-08-01-beast-cockpit-future-roadmap.mddocs/plans/2026-08-02-beast-agent-architecture.mddocs/plans/2026-08-02-beast-agent-pr1-safety-spine.mddocs/plans/2026-08-02-beast-agent-pr2-power-telemetry.mddocs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.mddocs/plans/2026-08-02-beast-agent-pr4-agent-command.mddocs/plans/2026-08-02-beast-agent-pr5-hygiene.mddocs/plans/2026-08-02-beast-immobile-execution.mddocs/plans/README.mddocs/plans/archived/2026-07-30-wiring-model-completion.mddocs/plans/archived/2026-07-31-beast-command-deck-spec.mddocs/plans/archived/README.mddocs/plans/beast-command-deck-drafts/README.mddocs/plans/beast-command-deck-drafts/beast-command-deck.htmldocs/plans/beast-command-deck-drafts/cockpit_robot.launch.pydocs/plans/beast-command-deck-drafts/foxglove_bridge.launch.pydocs/plans/beast-command-deck-drafts/teleop_joy_operator.yamldocs/plans/beast-command-deck-drafts/twist_mux.yamlpackage.jsonsrc/__tests__/agent-model.test.tssrc/__tests__/agent-tools.test.tssrc/__tests__/briefings-parity.test.tssrc/__tests__/cockpit-client.test.tsxsrc/__tests__/command-rail.test.tsxsrc/__tests__/estop-election.test.tssrc/__tests__/ros-client.test.tssrc/__tests__/ros-singleton.test.tssrc/app/agent/AgentClient.tsxsrc/app/agent/page.tsxsrc/app/api/agent/chat/route.tssrc/app/bay/[id]/page.tsxsrc/app/cockpit/CockpitClient.tsxsrc/app/items/page.tsxsrc/app/missions/page.tsxsrc/app/page.tsxsrc/app/quartermaster/page.tsxsrc/app/tech-tree/page.tsxsrc/components/Shell.tsxsrc/components/UnitCard.tsxsrc/components/board/Port.tsxsrc/components/board/TwinCanvas.tsxsrc/components/cockpit/CommandRail.tsxsrc/components/cockpit/HonestyRail.tsxsrc/components/cockpit/OpticsWall.tsxsrc/components/cockpit/SafetyStrip.tsxsrc/lib/ros/client.tssrc/lib/ros/estop-store.tssrc/server/beast/model.tssrc/server/beast/motion-gate.tssrc/server/beast/prompts.tssrc/server/beast/ros-singleton.tssrc/server/beast/schemas.tssrc/server/beast/tools.tssrc/server/beast/types.ts
💤 Files with no reviewable changes (10)
- docs/plans/beast-command-deck-drafts/README.md
- docs/plans/beast-command-deck-drafts/teleop_joy_operator.yaml
- docs/plans/2026-07-11-beast-nvme-storage-implementation.md
- docs/plans/beast-command-deck-drafts/foxglove_bridge.launch.py
- docs/plans/2026-07-31-beast-command-deck-plan.md
- docs/plans/beast-command-deck-drafts/beast-command-deck.html
- src/app/cockpit/CockpitClient.tsx
- docs/plans/beast-command-deck-drafts/cockpit_robot.launch.py
- docs/plans/2026-08-01-beast-cockpit-future-roadmap.md
- docs/plans/beast-command-deck-drafts/twist_mux.yaml
Comment on lines
+98
to
+99
| 2. `ros2 launch ugv_nav nav.launch.py use_localization:=slam_toolbox | ||
| use_localplan:=rpp` (RPP is simplest robust controller; TEB also tuned). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
git clone --depth 1 https://github.com/Coldaine/ugv_ws "$tmp_dir/ugv_ws"
rg -n -C 4 'use_localplan|use_localplanner|DeclareLaunchArgument' \
"$tmp_dir/ugv_ws/src/ugv_main/ugv_nav"Repository: Coldaine/RobotOverview
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== nav.launch.py relevant sections =="
sed -n '1,180p' src/ugv_main/ugv_nav/launch/nav.launch.py
echo "== use_localplan and rpp references in tracked repository files =="
rg -n -C 3 'use_localplan|rpp|localplan|controller' README.md .kilo src 2>/dev/null || true
echo "== exact file lines =="
wc -l .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md
sed -n '90,105p' .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.mdRepository: Coldaine/RobotOverview
Length of output: 283
Use a declared use_localplan value.
nav.launch.py declares use_localplan with defaults/choices dwa and teb, so use_localplan:=rpp is not a valid launch argument. It will be treated as an unknown user-supplied argument; use an existing local planner value or add and wire through rpp before citing it here.
🤖 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 @.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md around lines 98 -
99, Update the example launch command to pass a declared use_localplan value
supported by nav.launch.py, such as dwa or teb, instead of rpp; only retain rpp
if nav.launch.py is updated to declare and wire it through.
Comment on lines
+138
to
+140
| - **Wi-Fi reachability**: robot was unreachable during this planning | ||
| session (`beast-01.local` DNS fail, Tailscale + .187 timeouts) — all | ||
| robot-side phases need it on the LAN. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an explicit last-verified date to the reachability claim.
The plan says the robot was unreachable but does not state the verification date in the claim. Add the actual date, such as “last verified August 2, 2026,” and label the statement as historical.
🤖 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 @.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md around lines 138 -
140, Update the Wi-Fi reachability statement in the plan to label the claim as
historical and include the actual last-verified date, August 2, 2026. Preserve
the existing unreachable details and LAN dependency for robot-side phases.
Source: Path instructions
Comment on lines
+13
to
+19
| ## Always commit and open a PR | ||
|
|
||
| Finished work is not done until it is **committed, pushed, and in a GitHub PR** | ||
| (each repo that changed). Do not leave session deliverables as an uncommitted | ||
| tree or an unpushed branch. Cross-repo Beast work → PR in RobotOverview **and** | ||
| `Coldaine/ugv_ws` when both changed. Only skip when the user explicitly says not | ||
| to commit / not to open a PR. (Also enforced by `.cursor/rules/always-commit-and-pr.mdc`.) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include the explicit “do not push” exception.
The exception text omits “not to push,” although .cursor/rules/always-commit-and-pr.mdc includes it. A user who explicitly says not to push can still be interpreted as subject to the push requirement. State all three exceptions: not to commit, not to push, or not to open a PR.
As per coding guidelines, an explicit request not to commit, not to push, or not to open a PR is an allowed exception.
🤖 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 `@AGENTS.md` around lines 13 - 19, Update the “Always commit and open a PR”
guidance to explicitly allow skipping the requirement when the user says not to
commit, not to push, or not to open a PR. Keep the existing cross-repository PR
instructions and reference to the enforcement rule unchanged.
Source: Coding guidelines
Comment on lines
+21
to
+30
| ## Two repos for BEAST-01 (do not look for `ugv_ws` in this tree) | ||
|
|
||
| | Repo | On-disk (this PC) | Role | | ||
| | --- | --- | --- | | ||
| | **This repo** (`RobotOverview`) | `D:\_projects\RobotOverview` | Hangar UI, `/cockpit`, `/agent`, plans, beast-ops | | ||
| | **`Coldaine/ugv_ws`** | `D:\_projects\ugv_ws` (+ `.worktrees\ugv_ws-*`) | ROS 2 Humble robot brain; runs at `~/beast/ugv_ws` on the Jetson | | ||
|
|
||
| Hangar **never** deploys to the Jetson. Cross-repo map: | ||
| [`docs/beast-control-topology.md`](docs/beast-control-topology.md). Live robot HEAD / | ||
| boot args: [`docs/beast-ops.md`](docs/beast-ops.md) Quick connect only. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep repository-location guidance canonical in README.md.
This section duplicates the repository paths, roles, and topology guidance in README.md Lines 31-41. Keep the BEAST-specific process warning here, but replace the duplicated table with a link to README.md and docs/beast-control-topology.md.
As per coding guidelines, README.md owns repository structure and location guidance.
🤖 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 `@AGENTS.md` around lines 21 - 30, Update the “Two repos for BEAST-01” section
in AGENTS.md to retain only the BEAST-specific process warning and replace its
duplicated repository table and topology details with links to README.md and
docs/beast-control-topology.md. Keep repository paths, roles, and location
guidance owned by README.md.
Source: Coding guidelines
| Write-Host '---' | ||
| Write-Host "TRY $h" | ||
| try { | ||
| ssh -i $key -o BatchMode=yes -o ConnectTimeout=10 -o StrictHostKeyChecking=no beast@$h hostname | Write-Host |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if rg -n --glob '*.ps1' 'StrictHostKeyChecking=no' .; then
echo "Disabled SSH host-key verification remains." >&2
exit 1
fi
rg -n -C 2 'StrictHostKeyChecking|known_hosts' \
beast_probe.ps1 beast_status.ps1 || trueRepository: Coldaine/RobotOverview
Length of output: 801
Keep SSH host-key verification enabled in both scripts.
StrictHostKeyChecking=no accepts changed robot host keys, which can make these scripts connect to a man-in-the-middle endpoint. Use valid known_hosts entries with StrictHostKeyChecking=yes.
beast_probe.ps1::L8beast_status.ps1::L5beast_status.ps1::L11
📍 Affects 2 files
beast_probe.ps1#L8-L8(this comment)beast_status.ps1#L5-L5beast_status.ps1#L11-L11
🤖 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 `@beast_probe.ps1` at line 8, Replace StrictHostKeyChecking=no with
StrictHostKeyChecking=yes in the SSH commands at beast_probe.ps1 lines 8,
beast_status.ps1 lines 5 and 11, and ensure valid known_hosts entries are
available for the robot hosts.
Source: MCP tools
Comment on lines
+59
to
+63
| <motion.section | ||
| className="panel border-rim bg-panel/85 grid grid-cols-1 gap-4 sm:grid-cols-2 md:grid-cols-5 p-4 items-stretch shadow-md relative overflow-hidden" | ||
| aria-label="Safety strip" | ||
| animate={estopEngaged ? { borderColor: ["#404040", "#ef4444", "#404040"] } : {}} | ||
| transition={estopEngaged ? { repeat: Infinity, duration: 1.5 } : {}} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
The border colour does not reset when the E-STOP clears.
When estopEngaged becomes false, animate becomes {}. Framer Motion has no target to animate toward, so the inline borderColor written by the previous keyframe animation remains. The panel keeps whatever red or grey value the loop stopped on instead of returning to the border-rim class colour. Animate back to the resting colour explicitly.
🎨 Proposed fix
- animate={estopEngaged ? { borderColor: ["`#404040`", "`#ef4444`", "`#404040`"] } : {}}
- transition={estopEngaged ? { repeat: Infinity, duration: 1.5 } : {}}
+ animate={{ borderColor: estopEngaged ? ['`#404040`', '`#ef4444`', '`#404040`'] : '`#404040`' }}
+ transition={estopEngaged ? { repeat: Infinity, duration: 1.5 } : { duration: 0.2 }}Confirm that #404040 matches the border-rim token before you apply this.
🤖 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/cockpit/SafetyStrip.tsx` around lines 59 - 63, Update the
motion.section animation in SafetyStrip so clearing estopEngaged explicitly
animates borderColor back to the resting `#404040` value instead of using an empty
target. First confirm `#404040` matches the border-rim token, then preserve the
existing pulsing keyframes and timing while ensuring the inline colour resets
when E-STOP is disengaged.
Comment on lines
+1151
to
+1161
| callService(serviceName: string, args: unknown) { | ||
| if (!socket || socket.readyState !== WebSocket.OPEN) return; | ||
| const callId = `call_${Math.random().toString(36).slice(2, 11)}`; | ||
| const triggerMsg = JSON.stringify({ | ||
| op: 'call_service', | ||
| service: serviceName, | ||
| args, | ||
| id: callId, | ||
| }); | ||
| socket.send(triggerMsg); | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
callService gives the caller no way to detect failure.
The method returns undefined and silently does nothing when the socket is closed. The sibling publish at line 1140 returns a boolean, and CommandRail relies on that boolean to raise the "command did not leave the browser" fault. A service call that vanishes has no such signal. The rosbridge service_response frame for callId is also never consumed, so the result and any error string are dropped, and no timeout exists.
Return a boolean at minimum. If callers need the result, correlate callId with the inbound service_response and add a timeout.
♻️ Proposed minimum change
- callService(serviceName: string, args: unknown) {
- if (!socket || socket.readyState !== WebSocket.OPEN) return;
- const callId = `call_${Math.random().toString(36).slice(2, 11)}`;
- const triggerMsg = JSON.stringify({
- op: 'call_service',
- service: serviceName,
- args,
- id: callId,
- });
- socket.send(triggerMsg);
- },
+ callService(serviceName: string, args: unknown): boolean {
+ if (!socket || socket.readyState !== WebSocket.OPEN) return false;
+ socket.send(JSON.stringify({
+ op: 'call_service',
+ service: serviceName,
+ args,
+ id: opId('call', serviceName),
+ }));
+ return true;
+ },Using opId also keeps the id scheme consistent with publish; Math.random().toString(36).slice(2, 11) can collide and does not identify the service.
🤖 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/lib/ros/client.ts` around lines 1151 - 1161, Update callService to return
a boolean like publish: return false when the socket is unavailable and return
true only after the service request is successfully sent, so callers can detect
whether it left the browser. Preserve the existing request payload while
replacing the random call ID generation with the established opId scheme used by
publish, including service identity as required.
Comment on lines
1176
to
1185
| setEstopLock(engaged: boolean): boolean { | ||
| if (typeof window === 'undefined') return false; | ||
| const live = !!socket && socket.readyState === WebSocket.OPEN; | ||
|
|
||
| if (engaged) { | ||
| // Another tab owns the lock; two heartbeats fighting is the hazard this | ||
| // guards against. Only ENGAGE is gated on being the writer — a release | ||
| // must always be able to drop this tab's own intent. | ||
| if (!getEstopState().writer) return false; | ||
| // No socket means no way to reach the mux. Refusing here is what keeps | ||
| // the UI honest: we never latch a state we could not transmit. | ||
| if (!live) return false; | ||
| const armed = startEstopHeartbeat(); | ||
| if (!armed) return false; | ||
| operatorEngaged = true; | ||
| setEstopState({ engaged: true, engagedAt: getEstopState().engagedAt ?? Date.now() }); | ||
| operatorEngaged = engaged; | ||
| setEstopState({ engaged, engagedAt: engaged ? Date.now() : null, writer: true }); | ||
| if (socket && socket.readyState === WebSocket.OPEN) { | ||
| publishEstopLock(engaged); | ||
| return true; | ||
| } | ||
|
|
||
| // A refused RELEASE must still drop local intent, or the next reconnect | ||
| // would re-assert a lock the operator already cleared. | ||
| operatorEngaged = false; | ||
| if (!live) { | ||
| stopEstopTimers(); | ||
| setEstopState({ engaged: false, engagedAt: null }); | ||
| return false; | ||
| } | ||
| stopEstopHeartbeat(); | ||
| const sent = publishEstopLock(false); | ||
| estopReleaseSends = sent ? 1 : 0; | ||
| if (!estopReleaseTimer) { | ||
| estopReleaseTimer = setInterval(() => { | ||
| if (estopReleaseSends >= ESTOP_RELEASE_SENDS || !publishEstopLock(false)) { | ||
| stopEstopRelease(); | ||
| return; | ||
| } | ||
| estopReleaseSends += 1; | ||
| }, ESTOP_RELEASE_INTERVAL_MS); | ||
| } | ||
| setEstopState({ engaged: false, releasing: true, engagedAt: null }); | ||
| return sent; | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
The e-stop store is mutated before the frame is sent, and the new test locks that behaviour in. setEstopLock writes engaged into the store, then checks the socket and returns false when nothing was sent. The UI reads the store, so a failed E-STOP still renders as an engaged software lock.
src/lib/ros/client.ts#L1176-L1185: movesetEstopStateafter a successfulpublishEstopLock, and returnfalsewithout changing the store when the socket is closed.src/__tests__/estop-election.test.ts#L10-L15: assert thatsetEstopLock(true)returnsfalseand leavesengagedasfalsewhen no socket is open, and add a separate case with an open mock socket for the success path.
📍 Affects 2 files
src/lib/ros/client.ts#L1176-L1185(this comment)src/__tests__/estop-election.test.ts#L10-L15
🤖 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/lib/ros/client.ts` around lines 1176 - 1185, Update src/lib/ros/client.ts
lines 1176-1185 in setEstopLock so the socket-open check and successful
publishEstopLock occur before setEstopState; return false without mutating the
store when no frame is sent, while preserving the successful state update.
Update src/__tests__/estop-election.test.ts lines 10-15 to verify the
closed-socket failure leaves engaged false, and add a separate open-mock-socket
case covering the successful path.
Comment on lines
+359
to
+367
| private scheduleReconnect(): void { | ||
| if (this.disposed || !this.url || this.intentionalClose) return; | ||
| this.clearReconnect(); | ||
| this.reconnectTimer = this.setTimeoutFn(() => { | ||
| this.reconnectTimer = null; | ||
| if (this.disposed || !this.url) return; | ||
| void this.ensureRoslib().then(() => this.openSocket()); | ||
| }, this.reconnectMs) as ReturnType<typeof setTimeout>; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add backoff to the reconnect loop.
scheduleReconnect always waits reconnectMs (default 2000 ms). When BEAST-01 is powered off, the server retries every 2 seconds for the process lifetime. Use exponential backoff with a cap, and reset the delay after a successful connection event.
🤖 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/beast/ros-singleton.ts` around lines 359 - 367, Update
scheduleReconnect to use an exponentially increasing reconnect delay with a
defined maximum cap instead of always using reconnectMs. Track the current
backoff across failed attempts, and reset it to the initial reconnectMs when the
successful connection event is handled. Preserve the existing disposal,
intentional-close, timer cleanup, and socket-opening guards.
Comment on lines
+81
to
+85
| export function toolNeedsApproval(toolName: string, tools: ToolSet = createAgentTools(noopBridge())): boolean { | ||
| const t = tools[toolName]; | ||
| if (!t || !('needsApproval' in t)) return false; | ||
| return t.needsApproval === true; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Avoid building a full tool set in the default parameter.
toolNeedsApproval builds a complete tool set through createAgentTools(noopBridge()) on every call that omits tools. The function then reads one metadata flag. Derive the answer from MOTION_TOOL_NAMES when no tool set is supplied.
♻️ Proposed refactor
-export function toolNeedsApproval(toolName: string, tools: ToolSet = createAgentTools(noopBridge())): boolean {
+export function toolNeedsApproval(toolName: string, tools?: ToolSet): boolean {
+ if (!tools) {
+ return (MOTION_TOOL_NAMES as readonly string[]).includes(toolName);
+ }
const t = tools[toolName];
if (!t || !('needsApproval' in t)) return false;
return t.needsApproval === true;
}Note: noopBridge then becomes unused. Remove it if no test imports it.
📝 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
| export function toolNeedsApproval(toolName: string, tools: ToolSet = createAgentTools(noopBridge())): boolean { | |
| const t = tools[toolName]; | |
| if (!t || !('needsApproval' in t)) return false; | |
| return t.needsApproval === true; | |
| } | |
| export function toolNeedsApproval(toolName: string, tools?: ToolSet): boolean { | |
| if (!tools) { | |
| return (MOTION_TOOL_NAMES as readonly string[]).includes(toolName); | |
| } | |
| const t = tools[toolName]; | |
| if (!t || !('needsApproval' in t)) return false; | |
| return t.needsApproval === true; | |
| } |
🤖 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/beast/tools.ts` around lines 81 - 85, Update toolNeedsApproval to
avoid eagerly constructing createAgentTools(noopBridge()) when tools is omitted;
derive approval from MOTION_TOOL_NAMES in that case, while preserving
metadata-based checks for an explicitly provided ToolSet. Remove noopBridge if
it becomes unused and is not imported by tests.
| "kind": "plan", | ||
| "href": "/datacore/briefing/wiring-model-completion", | ||
| "repoPath": "docs/plans/2026-07-30-wiring-model-completion.md" | ||
| "repoPath": "docs/plans/archived/2026-07-30-wiring-model-completion.md" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize all corpus path declarations after the archive move.
The manifest and registry now use the archived path, but the runtime corpus still uses the old path.
db/hangar/research-corpus-manifest.json#L25-L25: Keepsrc/data/datacore-corpus.tson the archived path.db/hangar/research-corpus-registry.ts#L99-L99: Add or update parity coverage for the manifest, registry, and runtime corpus paths.
📍 Affects 2 files
db/hangar/research-corpus-manifest.json#L25-L25(this comment)db/hangar/research-corpus-registry.ts#L99-L99
🤖 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 `@db/hangar/research-corpus-manifest.json` at line 25, Synchronize the corpus
path declarations: in db/hangar/research-corpus-manifest.json at lines 25-25,
keep src/data/datacore-corpus.ts mapped to the archived path; in
db/hangar/research-corpus-registry.ts at lines 99-99, add or update parity
coverage validating the manifest, registry, and runtime corpus paths.
| - Nothing in `keyArtifactstosort/` may be deleted — see `keyArtifactstosort/agents.md`. Copy and | ||
| extract freely. | ||
| $b_wiring_model_completion$,'docs/plans/2026-07-30-wiring-model-completion.md'); | ||
| $b_wiring_model_completion$,'docs/plans/archived/2026-07-30-wiring-model-completion.md'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the seeded briefing content consistent with the archived path.
This line changes repo_path to docs/plans/archived/2026-07-30-wiring-model-completion.md, but the same seed still declares Status: ACTIVE at Line [1797]. It also links to the old active path at Line [2102]. A seed reload will expose an archived record with stale status and a broken link.
Update the embedded body_markdown references, or generate the briefing content from the archived document before changing repo_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 `@db/hangar/seed.sql` at line 2005, Update the seeded briefing content
associated with $b_wiring_model_completion$ so its embedded body_markdown
references use the archived document consistently: change the stale ACTIVE
status and old active-path link to match
docs/plans/archived/2026-07-30-wiring-model-completion.md, while preserving the
updated repo_path.
| humans; `ugv_bringup` can refuse them all. | ||
|
|
||
| Default remains **disarmed** until the Set 1 crawl+kill re-gate passes. Live | ||
| `allow_motion` / publisher counts: [beast-ops Quick connect](beast-ops.md#quick-connect-verified-2026-08-02). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the Quick connect anchor.
The link uses #quick-connect-verified-2026-08-02. The heading in docs/beast-ops.md includes Wave 3 session close, so the generated anchor is #quick-connect-verified-2026-08-02-wave-3-session-close. The current link does not resolve.
Proposed fix
-Live `allow_motion` / publisher counts: [beast-ops Quick connect](beast-ops.md#quick-connect-verified-2026-08-02).
+Live `allow_motion` / publisher counts: [beast-ops Quick connect](beast-ops.md#quick-connect-verified-2026-08-02-wave-3-session-close).📝 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
| `allow_motion` / publisher counts: [beast-ops Quick connect](beast-ops.md#quick-connect-verified-2026-08-02). | |
| `allow_motion` / publisher counts: [beast-ops Quick connect](beast-ops.md#quick-connect-verified-2026-08-02-wave-3-session-close). |
🤖 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 `@docs/beast-control-topology.md` at line 105, Update the Quick connect link in
the allow_motion/publisher counts reference to use the heading’s complete
generated anchor, including the Wave 3 session close suffix, so the link
resolves correctly.
|
|
||
| What the CAD is *for* (mounting holes, mast planning, URDF, twin geometry) is tracked as work in | ||
| [`docs/plans/2026-07-30-wiring-model-completion.md`](./plans/2026-07-30-wiring-model-completion.md). | ||
| [`docs/plans/archived/2026-07-30-wiring-model-completion.md`](./plans/archived/2026-07-30-wiring-model-completion.md). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the link to the temporary archive file.
Line 97 links the Hardware Library to a plan that will be deleted after extraction. The link will become stale. Move the stable CAD-purpose reference into the owner document or link to the replacement live work order before deleting the archived plan.
🤖 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 `@docs/hardware-library.md` at line 97, Remove the temporary archived-plan link
from the Hardware Library documentation and replace it with the stable
CAD-purpose reference in the owner document or the replacement live work order,
preserving useful navigation without referencing the deletable archive file.
|
|
||
| | File | What still matters in it | Extraction target | | ||
| | --- | --- | --- | | ||
| | [2026-07-31-beast-command-deck-spec.md](2026-07-31-beast-command-deck-spec.md) | Approved cockpit contract: closed topic globs, loopback bridge + Tailscale Serve WSS, safety model (capability vs permission), visual language, sensor-spine verdict | Master plan Set 1b subplan + the future `docs/beast-control-topology.md` (PR-0); then delete | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Refresh the topology extraction status.
The supplied PR stack includes docs/beast-control-topology.md, but Line 14 still labels it as future work in PR-0. Update this row if the document landed in this PR. Mark the extraction complete or identify the exact remaining content before deleting the archive.
🤖 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 `@docs/plans/archived/README.md` at line 14, Update the archived README row for
2026-07-31-beast-command-deck-spec.md to reflect the current status of
docs/beast-control-topology.md: mark the topology extraction complete if the
document landed, or replace the future-work note with the exact remaining
content required before deletion.
Comment on lines
+56
to
+58
| const bridge = getBeastRosClient(); | ||
| // Kick a connect attempt when the bridge URL exists; no-op when unset. | ||
| void bridge.start(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Attach a rejection handler to the fire-and-forget start() call.
bridge.start() awaits ensureRoslib(), which performs await import('roslib'). If that import fails, the promise rejects. void does not handle the rejection, so Node reports an unhandled rejection and can terminate the process under the default --unhandled-rejections=throw behavior.
🛠️ Proposed fix
- // Kick a connect attempt when the bridge URL exists; no-op when unset.
- void bridge.start();
+ // Kick a connect attempt when the bridge URL exists; no-op when unset.
+ // Failures are surfaced later through get_status; never reject here.
+ void bridge.start().catch(() => {});📝 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 bridge = getBeastRosClient(); | |
| // Kick a connect attempt when the bridge URL exists; no-op when unset. | |
| void bridge.start(); | |
| const bridge = getBeastRosClient(); | |
| // Kick a connect attempt when the bridge URL exists; no-op when unset. | |
| // Failures are surfaced later through get_status; never reject here. | |
| void bridge.start().catch(() => {}); |
🤖 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/api/agent/chat/route.ts` around lines 56 - 58, Update the
fire-and-forget call in the route’s bridge startup flow to attach a rejection
handler to bridge.start(), preventing failures from the awaited roslib import
from becoming unhandled promise rejections while preserving the existing
non-blocking behavior.
Comment on lines
15
to
+19
| { | ||
| tone: 'amber', | ||
| text: 'E-STOP CONFIRMATION UNAVAILABLE', | ||
| text: 'E-STOP = DIRECT MANUAL LATCH', | ||
| title: | ||
| 'The stop is asserted at 2 Hz, but the robot cannot echo it back until /cockpit/status ships. The button stays in ASSERTING.', | ||
| 'Software E-STOP provides direct latching for teleop sessions.', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State the limit of the software E-STOP in the chip title.
The new title says the software E-STOP "provides direct latching for teleop sessions". It does not tell the operator that this latch is software-only and depends on the rosbridge link. The honesty rail exists to disclose limits, and this wording removes that disclosure on a safety control. Restore the caveat.
🛠️ Proposed wording
title:
- 'Software E-STOP provides direct latching for teleop sessions.',
+ 'Software E-STOP latches the teleop session over rosbridge. It is not a hardware power cut; use the physical switch if the link drops.',This also keeps the rail informative rather than flattened, as required for rendered UI surfaces.
📝 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
| { | |
| tone: 'amber', | |
| text: 'E-STOP CONFIRMATION UNAVAILABLE', | |
| text: 'E-STOP = DIRECT MANUAL LATCH', | |
| title: | |
| 'The stop is asserted at 2 Hz, but the robot cannot echo it back until /cockpit/status ships. The button stays in ASSERTING.', | |
| 'Software E-STOP provides direct latching for teleop sessions.', | |
| { | |
| tone: 'amber', | |
| text: 'E-STOP = DIRECT MANUAL LATCH', | |
| title: | |
| 'Software E-STOP latches the teleop session over rosbridge. It is not a hardware power cut; use the physical switch if the link drops.', |
🤖 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/cockpit/HonestyRail.tsx` around lines 15 - 19, Update the
E-STOP chip title in HonestyRail to explicitly state that the latch is
software-only and depends on the rosbridge link, while preserving the existing
teleoperation context and rendered chip structure.
Source: Coding guidelines
Comment on lines
+145
to
+155
| private onClose = () => { | ||
| this.teardownTopics(); | ||
| if (this.intentionalClose || this.disposed || !this.url) { | ||
| if (!this.disposed && this.url && !this.intentionalClose) { | ||
| this.state = 'disconnected'; | ||
| } | ||
| return; | ||
| } | ||
| this.state = 'disconnected'; | ||
| this.scheduleReconnect(); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove the unreachable branch in onClose.
The inner condition at Line 148 requires !this.disposed && this.url && !this.intentionalClose. The outer condition at Line 147 is only true when at least one of those is false. The inner assignment can never run. Delete it to make the state transitions explicit.
♻️ Proposed simplification
private onClose = () => {
this.teardownTopics();
- if (this.intentionalClose || this.disposed || !this.url) {
- if (!this.disposed && this.url && !this.intentionalClose) {
- this.state = 'disconnected';
- }
- return;
- }
+ if (this.intentionalClose || this.disposed || !this.url) return;
this.state = 'disconnected';
this.scheduleReconnect();
};📝 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
| private onClose = () => { | |
| this.teardownTopics(); | |
| if (this.intentionalClose || this.disposed || !this.url) { | |
| if (!this.disposed && this.url && !this.intentionalClose) { | |
| this.state = 'disconnected'; | |
| } | |
| return; | |
| } | |
| this.state = 'disconnected'; | |
| this.scheduleReconnect(); | |
| }; | |
| private onClose = () => { | |
| this.teardownTopics(); | |
| if (this.intentionalClose || this.disposed || !this.url) return; | |
| this.state = 'disconnected'; | |
| this.scheduleReconnect(); | |
| }; |
🤖 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/beast/ros-singleton.ts` around lines 145 - 155, Remove the
unreachable inner condition and its state assignment from the onClose method.
Keep the outer early-return guard for intentionalClose, disposed, or missing
url, and preserve the disconnected state update and scheduleReconnect flow for
active unexpected closures.
Comment on lines
+186
to
+198
| async start(): Promise<void> { | ||
| if (this.disposed) return; | ||
| if (!this.url) { | ||
| this.state = 'unconfigured'; | ||
| return; | ||
| } | ||
| if (this.ros?.isConnected) { | ||
| this.state = 'connected'; | ||
| return; | ||
| } | ||
| await this.ensureRoslib(); | ||
| this.openSocket(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard start() against restarting an in-flight connection.
start() only returns early when this.ros?.isConnected is true. When the state is connecting, openSocket() runs again, closes the pending socket, clears the reconnect timer, and creates a new Ros. getStatus() calls start() on every invocation while not connected (Line 221), and each agent tool call runs getStatus() first. Repeated calls can therefore restart the handshake before it completes and prevent the bridge from ever reaching connected.
Skip the reopen while the state is connecting or while a reconnect timer is pending.
🛠️ Proposed fix
if (this.ros?.isConnected) {
this.state = 'connected';
return;
}
+ // A handshake or a scheduled reconnect is already in flight; do not restart it.
+ if (this.state === 'connecting' || this.reconnectTimer !== null) return;
await this.ensureRoslib();
this.openSocket();📝 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 start(): Promise<void> { | |
| if (this.disposed) return; | |
| if (!this.url) { | |
| this.state = 'unconfigured'; | |
| return; | |
| } | |
| if (this.ros?.isConnected) { | |
| this.state = 'connected'; | |
| return; | |
| } | |
| await this.ensureRoslib(); | |
| this.openSocket(); | |
| } | |
| async start(): Promise<void> { | |
| if (this.disposed) return; | |
| if (!this.url) { | |
| this.state = 'unconfigured'; | |
| return; | |
| } | |
| if (this.ros?.isConnected) { | |
| this.state = 'connected'; | |
| return; | |
| } | |
| // A handshake or a scheduled reconnect is already in flight; do not restart it. | |
| if (this.state === 'connecting' || this.reconnectTimer !== null) return; | |
| await this.ensureRoslib(); | |
| this.openSocket(); | |
| } |
🤖 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/beast/ros-singleton.ts` around lines 186 - 198, Update start() in
the connection lifecycle to return without reopening when this.state is
'connecting' or a reconnect timer is pending, in addition to the existing
connected guard. Preserve the current unconfigured and connected handling, and
ensure openSocket() only runs when no connection attempt or scheduled reconnect
is active.
Comment on lines
+270
to
+276
| async backUp(input: BackUpInput): Promise<MotionToolResult> { | ||
| return this.runAction('backup', { | ||
| target: input.meters, | ||
| speed: input.speed ?? BEAST_MAX_SPEED_MPS, | ||
| time_allowance: durationFromSeconds(Math.max(10, input.meters / 0.05 + 5)), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
nav2_msgs action BackUp.action target geometry_msgs Point fields
💡 Result:
The nav2_msgs/action/BackUp action uses the geometry_msgs/Point message type for its target field [1][2]. Because geometry_msgs/Point is a standard ROS 2 message type, it contains the following three float64 fields [3][4]: x: The x-coordinate of the point. y: The y-coordinate of the point. z: The z-coordinate of the point. In the context of the BackUp action, this field typically represents the target position or displacement vector for the robot's backward movement [3][5][6]. Users set these x, y, and z values to define the desired movement goal before sending the action request [3][4][6].
Citations:
- 1: https://docs.ros.org/en/jazzy/p/nav2_msgs/action/BackUp.html
- 2: https://github.com/ros-navigation/navigation2/blob/main/nav2_msgs/action/BackUp.action
- 3: https://api.nav2.org/actions/humble/backup.html
- 4: https://api.nav2.org/actions/jazzy/backup.html
- 5: https://api.nav2.org/actions/kilted/backup.html
- 6: https://api.nav2.org/actions/rolling/backup.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching ros-singleton/action definitions =="
git ls-files | rg 'ros-singleton|action|back.?up|backup|Beast|BEAST' || true
echo
echo "== targeted file excerpt =="
if [ -f src/server/beast/ros-singleton.ts ]; then
sed -n '220,290p' src/server/beast/ros-singleton.ts | nl -ba -v220
else
echo "src/server/beast/ros-singleton.ts not found"
fi
echo
echo "== runAction / action definitions =="
rg -n "runAction|action.*backup|backup.*action|driveOnHeading|BackUp|DriveOnHeading" src/server/beast -S || true
echo
echo "== package files for typescript deps =="
for f in package.json src/server/beast/*.ts; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
done 2>/dev/null || trueRepository: Coldaine/RobotOverview
Length of output: 1736
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/server/beast/ros-singleton.ts excerpt =="
sed -n '220,290p' src/server/beast/ros-singleton.ts > /tmp/ros_singleton_excerpt.txt 2>/dev/null || true
awk '{ printf "%5d\t%s\n", NR+220-1, $0 }' /tmp/ros_singleton_excerpt.txt
echo
echo "== runs/action/backup occurrences in src/server/beast =="
rg -n "runAction|action.*backup|backup.*action|driveOnHeading|BackUp|DriveOnHeading|target:" src/server/beast src/__tests__/ros-singleton.test.ts -S || true
echo
echo "== TypeScript package/imports around rosbag/rosbridge-ish support =="
rg -n "rosbridge|roslib|rcl|action|geometry_msgs|backup" package.json src/server/beast/src src/server/beast src/__tests__ -S || trueRepository: Coldaine/RobotOverview
Length of output: 18289
Send backUp.target as a Point, not a scalar.
nav2_msgs/action/BackUp.target is geometry_msgs/Point, which has x, y, and z fields. Passing input.meters as the goal target field does not match the ROS 2 action message shape used by driveOnHeading.
🛠️ Proposed fix
return this.run.Action('backup', {
- target: input.meters,
+ target: { x: input.meters, y: 0, z: 0 },
speed: input.speed ?? BEAST_MAX_SPEED_MPS,🤖 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/beast/ros-singleton.ts` around lines 270 - 276, Update the backUp
method’s runAction('backup') goal so target is a geometry_msgs/Point object with
the requested backup distance on the appropriate axis, matching the shape used
by driveOnHeading; preserve the existing speed and time_allowance calculations.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 56
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/cockpit/OpticsWall.tsx (1)
65-74: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBound-test the new clearance thresholds.
Add coverage for the new critical and warning boundaries at
0.159m,0.160m, and0.280m.🤖 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/cockpit/OpticsWall.tsx` around lines 65 - 74, Update the tests for the clearanceStatus logic in OpticsWall to cover values of 0.159m, 0.160m, and 0.280m, asserting that 0.159m is CRITICAL while 0.160m and 0.280m are not classified as CRITICAL or WARNING according to the existing strict threshold comparisons.src/lib/ros/client.ts (1)
1346-1354: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe
powerbranch overwrites metrics parsed fromsystem_metrics.Lines 1351-1354 assign
wifiRssi,diskFree,cpuTempandgpuTempunconditionally. Apowerdiagnostic that carries onlychargingtherefore resets all four fields tonull, including values that asystem_metricsentry set earlier in the samediagArray.forEachpass. Lines 1347-1349 already use the!== undefinedguard pattern; apply it to the remaining fields.🐛 Proposed fix
} else if (d.name === 'system_metrics' || d.name === 'power') { if (values.charging !== undefined) next.isCharging = safeBool(values.charging); if (values.ethernet !== undefined || values.ethernet_connected !== undefined) { next.isEthernetConnected = safeBool(values.ethernet_connected ?? values.ethernet); } - next.wifiRssi = safeNumber(values.wifi_rssi); - next.diskFree = values.disk_free || null; - next.cpuTemp = safeNumber(values.cpu_temp); - next.gpuTemp = safeNumber(values.gpu_temp); + if (values.wifi_rssi !== undefined) next.wifiRssi = safeNumber(values.wifi_rssi); + if (values.disk_free !== undefined) next.diskFree = values.disk_free || null; + if (values.cpu_temp !== undefined) next.cpuTemp = safeNumber(values.cpu_temp); + if (values.gpu_temp !== undefined) next.gpuTemp = safeNumber(values.gpu_temp); }🤖 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/lib/ros/client.ts` around lines 1346 - 1354, Update the power/system_metrics handling in the diagnostic iteration to assign wifiRssi, diskFree, cpuTemp, and gpuTemp only when their corresponding values are not undefined, preserving previously parsed system_metrics values when a power diagnostic omits them. Follow the existing guarded assignment pattern used for charging and Ethernet fields.
🤖 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 @.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md:
- Around line 138-140: Update the Wi-Fi reachability statement in the plan to
label the claim as historical and include the actual last-verified date, August
2, 2026. Preserve the existing unreachable details and LAN dependency for
robot-side phases.
- Around line 98-99: Update the example launch command to pass a declared
use_localplan value supported by nav.launch.py, such as dwa or teb, instead of
rpp; only retain rpp if nav.launch.py is updated to declare and wire it through.
In `@AGENTS.md`:
- Around line 21-30: Update the “Two repos for BEAST-01” section in AGENTS.md to
retain only the BEAST-specific process warning and replace its duplicated
repository table and topology details with links to README.md and
docs/beast-control-topology.md. Keep repository paths, roles, and location
guidance owned by README.md.
- Around line 13-19: Update the “Always commit and open a PR” guidance to
explicitly allow skipping the requirement when the user says not to commit, not
to push, or not to open a PR. Keep the existing cross-repository PR instructions
and reference to the enforcement rule unchanged.
In `@beast_probe.ps1`:
- Line 8: Replace StrictHostKeyChecking=no with StrictHostKeyChecking=yes in the
SSH commands at beast_probe.ps1 lines 8, beast_status.ps1 lines 5 and 11, and
ensure valid known_hosts entries are available for the robot hosts.
- Around line 8-12: Check $LASTEXITCODE immediately after each ssh invocation in
beast_probe.ps1 lines 8-12 and beast_status.ps1 lines 4-13, and treat any
non-zero value as failure so the corresponding catch/failure reporting runs
instead of printing SUCCESS or omitting SERVICE CHECK FAILED/PARAM CHECK FAILED.
Preserve the existing success output only for zero exit codes.
In `@beast_status.ps1`:
- Line 11: Update the SSH command in beast_status.ps1 to target the current
BEAST-01 SSH host and query the live, re-probed ROS 2 node name for
allow_motion, using the node name documented in docs/beast-ops.md (/ugv_bringup)
if confirmed. Replace the existing /bringup argument while preserving the ROS
environment setup and output behavior.
- Around line 1-5: Update beast_status.ps1 lines 1-13 to use the Quick connect
source via ssh beast-01 instead of the hard-coded key, IP address, and SSH
options; keep direct fallbacks documented in the Quick connect block. For
beast_probe.ps1 lines 1-3, either retain mDNS/Tailscale and direct fallback
discovery only if intentional, or derive its targets from the same Quick connect
source.
In `@db/hangar/research-corpus-manifest.json`:
- Line 25: Synchronize the corpus path declarations: in
db/hangar/research-corpus-manifest.json at lines 25-25, keep
src/data/datacore-corpus.ts mapped to the archived path; in
db/hangar/research-corpus-registry.ts at lines 99-99, add or update parity
coverage validating the manifest, registry, and runtime corpus paths.
In `@db/hangar/seed.sql`:
- Line 2005: Update the seeded briefing content associated with
$b_wiring_model_completion$ so its embedded body_markdown references use the
archived document consistently: change the stale ACTIVE status and old
active-path link to match
docs/plans/archived/2026-07-30-wiring-model-completion.md, while preserving the
updated repo_path.
In `@docs/beast-control-topology.md`:
- Line 105: Update the Quick connect link in the allow_motion/publisher counts
reference to use the heading’s complete generated anchor, including the Wave 3
session close suffix, so the link resolves correctly.
In `@docs/beast-ops.md`:
- Around line 1852-1866: Rewrite the heartbeat test section in the documented
procedure to remove the ESP32 three-second stale-command watchdog and instead
describe the supervised Jetson cmd_vel_timeout crawl-and-kill check. Require
verification of the documented stop result, or explicitly instruct the operator
to wait for motion lock before enabling motion, while preserving the existing
safety constraints and command flow.
- Around line 648-651: Update the WSS transport row in the documentation so it
is not labeled as current; mark it unavailable based on the existing dated
verification, unless live SSH verification confirms and supports updating the
Quick connect block and transport status.
- Around line 1820-1827: Update the bringup_lidar.launch.py command to use the
current live USB serial device path /dev/ttyACM0 instead of the retired
/dev/ttyTHS1 path, keeping the remaining environment setup and launch arguments
unchanged.
In `@docs/hardware-library.md`:
- Line 97: Remove the temporary archived-plan link from the Hardware Library
documentation and replace it with the stable CAD-purpose reference in the owner
document or the replacement live work order, preserving useful navigation
without referencing the deletable archive file.
In `@docs/plans/2026-08-02-beast-agent-architecture.md`:
- Line 217: The safety-status topic contract is inconsistent across the plans.
In docs/plans/2026-08-02-beast-agent-architecture.md:217, declare the canonical
producer topic and message type; in
docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md:44-45, replace
“compatible” wording with the exact wire contract; in
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md:104-105, bind the agent
UI to that canonical topic; and in
docs/plans/2026-08-02-beast-immobile-execution.md:21-23, record the same topic
or explicitly document an adapter.
- Around line 292-295: Standardize the disarmed-goal contract across all cited
documents: locked goals must run without actuation and end in the explicit
ABORTED terminal state. Update docs/plans/2026-08-02-beast-agent-architecture.md
lines 292-295 and 350-357, docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md
lines 63-66, docs/plans/2026-08-02-beast-agent-pr4-agent-command.md lines 76-81,
and docs/plans/2026-08-02-beast-immobile-execution.md line 24 to reflect this
behavior, while keeping only armed Nav2 goals excluded from scope.
- Around line 24-32: The bounded-skill contract must require verified costmap
collision checking before motion is available. In
docs/plans/2026-08-02-beast-agent-architecture.md:24-32, make the costmap path
an explicit prerequisite for bounded skills; in
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md:73-79, remove the
blind-primitives option or mark motion tools unavailable until collision
checking is verified.
- Around line 37-47: Align the rosbridge allowlist documentation across the
master and commissioning plans: in
docs/plans/2026-08-02-beast-agent-architecture.md lines 37-47,
docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md lines 28-35,
docs/plans/2026-08-02-beast-agent-pr4-agent-command.md lines 83-95, and
docs/plans/2026-08-02-beast-immobile-execution.md lines 237-241, replace the
blanket “no services/actions” guidance with the exact topics_glob/services_glob
entries required for the Nav2 goal, cancel, feedback, and result flow. Keep the
idle-session failure rule restrictive so no additional services or actions are
exposed until explicitly needed.
In `@docs/plans/2026-08-02-beast-agent-pr1-safety-spine.md`:
- Around line 58-62: Update the EstopLock release transition in the state
diagram so operator release and re-arm first re-evaluate EthernetLock,
ChargingLock, and any other active interlocks before permitting Armed; route
back into the applicable lock state when a condition remains active rather than
transitioning directly to Armed.
- Around line 42-45: Update the charging interlock and arming flow described
under “Charging” so charging_active telemetry must be present and fresh before
arming is allowed. Treat an absent or stale charging_active topic as a motion
lock rather than fail-open, while preserving the existing default-disarmed guard
and publishing the corresponding lock reason through /cockpit/status-compatible
diagnostics.
In `@docs/plans/2026-08-02-beast-agent-pr2-power-telemetry.md`:
- Around line 36-41: Choose either beast_power or ugv_bringup as the sole
publisher of /ugv/voltage, remove the competing publisher or relay, and document
the ownership decision in the PR-2b plan. Update relevant tests to assert that
exactly one publisher provides the BatteryState topic and prevent competing
charging-state samples.
In `@docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md`:
- Around line 51-56: Align the motion limits across the documented plans: in
docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md lines 51-56, explicitly
apply the ≤0.15 m/s limit to all motion-bearing work; in
.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines 85-87, replace the
0.26 m/s mapping limit or document it as a separately tested teleoperation-only
limit.
- Around line 45-49: The motion-gate references are undefined and must use the
named Set 1 re-gate with dated proof. In
docs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.md lines 45-49, replace the
“Phase 0.5” dependency with the Set 1 crawl-and-kill re-gate and its dated
proof; in .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines 80-84,
require that same gate before setting allow_motion:=true.
In `@docs/plans/2026-08-02-beast-agent-pr5-hygiene.md`:
- Around line 43-46: The Vizanti migration documentation must provide a
replacement launch path before removing ugv_web_app. In
docs/plans/2026-08-02-beast-agent-pr5-hygiene.md lines 43-46, update the
reference sweep to require documenting and validating the replacement before
deletion; in .kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md lines
17-20, document launching Vizanti directly and the required bridge
configuration.
In `@docs/plans/2026-08-02-beast-immobile-execution.md`:
- Around line 33-40: Update the Packaging status table to replace every “(fill
after gh pr create)” entry with its actual GitHub PR URL after confirming the
corresponding work was committed, pushed, and opened. If any repository lacks a
completed PR, mark that work unfinished rather than leaving placeholder records.
- Around line 154-157: Update the Set 1a test plan to remove any instruction to
flip or test allow_motion:=true during a hard-ban session. Keep this session
limited to verifying the locked false gate with motors inert, and move any
armed-path validation to the next-session gate as directed by the surrounding
plan.
In `@docs/plans/archived/2026-07-30-wiring-model-completion.md`:
- Line 3: Update the plan’s Status metadata from ACTIVE to PARKED or ARCHIVED,
and explicitly state that a new work order is required before any wiring phases
can be executed.
In `@docs/plans/archived/README.md`:
- Line 14: Update the archived README row for
2026-07-31-beast-command-deck-spec.md to reflect the current status of
docs/beast-control-topology.md: mark the topology extraction complete if the
document landed, or replace the future-work note with the exact remaining
content required before deletion.
In `@src/__tests__/agent-model.test.ts`:
- Around line 26-31: Extend the test “gates the hangar agent behind
HANGAR_AGENT_ENABLED” to verify that isHangarAgentEnabled() returns true for the
exact value '1' and false for a rejected variant such as 'TRUE', while
preserving the existing empty and 'true' assertions.
In `@src/__tests__/agent-tools.test.ts`:
- Around line 128-133: Add a test alongside the existing gateMotionIntent
coverage in agent-tools.test.ts that passes status values with connection set to
disconnected and error while motion is allowed, and assert each result has
status bridge_unavailable. Keep the unconfigured test unchanged and target the
separate non-connected branch in gateMotionIntent.
In `@src/__tests__/cockpit-client.test.tsx`:
- Line 78: Replace the unconditional unmount disconnect test with E-STOP
coverage: in the CockpitClient unmount test, engage E-STOP before calling
unmount(), then assert the socket remains connected or retained and is not
disconnected. Preserve the existing test setup and use the component’s
established E-STOP and socket/disconnect symbols.
In `@src/__tests__/command-rail.test.tsx`:
- Around line 112-124: Add a test alongside the charging case that sets
mocks.isEthernetConnected to true, renders CommandRail, advances timers, clears
publish calls, and sends a non-repeating “w” keydown. Assert motion is not
published and the “motion locked — Ethernet tether connected” message is
rendered, using the existing mock and test setup.
- Around line 60-67: Update the beforeEach setup in command-rail tests to reset
both mocks.isCharging and mocks.isEthernetConnected to their default values,
preventing state leakage between tests while preserving the existing mock
resets.
In `@src/__tests__/estop-election.test.ts`:
- Around line 10-15: Update the E-STOP tests around setEstopLock to assert its
return value instead of relying only on getEstopState().engaged. Keep the
no-socket case aligned with the intended failure behavior, and add a separate
connected case using a mocked open socket that verifies the published frame and
confirms engaged is true.
In `@src/app/agent/AgentClient.tsx`:
- Around line 282-292: Add a stable, explicit aria-label to the chat input
element in AgentClient, independent of the conditional placeholder text. Keep
the existing value, disabled state, placeholder, and styling behavior unchanged.
In `@src/app/api/agent/chat/route.ts`:
- Around line 56-58: Update the fire-and-forget call in the route’s bridge
startup flow to attach a rejection handler to bridge.start(), preventing
failures from the awaited roslib import from becoming unhandled promise
rejections while preserving the existing non-blocking behavior.
- Around line 45-54: Update the request parsing in the agent chat route to
validate body.messages with the existing Zod UIMessage schema before assigning
or passing it to convertToModelMessages and streamText, returning the
established 400 response for invalid message objects. Also add the same operator
authentication or explicit operator-token check used by other protected routes
before allowing agent tool invocation, while preserving the HANGAR_AGENT_ENABLED
gate.
In `@src/app/bay/`[id]/page.tsx:
- Around line 39-52: Update the Bay Command Panel metrics in the page component
to use a defined data source tied to the selected bay and its current condition
instead of hardcoded “85%” and “1.2kW” values; if no such source exists,
explicitly label both metrics as static design values.
In `@src/app/items/page.tsx`:
- Around line 52-76: The activeFilter state currently changes only button
styling; update the items catalog rendering to apply predicates for each
supported value in mockFilters before mapping entries, while preserving the
existing unfiltered behavior for the default filter. Use the activeFilter and
items symbols in the page’s catalog rendering path, or remove any filter
controls whose values cannot be implemented.
In `@src/app/quartermaster/page.tsx`:
- Around line 106-134: Replace the Tailwind-class parsing used for the pipeline
status dot in the ACQUISITION_PIPELINE_STATUSES map with an explicit color value
sourced from WISHLIST_STATUS_META or a status-to-color mapping. Update the
relevant metadata or mapping and the dot className so watching retains its
intended indicator color without deriving colors from st.cls.
- Around line 157-185: Update the budget display in the upgradePath map near
costConstraint and hypotheticalBudget so groups without a dollar constraint do
not use or display the $250 fallback. Keep the budget optional: show “Budget
unavailable” and omit the progress bar when costConstraint is absent, while
preserving the existing cost, percentage, and over-budget behavior for groups
with a real dollar budget.
In `@src/components/board/Port.tsx`:
- Around line 39-48: Update the ownership inference in Port.tsx to use a shared
wiring-derived mapping for driver-board ports whose host varies by build, rather
than maintaining terminal-ID exceptions locally. Extend the wiring model in
src/data/wiring.ts with the mapping and reuse it when selecting the π or Σ
badge, preserving existing unit and terminal behavior for other ports.
In `@src/components/board/TwinCanvas.tsx`:
- Around line 151-164: Move the legend `<g>` containing the `LEGEND`, `Junction
(⊕)`, and `Ownership (π/Σ)` elements out of the transformed board/world group
and render it as a sibling group at the SVG root level. Preserve its existing
translate position and styling while ensuring pan-and-zoom transforms apply only
to board content.
In `@src/components/cockpit/CommandRail.tsx`:
- Around line 71-84: The cockpit must honor the robot-reported arming state and
represent unknown tether data correctly. In
src/components/cockpit/CommandRail.tsx lines 71-84, add an status.allowMotion
=== false gate to driveGateReason so motion is disabled when the robot reports
disarmed. In src/components/cockpit/SafetyStrip.tsx lines 86-104, render Unknown
when both isCharging and isEthernetConnected are null, and retain an
allowMotion-based indication in the motion-state block instead of always
displaying ARMED.
- Around line 71-84: Update the driveGateReason chain in CommandRail to consult
status.allowMotion and lock motion whenever the robot’s arming flag is not true,
preserving the existing connectivity, E-STOP, charging, Ethernet, and dead-topic
checks. Keep the resulting gate behavior consistent with the agent motion-gate
path and ensure the commanding indicator cannot activate when the robot rejects
motion.
In `@src/components/cockpit/HonestyRail.tsx`:
- Around line 15-19: Update the E-STOP chip title in HonestyRail to explicitly
state that the latch is software-only and depends on the rosbridge link, while
preserving the existing teleoperation context and rendered chip structure.
In `@src/components/cockpit/OpticsWall.tsx`:
- Around line 139-148: Update the animated reticles in OpticsWall, including the
motion.svg blocks gated by rgb.active and depth.active, to respect the user’s
reduced-motion preference. Use Framer Motion’s reduced-motion support or omit
the opacity animation and transition when reduced motion is enabled, while
preserving the existing reticle rendering and animation for other users.
In `@src/components/cockpit/SafetyStrip.tsx`:
- Around line 59-63: Update the motion.section animation in SafetyStrip so
clearing estopEngaged explicitly animates borderColor back to the resting
`#404040` value instead of using an empty target. First confirm `#404040` matches
the border-rim token, then preserve the existing pulsing keyframes and timing
while ensuring the inline colour resets when E-STOP is disengaged.
In `@src/lib/ros/client.ts`:
- Around line 1176-1185: Update src/lib/ros/client.ts lines 1176-1185 in
setEstopLock so the socket-open check and successful publishEstopLock occur
before setEstopState; return false without mutating the store when no frame is
sent, while preserving the successful state update. Update
src/__tests__/estop-election.test.ts lines 10-15 to verify the closed-socket
failure leaves engaged false, and add a separate open-mock-socket case covering
the successful path.
- Around line 1151-1161: Update callService to return a boolean like publish:
return false when the socket is unavailable and return true only after the
service request is successfully sent, so callers can detect whether it left the
browser. Preserve the existing request payload while replacing the random call
ID generation with the established opId scheme used by publish, including
service identity as required.
In `@src/server/beast/ros-singleton.ts`:
- Around line 359-367: Update scheduleReconnect to use an exponentially
increasing reconnect delay with a defined maximum cap instead of always using
reconnectMs. Track the current backoff across failed attempts, and reset it to
the initial reconnectMs when the successful connection event is handled.
Preserve the existing disposal, intentional-close, timer cleanup, and
socket-opening guards.
- Around line 145-155: Remove the unreachable inner condition and its state
assignment from the onClose method. Keep the outer early-return guard for
intentionalClose, disposed, or missing url, and preserve the disconnected state
update and scheduleReconnect flow for active unexpected closures.
- Around line 186-198: Update start() in the connection lifecycle to return
without reopening when this.state is 'connecting' or a reconnect timer is
pending, in addition to the existing connected guard. Preserve the current
unconfigured and connected handling, and ensure openSocket() only runs when no
connection attempt or scheduled reconnect is active.
- Around line 270-276: Update the backUp method’s runAction('backup') goal so
target is a geometry_msgs/Point object with the requested backup distance on the
appropriate axis, matching the shape used by driveOnHeading; preserve the
existing speed and time_allowance calculations.
In `@src/server/beast/tools.ts`:
- Around line 81-85: Update toolNeedsApproval to avoid eagerly constructing
createAgentTools(noopBridge()) when tools is omitted; derive approval from
MOTION_TOOL_NAMES in that case, while preserving metadata-based checks for an
explicitly provided ToolSet. Remove noopBridge if it becomes unused and is not
imported by tests.
---
Outside diff comments:
In `@src/components/cockpit/OpticsWall.tsx`:
- Around line 65-74: Update the tests for the clearanceStatus logic in
OpticsWall to cover values of 0.159m, 0.160m, and 0.280m, asserting that 0.159m
is CRITICAL while 0.160m and 0.280m are not classified as CRITICAL or WARNING
according to the existing strict threshold comparisons.
In `@src/lib/ros/client.ts`:
- Around line 1346-1354: Update the power/system_metrics handling in the
diagnostic iteration to assign wifiRssi, diskFree, cpuTemp, and gpuTemp only
when their corresponding values are not undefined, preserving previously parsed
system_metrics values when a power diagnostic omits them. Follow the existing
guarded assignment pattern used for charging and Ethernet fields.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9795bce-ae25-48ee-ae7d-bd5435c57c8a
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/beast-ups-i2c-wiring.svgis excluded by!**/*.svgups-module-3s-header.pngis excluded by!**/*.png
📒 Files selected for processing (73)
.claude/launch.json.cursor/rules/always-commit-and-pr.mdc.kilo/plans/1785521009354-beast-lidar-slam-nav2-plan.md.playwright-mcp/page-2026-07-31T17-56-40-364Z.yml.playwright-mcp/page-2026-08-02T16-22-03-718Z.ymlAGENTS.mdREADME.md_ppp/worksheet.mdbeast_probe.ps1beast_status.ps1db/hangar/research-corpus-manifest.jsondb/hangar/research-corpus-registry.tsdb/hangar/seed.sqldocs/beast-cockpit-future-roadmap.mddocs/beast-control-topology.mddocs/beast-ops.mddocs/hardware-library.mddocs/plans/2026-07-11-beast-nvme-storage-implementation.mddocs/plans/2026-07-31-beast-command-deck-plan.mddocs/plans/2026-08-01-beast-cockpit-future-roadmap.mddocs/plans/2026-08-02-beast-agent-architecture.mddocs/plans/2026-08-02-beast-agent-pr1-safety-spine.mddocs/plans/2026-08-02-beast-agent-pr2-power-telemetry.mddocs/plans/2026-08-02-beast-agent-pr3-lidar-slam-nav.mddocs/plans/2026-08-02-beast-agent-pr4-agent-command.mddocs/plans/2026-08-02-beast-agent-pr5-hygiene.mddocs/plans/2026-08-02-beast-immobile-execution.mddocs/plans/README.mddocs/plans/archived/2026-07-30-wiring-model-completion.mddocs/plans/archived/2026-07-31-beast-command-deck-spec.mddocs/plans/archived/README.mddocs/plans/beast-command-deck-drafts/README.mddocs/plans/beast-command-deck-drafts/beast-command-deck.htmldocs/plans/beast-command-deck-drafts/cockpit_robot.launch.pydocs/plans/beast-command-deck-drafts/foxglove_bridge.launch.pydocs/plans/beast-command-deck-drafts/teleop_joy_operator.yamldocs/plans/beast-command-deck-drafts/twist_mux.yamlpackage.jsonsrc/__tests__/agent-model.test.tssrc/__tests__/agent-tools.test.tssrc/__tests__/briefings-parity.test.tssrc/__tests__/cockpit-client.test.tsxsrc/__tests__/command-rail.test.tsxsrc/__tests__/estop-election.test.tssrc/__tests__/ros-client.test.tssrc/__tests__/ros-singleton.test.tssrc/app/agent/AgentClient.tsxsrc/app/agent/page.tsxsrc/app/api/agent/chat/route.tssrc/app/bay/[id]/page.tsxsrc/app/cockpit/CockpitClient.tsxsrc/app/items/page.tsxsrc/app/missions/page.tsxsrc/app/page.tsxsrc/app/quartermaster/page.tsxsrc/app/tech-tree/page.tsxsrc/components/Shell.tsxsrc/components/UnitCard.tsxsrc/components/board/Port.tsxsrc/components/board/TwinCanvas.tsxsrc/components/cockpit/CommandRail.tsxsrc/components/cockpit/HonestyRail.tsxsrc/components/cockpit/OpticsWall.tsxsrc/components/cockpit/SafetyStrip.tsxsrc/lib/ros/client.tssrc/lib/ros/estop-store.tssrc/server/beast/model.tssrc/server/beast/motion-gate.tssrc/server/beast/prompts.tssrc/server/beast/ros-singleton.tssrc/server/beast/schemas.tssrc/server/beast/tools.tssrc/server/beast/types.ts
💤 Files with no reviewable changes (10)
- docs/plans/beast-command-deck-drafts/README.md
- docs/plans/beast-command-deck-drafts/teleop_joy_operator.yaml
- docs/plans/2026-07-11-beast-nvme-storage-implementation.md
- docs/plans/beast-command-deck-drafts/foxglove_bridge.launch.py
- docs/plans/2026-07-31-beast-command-deck-plan.md
- docs/plans/beast-command-deck-drafts/beast-command-deck.html
- src/app/cockpit/CockpitClient.tsx
- docs/plans/beast-command-deck-drafts/cockpit_robot.launch.py
- docs/plans/2026-08-01-beast-cockpit-future-roadmap.md
- docs/plans/beast-command-deck-drafts/twist_mux.yaml
🛑 Comments failed to post (4)
docs/beast-ops.md (3)
648-651: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail ssh beast-01 ' systemctl is-enabled beast-cockpit.service || true systemctl is-active beast-cockpit.service || true tailscale serve status || true ss -tlnp 2>/dev/null | grep ":9090" || true 'Repository: Coldaine/RobotOverview
Length of output: 233
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Files:" git ls-files 'docs/beast-ops.md' 'docs/{NORTH_STAR.md,deploy.md,beast-control-topology.md}' echo echo "Relevant docs/beast-ops.md sections:" if [ -f docs/beast-ops.md ]; then wc -l docs/beast-ops.md sed -n '600,670p' docs/beast-ops.md echo rg -n "Quick connect|beast-cockpit|Tailscale Serve|tailscale serve|Current Orin|retired|beast-01\.tyrannosaurus-magellanic|192\.168\.20\.184:5000" docs/beast-ops.md fiRepository: Coldaine/RobotOverview
Length of output: 8438
🏁 Script executed:
#!/bin/bash set -euo pipefail curl -fsSL --max-time 20 "https://raw.githubusercontent.com/Coldaine/RobotOverview/main/docs/beast-ops.md" -o /tmp/docs-beast-ops.md sed -n '600,670p' /tmp/docs-beast-ops.mdRepository: Coldaine/RobotOverview
Length of output: 3441
Do not label the cockpit WSS transport as current.
docs/beast-ops.mdcurrently callswss://beast-01.tyrannosaurus-magellanic.ts.netthe current Orin cockpit transport, but the dated Quick connect block reportsbeast-cockpit.serviceabsent andtailscale servewith no serve config on 2026-08-02. Update this row to show the transport as unavailable, or update the dated Quick connect block after live SSH verification first.🤖 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 `@docs/beast-ops.md` around lines 648 - 651, Update the WSS transport row in the documentation so it is not labeled as current; mark it unavailable based on the existing dated verification, unless live SSH verification confirms and supports updating the Quick connect block and transport status.Source: Coding guidelines
1820-1827: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use the live USB serial path in the bring-up command.
The current robot uses the driver-board USB-C path, mapped to
/dev/ttyACM0. This command still opens/dev/ttyTHS1, which belongs to the retired GPIO-UART plan. The documented bring-up will fail on the current hardware.Proposed fix
ros2 launch ugv_bringup bringup_lidar.launch.py \ - serial_port:=/dev/ttyTHS1 use_lidar:=false use_rviz:=false allow_motion:=false + serial_port:=/dev/serial/by-id/usb-1a86_USB_Single_Serial_5B5E130201-if00 \ + use_lidar:=false use_rviz:=false allow_motion:=falseAs per coding guidelines, current BEAST-01 hardware paths must be verified live before documenting them.
📝 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.🤖 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 `@docs/beast-ops.md` around lines 1820 - 1827, Update the bringup_lidar.launch.py command to use the current live USB serial device path /dev/ttyACM0 instead of the retired /dev/ttyTHS1 path, keeping the remaining environment setup and launch arguments unchanged.Source: Coding guidelines
1852-1866: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== file excerpt ==" sed -n '1820,1885p' docs/beast-ops.md || true echo echo "== heartbeat/cmd_vel_timeout terms ==" rg -n "heartbeat|cmd_vel_timeout|three-second|ESP32|Jetson|allow_motion|motion" docs/beast-ops.md docs -S || trueRepository: Coldaine/RobotOverview
Length of output: 50380
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== candidate runbook/live-robot sections ==" python3 - <<'PY' from pathlib import Path p = Path("docs/beast-ops.md") text = p.read_text() for needle in [ "## LIVE ROBOT", "## LIVE ROBOT: do NOT disable", "Supervised motion shakedown", "Remaining physical cutover record", "craw", ]: hits = [] for i, line in enumerate(text.splitlines(), 1): if needle.lower() in line.lower(): hits.append(i) print(f"{needle!r}: {hits[:12]}") PY echo echo "== quick relevant excerpts ==" sed -n '250,275p;320,330p;695,706p;1880,1895p' docs/beast-ops.md || trueRepository: Coldaine/RobotOverview
Length of output: 5815
Replace the ESP32 stale-command watchdog with the live re-gate.
The ESP32 heartbeat-stop test failed, and the remaining step is the supervised Jetson
cmd_vel_timeoutcrawl+kill check, not a 3-second ESP32 heartbeat. Reword this procedure to test the Jetson watchdog and require the documented stop result or explicitly wait for motion lock before enabling motion.🤖 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 `@docs/beast-ops.md` around lines 1852 - 1866, Rewrite the heartbeat test section in the documented procedure to remove the ESP32 three-second stale-command watchdog and instead describe the supervised Jetson cmd_vel_timeout crawl-and-kill check. Require verification of the documented stop result, or explicitly instruct the operator to wait for motion lock before enabling motion, while preserving the existing safety constraints and command flow.Source: Coding guidelines
docs/plans/archived/2026-07-30-wiring-model-completion.md (1)
3-3: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use an archived status for this file.
Line 3 says
Status: ACTIVE, but the plan indexes classify files inarchived/as not live work orders. This status can cause an agent to execute the wiring phases before extraction. Set the status toPARKEDorARCHIVED, and state that a new work order is required before execution.🤖 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 `@docs/plans/archived/2026-07-30-wiring-model-completion.md` at line 3, Update the plan’s Status metadata from ACTIVE to PARKED or ARCHIVED, and explicitly state that a new work order is required before any wiring phases can be executed.
added 2 commits
August 3, 2026 01:49
…ARM/DISARM rewire, UI extensions
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| text: 'E-STOP = DIRECT MANUAL LATCH', | ||
| title: | ||
| 'The stop is asserted at 2 Hz, but the robot cannot echo it back until /cockpit/status ships. The button stays in ASSERTING.', | ||
| 'Software E-STOP provides direct latching for teleop sessions.', |
There was a problem hiding this comment.
SUGGESTION: The E-STOP chip title does not disclose that the latch is software-only and depends on the rosbridge link.
"Software E-STOP provides direct latching for teleop sessions" reads as a complete description, but it omits the limit: if the rosbridge connection drops, the software latch is the only thing holding the stop and the operator has no hardware power cut to fall back on. Add a caveat such as "…over rosbridge. Not a hardware power cut — use the physical switch if the link drops."
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return; | ||
| } | ||
| rosClient.setEstopLock(!(robotConfirmed || estop.engaged)); | ||
| rosClient.setEstopLock(!estopEngaged); |
There was a problem hiding this comment.
SUGGESTION: handleEstop calls setEstopLock directly without checking whether this tab is the elected writer.
The previous code guarded the toggle with if (!estop.writer) { rosClient.probeEstopWriter(); return; } so that a tab that lost the writer election could not silently override the incumbent. The new code bypasses this check entirely. Restore the writer guard (or call probeEstopWriter when the tab is not the writer) to preserve the single-writer invariant.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (!armed) return false; | ||
| operatorEngaged = true; | ||
| setEstopState({ engaged: true, engagedAt: getEstopState().engagedAt ?? Date.now() }); | ||
| operatorEngaged = engaged; |
There was a problem hiding this comment.
WARNING: setEstopLock bypasses the single-writer election.
The old code checked getEstopState().writer before engaging and used claimEstopWriter/yieldEstopWriter to coordinate tabs. The new code sets writer: true unconditionally on every call, so any tab can claim writer status and publish E-STOP commands. This breaks the invariant that only one tab can hold the lock at a time.
Restore the writer check (or at minimum, only set writer: true when the tab is already the elected writer) to preserve the single-writer guarantee.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Collaborator
Author
Coldaine
pushed a commit
that referenced
this pull request
Aug 3, 2026
…estop, globs authored - Agent path merged on main: ros-singleton + motion-gate both anchor on /ugv/allow_motion - Main's cockpit estop is a one-shot volatile publish (no heartbeat) — the silent-failure pattern - rosbridge globs already authored; Phase 0.4 is verification, not authoring - beast-cockpit.service unit + cutover mechanics documented on main - PR #152 mined by #157: close it
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
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.
User description
Summary
/agentUI, API route, and Beast ROS bridge tools (motion-gated) for immobile bench sessions.Test plan
npm test(or vitest) covers new agent-model / agent-tools / ros-singleton tests/agentlocally; confirm Shell nav link and chat UI renderMade with Cursor
CodeAnt-AI Description
Add a safety-gated BEAST-01 agent path and tighten cockpit motion controls
What Changed
/agentchat surface where operators can request status, approve motion intents, deny them, or cancel active goals.Impact
✅ Motion stays blocked when charging or tethered✅ Approved robot commands remain bounded and disarmed requests are refused✅ Clearer offline, lock, and stale-telemetry states💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary
/agentUI and streaming/api/agent/chatroute.allowMotionis explicitly enabled.Test Plan
/agentrendering and navigation.