Add camera, Video Doorbell, cook doorbell, probe sensors, and cook-mode switches - #2
Conversation
… modes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spike A solved via live capture: 10011 delivers a pre-signed still-JPEG URL (~1fps during a cook) on our own trusted WebSocket, no emulator needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Camera backed by the oven's ~1fps 10011 still feed. Snapshot handler serves the latest pre-signed JPEG (verified live end-to-end: 10011 -> latestSnapshot -> HTTP 200 valid JPEG). Streaming spawns system ffmpeg per config.camera.ffmpegPath; SRTP path is build-verified only (needs a Home hub to confirm). Camera + doorbell combine into a Video Doorbell. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Confirmed live with probe in a water bowl: 10013 sensor_data.probe is an
array of {id:'left'|'right', value:<milliC>}, not the guessed *_probe fields.
Spike B resolved. Cavity 142F vs water probe 65F verified distinct.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Review Complete Files Reviewed: 13 By Severity:
This PR adds expanded HomeKit features (camera streaming, doorbell, mode switch, probe sensors) with significant issues including resource leaks, data cross-contamination, and a security vulnerability. Files Reviewed (13 files) |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds opt-in HomeKit support for a cook doorbell, interior camera, food-probe temperature sensors, and cook-mode switches. It updates configuration normalization, the Homebridge UI/schema, client telemetry parsing, platform discovery, tests, documentation, and the package version. ChangesExpanded HomeKit accessories
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant June WebSocket
participant JuneClient
participant Homebridge Platform
participant HomeKit
participant ffmpeg
June WebSocket->>JuneClient: Send probe and camera messages
JuneClient->>JuneClient: Parse telemetry and store latest snapshot
Homebridge Platform->>JuneClient: Discover configured accessories
Homebridge Platform->>HomeKit: Register doorbell, probe, mode, and camera services
HomeKit->>JuneClient: Start configured cook mode
HomeKit->>Homebridge Platform: Request camera snapshot or stream
Homebridge Platform->>ffmpeg: Convert JPEG frames to H.264 RTP
ffmpeg-->>HomeKit: Deliver camera stream
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/june-client.test.ts (1)
43-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
startModetest only verifies method existence.This test asserts
typeof client.startMode === 'function', which TypeScript already guarantees at compile time. Consider testing that it sends the correct command (e.g., by spying onsendCommandor verifyinglastCancelledis reset), or remove the test if mocking the WebSocket is too costly for the value provided.🤖 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/june-client.test.ts` around lines 43 - 49, The startMode test only checks that JuneClient.startMode exists, which is redundant and not behavior-focused. Update the test in the startMode describe block to assert actual behavior by exercising JuneClient.startMode and verifying it calls sendCommand with the expected command or resets lastCancelled as intended; if that setup is too heavy, remove the test rather than keeping a compile-time existence check.src/june-client.ts (1)
118-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
startModeduplicatespreheatlogic.Both methods reset
lastCancelledand sendMC_PREHEATwith the same payload shape.startModecould delegate topreheatto avoid drift:♻️ Proposed refactor
- public async startMode(primitiveType: string, tempF: number): Promise<string | null> { - this.lastCancelled = false; - return this.sendCommand(MC_PREHEAT, { primitive_type: primitiveType, temperature_cavity: fahrenheitToMilliC(tempF) }); - } + public async startMode(primitiveType: string, tempF: number): Promise<string | null> { + return this.preheat(primitiveType, tempF); + }🤖 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/june-client.ts` around lines 118 - 126, The startMode method duplicates the same reset-and-send flow already implemented in preheat, so refactor it to delegate to preheat instead of repeating the MC_PREHEAT payload construction. Keep the existing primitiveType and tempF parameters in startMode, but have it call preheat with those values so lastCancelled is still reset and the command shape stays centralized in preheat. Use the preheat and startMode methods in june-client.ts as the main points to update.
🤖 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 `@docs/superpowers/plans/2026-07-08-expanded-homekit-features.md`:
- Around line 13-18: The plan text still marks camera/ffmpeg as out of scope,
but the rest of the PR docs now treat camera/video-doorbell support as delivered
scope. Update the scope language in the plan document so it matches the shipped
behavior, or clearly label this section as historical planning; keep the wording
consistent with the other docs and the feature summary to avoid conflicting
scope statements.
In `@docs/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md`:
- Around line 63-67: The probe payload description is inconsistent because it
confirms sensor_data.probe[] and then still references
left_probe/right_probe/probe_temperature as alternatives. Update the spec text
around parseProbeTelemetry to keep only the confirmed sensor_data.probe array
shape and remove the stale fallback language so implementers don’t treat
multiple incompatible contracts as valid.
- Around line 121-136: Align the camera and probe sensor config names in the
June features spec with the runtime schema used elsewhere: update the Feature 2
camera section to include the actual `ffmpegPath` field alongside
`camera.enabled` and `camera.name`, and change the probe sensor naming config
from `probeSensors.names` to the real `probeSensors.leftName` and
`probeSensors.rightName` fields. Use the existing `JuneTelemetry`,
`JuneClient.handleMessage`, and `JuneProbeSensorAccessory` references to keep
the documented contract consistent with implementation.
In `@src/accessories/camera.ts`:
- Around line 100-122: `prepareStream` only logs `socket` errors today, so a
failed `socket.bind` can leave Homebridge waiting forever. Update
`prepareStream` in `CameraAccessory` to invoke the `PrepareStreamCallback` with
an error when the bind fails, and ensure the callback is guarded so it cannot be
called twice if both the bind path and the socket error path fire. Use the
existing `socket`, `callback`, and `sessions` setup to keep the success flow
unchanged while adding proper failure handling.
- Around line 137-187: The `startStream` flow in `camera.ts` leaves stale
sessions active because `proc.on('error')` and `proc.on('exit')` only log
instead of cleaning up. Update `startStream` so that when the ffmpeg child
process emits `error` or exits unexpectedly, it calls `stopSession` for the
current session and clears `session.ffmpeg` before returning/logging. Keep the
existing success path and `callback()` behavior, but ensure the `session` is
always torn down on ffmpeg failure or completion to avoid leaking the UDP socket
and leaving `sessions` stale.
In `@src/accessories/doorbell.ts`:
- Around line 26-31: The `Doorbell.update` logic currently presses on every
telemetry packet while `telemetry.ready` stays true, so add transition tracking
instead of checking the raw flag directly. Update `update(telemetry:
JuneTelemetry)` in `doorbell.ts` to remember the previous `ready` state on the
`Doorbell` instance and only call `press()` when `ready` changes from false to
true, while keeping the existing `done` behavior intact. Use the existing
`update`, `press`, and `JuneTelemetry` symbols to wire this deduplication into
the doorbell state machine.
In `@src/protocol.ts`:
- Around line 47-48: The `modes` input type in `JuneOvenConfig` is too strict
for what `normalizeOvenConfig` actually accepts. Update the type definitions in
`protocol.ts` so the public input for `modes` only requires `primitiveType` and
makes `label` and `tempF` optional, while keeping `normalizeOvenConfig`
responsible for filling defaults. Use a separate input-facing mode type or
adjust `JuneModeConfig` usage so `protocol.test.ts` no longer needs the `as
never` workaround.
---
Nitpick comments:
In `@src/june-client.test.ts`:
- Around line 43-49: The startMode test only checks that JuneClient.startMode
exists, which is redundant and not behavior-focused. Update the test in the
startMode describe block to assert actual behavior by exercising
JuneClient.startMode and verifying it calls sendCommand with the expected
command or resets lastCancelled as intended; if that setup is too heavy, remove
the test rather than keeping a compile-time existence check.
In `@src/june-client.ts`:
- Around line 118-126: The startMode method duplicates the same reset-and-send
flow already implemented in preheat, so refactor it to delegate to preheat
instead of repeating the MC_PREHEAT payload construction. Keep the existing
primitiveType and tempF parameters in startMode, but have it call preheat with
those values so lastCancelled is still reset and the command shape stays
centralized in preheat. Use the preheat and startMode methods in june-client.ts
as the main points to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3365ad13-e87b-496e-a7b4-b6553c8ebff6
📒 Files selected for processing (13)
README.mdconfig.schema.jsondocs/superpowers/plans/2026-07-08-expanded-homekit-features.mddocs/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.mdsrc/accessories/camera.tssrc/accessories/doorbell.tssrc/accessories/mode-switch.tssrc/accessories/probe-sensor.tssrc/june-client.test.tssrc/june-client.tssrc/platform.tssrc/protocol.test.tssrc/protocol.ts
| - Node floors: `18.20.4+`, `20.19.0+`, `22.12.0+`, or `24+`. Homebridge `>=1.8.0`. | ||
| - Every new feature is **opt-in**, default off (doorbell) or empty (modes, probes). Upgrading an existing install must change no behavior. | ||
| - Temperature on the wire is milli-°C; convert with existing helpers (`fahrenheitToMilliC`, `milliCToCelsius`). HomeKit temperature characteristics are Celsius. | ||
| - No new runtime dependencies. Camera/streaming/ffmpeg are OUT of scope for this plan (deferred to Spike A — see spec). | ||
| - Test runner: `npm test` (vitest run). Type-check: `npm run lint` (`tsc --noEmit`). | ||
| - Commit after each task with a `feat:`/`docs:` message. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the plan aligned with the shipped scope.
This still says camera/ffmpeg are out of scope, but the rest of the PR docs now describe camera/video-doorbell support as part of the delivered stack. Please either mark this as historical planning or update the scope so the docs don't disagree.
🤖 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/superpowers/plans/2026-07-08-expanded-homekit-features.md` around lines
13 - 18, The plan text still marks camera/ffmpeg as out of scope, but the rest
of the PR docs now treat camera/video-doorbell support as delivered scope.
Update the scope language in the plan document so it matches the shipped
behavior, or clearly label this section as historical planning; keep the wording
consistent with the other docs and the feature summary to avoid conflicting
scope statements.
| - Depends on Spike A. Config (per oven): `camera.enabled` (bool, default false), `camera.name`. | ||
| - Implemented as part of the same accessory as the doorbell (feature 1) when both are enabled; | ||
| camera-only (no doorbell triggers) is also valid. | ||
|
|
||
| ## Feature 3 — Food probe temperature sensors | ||
|
|
||
| Expose the oven's food probe temperature(s) as HomeKit **Temperature Sensor** services, so users | ||
| can build automations like "notify when probe reaches 145 °F." | ||
|
|
||
| - Source: `10013` telemetry probe fields (exact path from Spike B). Support up to two probes | ||
| (left/right); a sensor is only shown when its probe reports a reading, and reads as inactive / | ||
| its last value otherwise. | ||
| - Opt-in. Config (per oven): `probeSensors.enabled` (bool, default false). Optionally a | ||
| `probeSensors.names` map for left/right display names. | ||
| - New `JuneTelemetry` fields (e.g. `probeLeftC`, `probeRightC`, `probePresent`) populated in | ||
| `JuneClient.handleMessage` for `10013`; a `JuneProbeSensorAccessory` subscribes to telemetry. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the actual config fields.
The camera config here omits ffmpegPath, and probe sensors are described as probeSensors.names, but the schema/runtime use leftName/rightName. Please align these fields with the config contract used elsewhere in the stack.
🤖 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/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md`
around lines 121 - 136, Align the camera and probe sensor config names in the
June features spec with the runtime schema used elsewhere: update the Feature 2
camera section to include the actual `ffmpegPath` field alongside
`camera.enabled` and `camera.name`, and change the probe sensor naming config
from `probeSensors.names` to the real `probeSensors.leftName` and
`probeSensors.rightName` fields. Use the existing `JuneTelemetry`,
`JuneClient.handleMessage`, and `JuneProbeSensorAccessory` references to keep
the documented contract consistent with implementation.
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | ||
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | ||
| socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`)); | ||
| socket.bind(() => { | ||
| const localPort = socket.address().port; | ||
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | ||
| this.sessions.set(request.sessionID, { | ||
| socket, | ||
| targetAddress: request.targetAddress, | ||
| videoPort: request.video.port, | ||
| videoSsrc: ssrc, | ||
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | ||
| }); | ||
| callback(undefined, { | ||
| video: { | ||
| port: localPort, | ||
| ssrc, | ||
| srtp_key: request.video.srtp_key, | ||
| srtp_salt: request.video.srtp_salt, | ||
| }, | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
prepareStream never calls callback on socket bind error — Homebridge hangs.
If socket.bind fails (e.g., no available ports, network down), the error event fires and is logged, but callback is never invoked. Homebridge blocks indefinitely waiting for stream preparation to complete.
🔒 Proposed fix: guard callback against double-invocation
public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void {
const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4');
+ let settled = false;
+ const finish = (err: Error | undefined, info?: { video: { port: number; ssrc: number; srtp_key: Buffer; srtp_salt: Buffer } }) => {
+ if (settled) return;
+ settled = true;
+ callback(err, info);
+ };
- socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`));
+ socket.on('error', error => {
+ this.platform.log.warn(`June camera RTCP socket error: ${error.message}`);
+ finish(new Error(`RTCP socket error: ${error.message}`));
+ });
socket.bind(() => {
const localPort = socket.address().port;
const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource();
this.sessions.set(request.sessionID, {
socket,
targetAddress: request.targetAddress,
videoPort: request.video.port,
videoSsrc: ssrc,
videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'),
});
- callback(undefined, {
+ finish(undefined, {
video: {
port: localPort,
ssrc,
srtp_key: request.video.srtp_key,
srtp_salt: request.video.srtp_salt,
},
});
});
}📝 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.
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | |
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | |
| socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`)); | |
| socket.bind(() => { | |
| const localPort = socket.address().port; | |
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | |
| this.sessions.set(request.sessionID, { | |
| socket, | |
| targetAddress: request.targetAddress, | |
| videoPort: request.video.port, | |
| videoSsrc: ssrc, | |
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | |
| }); | |
| callback(undefined, { | |
| video: { | |
| port: localPort, | |
| ssrc, | |
| srtp_key: request.video.srtp_key, | |
| srtp_salt: request.video.srtp_salt, | |
| }, | |
| }); | |
| }); | |
| } | |
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | |
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | |
| let settled = false; | |
| const finish = (err: Error | undefined, info?: { video: { port: number; ssrc: number; srtp_key: Buffer; srtp_salt: Buffer } }) => { | |
| if (settled) return; | |
| settled = true; | |
| callback(err, info); | |
| }; | |
| socket.on('error', error => { | |
| this.platform.log.warn(`June camera RTCP socket error: ${error.message}`); | |
| finish(new Error(`RTCP socket error: ${error.message}`)); | |
| }); | |
| socket.bind(() => { | |
| const localPort = socket.address().port; | |
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | |
| this.sessions.set(request.sessionID, { | |
| socket, | |
| targetAddress: request.targetAddress, | |
| videoPort: request.video.port, | |
| videoSsrc: ssrc, | |
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | |
| }); | |
| finish(undefined, { | |
| video: { | |
| port: localPort, | |
| ssrc, | |
| srtp_key: request.video.srtp_key, | |
| srtp_salt: request.video.srtp_salt, | |
| }, | |
| }); | |
| }); | |
| } |
🤖 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/accessories/camera.ts` around lines 100 - 122, `prepareStream` only logs
`socket` errors today, so a failed `socket.bind` can leave Homebridge waiting
forever. Update `prepareStream` in `CameraAccessory` to invoke the
`PrepareStreamCallback` with an error when the bind fails, and ensure the
callback is guarded so it cannot be called twice if both the bind path and the
socket error path fire. Use the existing `socket`, `callback`, and `sessions`
setup to keep the success flow unchanged while adding proper failure handling.
| private startStream(request: StartStreamRequest, callback: StreamRequestCallback): void { | ||
| const session = this.sessions.get(request.sessionID); | ||
| const snapshot = this.client.latestSnapshot; | ||
| if (!session) { | ||
| callback(new Error('No prepared session')); | ||
| return; | ||
| } | ||
| if (!snapshot) { | ||
| this.platform.log.warn('June camera has no frame yet (no active cook) — cannot start live stream.'); | ||
| callback(new Error('No camera frame available')); | ||
| return; | ||
| } | ||
| const { video } = request; | ||
| const args = [ | ||
| '-loglevel', 'error', | ||
| '-loop', '1', '-re', '-i', snapshot.url, | ||
| '-an', '-sn', '-dn', | ||
| '-codec:v', 'libx264', '-pix_fmt', 'yuv420p', '-profile:v', 'baseline', | ||
| '-preset', 'ultrafast', '-tune', 'zerolatency', | ||
| '-r', String(video.fps), | ||
| '-vf', `scale=${video.width}:${video.height}`, | ||
| '-b:v', `${video.max_bit_rate}k`, '-bufsize', `${2 * video.max_bit_rate}k`, '-maxrate', `${video.max_bit_rate}k`, | ||
| '-payload_type', String(video.pt), | ||
| '-ssrc', String(session.videoSsrc), | ||
| '-f', 'rtp', | ||
| '-srtp_out_suite', 'AES_CM_128_HMAC_SHA1_80', | ||
| '-srtp_out_params', session.videoSrtp, | ||
| `srtp://${session.targetAddress}:${session.videoPort}?rtcpport=${session.videoPort}&pkt_size=1316`, | ||
| ]; | ||
|
|
||
| const ffmpegPath = this.client.config.camera.ffmpegPath; | ||
| let proc: ChildProcess; | ||
| try { | ||
| proc = spawn(ffmpegPath, args, { env: process.env }); | ||
| } catch (error) { | ||
| this.platform.log.error(`June camera: failed to spawn ffmpeg ("${ffmpegPath}"): ${(error as Error).message}`); | ||
| callback(new Error('ffmpeg not available')); | ||
| return; | ||
| } | ||
| session.ffmpeg = proc; | ||
| proc.on('error', error => | ||
| this.platform.log.error(`June camera ffmpeg error: ${error.message} (is ffmpeg installed at "${ffmpegPath}"?)`), | ||
| ); | ||
| proc.stderr?.on('data', data => this.platform.log.debug(`[june-camera ffmpeg] ${data}`)); | ||
| proc.on('exit', (code, signal) => { | ||
| if (code !== null && code !== 0 && signal !== 'SIGKILL') { | ||
| this.platform.log.warn(`June camera ffmpeg exited with code ${code}`); | ||
| } | ||
| }); | ||
| callback(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ffmpeg async errors and exits are not cleaned up — session leaks and stream appears active.
spawn returns successfully even when the ffmpeg binary is missing; the error event fires asynchronously after callback() has already been called, so Homebridge believes the stream started. Additionally, when ffmpeg exits (crash or normal completion), the session socket is never closed unless Homebridge separately sends a STOP request. Both cases leak the UDP socket and leave a stale session in the map.
🧹 Proposed fix: clean up session on ffmpeg error and exit
session.ffmpeg = proc;
proc.on('error', error =>
- this.platform.log.error(`June camera ffmpeg error: ${error.message} (is ffmpeg installed at "${ffmpegPath}"?)`),
+ this.platform.log.error(`June camera ffmpeg error: ${error.message} (is ffmpeg installed at "${ffmpegPath}"?)`),
+ this.stopSession(request.sessionID),
);
proc.stderr?.on('data', data => this.platform.log.debug(`[june-camera ffmpeg] ${data}`));
proc.on('exit', (code, signal) => {
if (code !== null && code !== 0 && signal !== 'SIGKILL') {
this.platform.log.warn(`June camera ffmpeg exited with code ${code}`);
}
+ this.stopSession(request.sessionID);
});
callback();Note: stopSession is idempotent — it no-ops if the session was already removed, so calling it from both the exit handler and a later Homebridge STOP request is safe.
🤖 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/accessories/camera.ts` around lines 137 - 187, The `startStream` flow in
`camera.ts` leaves stale sessions active because `proc.on('error')` and
`proc.on('exit')` only log instead of cleaning up. Update `startStream` so that
when the ffmpeg child process emits `error` or exits unexpectedly, it calls
`stopSession` for the current session and clears `session.ffmpeg` before
returning/logging. Keep the existing success path and `callback()` behavior, but
ensure the `session` is always torn down on ffmpeg failure or completion to
avoid leaking the UDP socket and leaving `sessions` stale.
| private update(telemetry: JuneTelemetry): void { | ||
| const triggers = this.client.config.doorbell.triggers; | ||
| if ((triggers.done && telemetry.done) || (triggers.ready && telemetry.ready)) { | ||
| this.press(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Doorbell rings on every telemetry update where ready is true — add transition tracking.
The done flag fires once per active→inactive transition, but ready can remain true across multiple telemetry updates (the oven keeps sending temperature readings while near target). Without deduplication, the doorbell will press on every update, flooding the user with HomeKit notifications.
🔔 Proposed fix: ring only on false→true transitions
export class JuneDoorbellAccessory {
private readonly service: Service;
+ private rangDone = false;
+ private rangReady = false;
constructor(
private readonly platform: JunePlatform,
private readonly accessory: PlatformAccessory,
private readonly client: JuneClient,
) {
const { Service, Characteristic } = this.platform;
this.service = accessory.getService(Service.Doorbell) || accessory.addService(Service.Doorbell);
this.service.setCharacteristic(Characteristic.Name, this.client.config.doorbell.name);
this.client.on('telemetry', telemetry => this.update(telemetry));
}
private update(telemetry: JuneTelemetry): void {
const triggers = this.client.config.doorbell.triggers;
+ // Reset when a new cook starts so the next done/ready event can ring again
+ if (telemetry.active) {
+ this.rangDone = false;
+ this.rangReady = false;
+ }
- if ((triggers.done && telemetry.done) || (triggers.ready && telemetry.ready)) {
- this.press();
- }
+ if (triggers.done && telemetry.done && !this.rangDone) {
+ this.rangDone = true;
+ this.press();
+ }
+ if (triggers.ready && telemetry.ready && !this.rangReady) {
+ this.rangReady = true;
+ this.press();
+ }
}🤖 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/accessories/doorbell.ts` around lines 26 - 31, The `Doorbell.update`
logic currently presses on every telemetry packet while `telemetry.ready` stays
true, so add transition tracking instead of checking the raw flag directly.
Update `update(telemetry: JuneTelemetry)` in `doorbell.ts` to remember the
previous `ready` state on the `Doorbell` instance and only call `press()` when
`ready` changes from false to true, while keeping the existing `done` behavior
intact. Use the existing `update`, `press`, and `JuneTelemetry` symbols to wire
this deduplication into the doorbell state machine.
| doorbell?: Partial<Omit<JuneDoorbellConfig, 'triggers'>> & { triggers?: Partial<JuneDoorbellConfig['triggers']> }; | ||
| modes?: JuneModeConfig[]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
modes input type is stricter than the normalization logic.
JuneOvenConfig.modes is typed as JuneModeConfig[], requiring label, primitiveType, and tempF on every entry. But normalizeOvenConfig (line 122) provides defaults for label (m.label || m.primitiveType) and tempF (m.tempF ?? 350), so users should be able to omit them. The test at src/protocol.test.ts:56 uses as never to work around this mismatch.
Consider a separate input type that only requires primitiveType:
🔧 Proposed type fix
export interface JuneOvenConfig {
// ...
- modes?: JuneModeConfig[];
+ modes?: Array<{ primitiveType: string; label?: string; tempF?: number }>;
// ...
}📝 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.
| doorbell?: Partial<Omit<JuneDoorbellConfig, 'triggers'>> & { triggers?: Partial<JuneDoorbellConfig['triggers']> }; | |
| modes?: JuneModeConfig[]; | |
| doorbell?: Partial<Omit<JuneDoorbellConfig, 'triggers'>> & { triggers?: Partial<JuneDoorbellConfig['triggers']> }; | |
| modes?: Array<{ primitiveType: string; label?: string; tempF?: number }>; |
🤖 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/protocol.ts` around lines 47 - 48, The `modes` input type in
`JuneOvenConfig` is too strict for what `normalizeOvenConfig` actually accepts.
Update the type definitions in `protocol.ts` so the public input for `modes`
only requires `primitiveType` and makes `label` and `tempF` optional, while
keeping `normalizeOvenConfig` responsible for filling defaults. Use a separate
input-facing mode type or adjust `JuneModeConfig` usage so `protocol.test.ts` no
longer needs the `as never` workaround.
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟠 High (72/100) — 3 high findings, 6 medium · 1599 LOC across 13 files
Overview
This PR introduces camera streaming (ffmpeg/RTP/SRTP), doorbell accessory, mode switch for oven control, and probe temperature sensors across 13 files (5 new, 8 modified). The review uncovered 10 findings with severity ranging from medium to high.
Critical Issues
- Probe telemetry cross-contamination (
src/june-client.ts:58): A disconnected left probe can inherit the right probe's temperature due to stale object reference sharing. This provides incorrect temperature data to users. - Camera advertises audio but never streams audio (
src/accessories/camera.ts:70): The camera's streaming configuration declares audio codecs but no audio track is set up, causing HomeKit clients to incorrectly expect audio capability.
Security
- SSRF via snapshot URL (
src/accessories/camera.ts:88): The snapshot URL received from WebSocket data is passed toffmpegwithout validation, allowing a compromised oven or MITM attacker to cause the host to request arbitrary URLs.
Resource Leaks
- No shutdown cleanup (
src/accessories/camera.ts:47): When the camera streaming session stops, ffmpeg subprocess and UDP sockets are not cleaned up, causing process orphans and socket leaks. - UDP socket leak on stream start failure (
src/accessories/camera.ts:144): If stream preparation fails after socket creation, the socket is never closed.
Protocol & Concurrency
- Doorbell repeated rings (
src/accessories/doorbell.ts:26): The doorbell firesProgrammableSwitchEventon every telemetry poll while the oven is in ready state instead of once per cooking cycle. - Mode switch race condition (
src/accessories/mode-switch.ts:46): Telemetry-drivensetAllOff()races with user-drivensetOn(), potentially toggling the mode incorrectly. - Side-effect before guard (
src/june-client.ts:129):lastCancelledis mutated beforesendCommandexecutes, persisting cancellation state even when the command send fails. - Missing error callback (
src/accessories/camera.ts:100):prepareStreamnever invokes the callback when the UDP socket bind fails, leaving the HomeKit stream setup hung indefinitely.
HomeKit Compliance
- Probe temperature range violation (
src/accessories/probe-sensor.ts:21): Probe sensors useCurrentTemperaturecharacteristic without clamping to HAP's valid 0-100°C range, potentially violating HomeKit spec.
| private readonly sessions = new Map<string, Session>(); | ||
|
|
||
| constructor( | ||
| private readonly platform: JunePlatform, | ||
| private readonly client: JuneClient, | ||
| ) { | ||
| const hap = this.platform.api.hap; | ||
| this.controller = new hap.CameraController({ | ||
| cameraStreamCount: 2, | ||
| delegate: this, | ||
| streamingOptions: { | ||
| supportedCryptoSuites: [hap.SRTPCryptoSuites.AES_CM_128_HMAC_SHA1_80], | ||
| video: { | ||
| resolutions: [ | ||
| [640, 480, 15], | ||
| [640, 480, 2], | ||
| [320, 240, 15], | ||
| ], | ||
| codec: { | ||
| profiles: [hap.H264Profile.BASELINE, hap.H264Profile.MAIN, hap.H264Profile.HIGH], | ||
| levels: [hap.H264Level.LEVEL3_1, hap.H264Level.LEVEL3_2, hap.H264Level.LEVEL4_0], | ||
| }, | ||
| }, | ||
| audio: { | ||
| codecs: [ | ||
| { type: hap.AudioStreamingCodecType.AAC_ELD, samplerate: hap.AudioStreamingSamplerate.KHZ_16 }, | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| public async handleSnapshotRequest(_request: SnapshotRequest, callback: SnapshotRequestCallback): Promise<void> { | ||
| const snapshot = this.client.latestSnapshot; | ||
| if (!snapshot) { | ||
| callback(undefined, PLACEHOLDER_JPEG); | ||
| return; | ||
| } | ||
| try { | ||
| const controller = new AbortController(); | ||
| const timer = setTimeout(() => controller.abort(), 5000); | ||
| const response = await fetch(snapshot.url, { signal: controller.signal }); | ||
| clearTimeout(timer); | ||
| if (!response.ok) { | ||
| throw new Error(`snapshot fetch ${response.status}`); | ||
| } | ||
| callback(undefined, Buffer.from(await response.arrayBuffer())); | ||
| } catch (error) { | ||
| this.platform.log.warn(`June camera snapshot failed: ${(error as Error).message}`); | ||
| callback(undefined, PLACEHOLDER_JPEG); | ||
| } | ||
| } | ||
|
|
||
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | ||
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | ||
| socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`)); | ||
| socket.bind(() => { | ||
| const localPort = socket.address().port; | ||
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | ||
| this.sessions.set(request.sessionID, { | ||
| socket, | ||
| targetAddress: request.targetAddress, | ||
| videoPort: request.video.port, | ||
| videoSsrc: ssrc, | ||
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | ||
| }); | ||
| callback(undefined, { | ||
| video: { | ||
| port: localPort, | ||
| ssrc, | ||
| srtp_key: request.video.srtp_key, | ||
| srtp_salt: request.video.srtp_salt, | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| public handleStreamRequest(request: StreamingRequest, callback: StreamRequestCallback): void { | ||
| const { StreamRequestTypes } = this.platform.api.hap; | ||
| if (request.type === StreamRequestTypes.START) { | ||
| this.startStream(request, callback); | ||
| return; | ||
| } | ||
| if (request.type === StreamRequestTypes.STOP) { | ||
| this.stopSession(request.sessionID); | ||
| } | ||
| // RECONFIGURE is a no-op for a still-based source. | ||
| callback(); | ||
| } | ||
|
|
||
| private startStream(request: StartStreamRequest, callback: StreamRequestCallback): void { | ||
| const session = this.sessions.get(request.sessionID); | ||
| const snapshot = this.client.latestSnapshot; | ||
| if (!session) { | ||
| callback(new Error('No prepared session')); | ||
| return; | ||
| } | ||
| if (!snapshot) { | ||
| this.platform.log.warn('June camera has no frame yet (no active cook) — cannot start live stream.'); | ||
| callback(new Error('No camera frame available')); | ||
| return; | ||
| } | ||
| const { video } = request; | ||
| const args = [ | ||
| '-loglevel', 'error', | ||
| '-loop', '1', '-re', '-i', snapshot.url, | ||
| '-an', '-sn', '-dn', | ||
| '-codec:v', 'libx264', '-pix_fmt', 'yuv420p', '-profile:v', 'baseline', | ||
| '-preset', 'ultrafast', '-tune', 'zerolatency', | ||
| '-r', String(video.fps), | ||
| '-vf', `scale=${video.width}:${video.height}`, | ||
| '-b:v', `${video.max_bit_rate}k`, '-bufsize', `${2 * video.max_bit_rate}k`, '-maxrate', `${video.max_bit_rate}k`, | ||
| '-payload_type', String(video.pt), | ||
| '-ssrc', String(session.videoSsrc), | ||
| '-f', 'rtp', | ||
| '-srtp_out_suite', 'AES_CM_128_HMAC_SHA1_80', | ||
| '-srtp_out_params', session.videoSrtp, | ||
| `srtp://${session.targetAddress}:${session.videoPort}?rtcpport=${session.videoPort}&pkt_size=1316`, | ||
| ]; | ||
|
|
||
| const ffmpegPath = this.client.config.camera.ffmpegPath; | ||
| let proc: ChildProcess; | ||
| try { | ||
| proc = spawn(ffmpegPath, args, { env: process.env }); | ||
| } catch (error) { | ||
| this.platform.log.error(`June camera: failed to spawn ffmpeg ("${ffmpegPath}"): ${(error as Error).message}`); | ||
| callback(new Error('ffmpeg not available')); | ||
| return; | ||
| } | ||
| session.ffmpeg = proc; | ||
| proc.on('error', error => | ||
| this.platform.log.error(`June camera ffmpeg error: ${error.message} (is ffmpeg installed at "${ffmpegPath}"?)`), | ||
| ); | ||
| proc.stderr?.on('data', data => this.platform.log.debug(`[june-camera ffmpeg] ${data}`)); | ||
| proc.on('exit', (code, signal) => { | ||
| if (code !== null && code !== 0 && signal !== 'SIGKILL') { | ||
| this.platform.log.warn(`June camera ffmpeg exited with code ${code}`); | ||
| } | ||
| }); | ||
| callback(); | ||
| } | ||
|
|
||
| private stopSession(sessionID: string): void { | ||
| const session = this.sessions.get(sessionID); | ||
| if (!session) { | ||
| return; | ||
| } | ||
| session.ffmpeg?.kill('SIGKILL'); | ||
| try { | ||
| session.socket?.close(); | ||
| } catch { | ||
| // socket may already be closed | ||
| } | ||
| this.sessions.delete(sessionID); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 No cleanup on shutdown — ffmpeg orphans and UDP socket leaks (bug)
The JuneCameraSource class (src/accessories/camera.ts:45-202) has no destroy() or cleanup method. Neither platform.ts nor any caller iterates camera sources to shut them down on plugin unload. When HomeBridge restarts or the plugin is unloaded, any running ffmpeg processes become orphaned (still running with no parent monitoring them), and all bound UDP sockets remain open until the Node.js process exits. The JunePlatform class does call client.stop() on its JuneClient instances (june-client.ts:111) but has no corresponding cleanup for attached camera sources.
💡 Suggestion: Add a public destroy() method to JuneCameraSource that iterates all sessions, kills ffmpeg processes, closes sockets, and clears the sessions map. Store the source reference from attachCamera and invoke destroy() from the platform's shutdown path.
📋 Prompt for AI Agents
- In src/accessories/camera.ts, add to JuneCameraSource class:
public destroy(): void {
for (const [sessionID] of this.sessions) {
this.stopSession(sessionID);
}
} - Change attachCamera() to return the JuneCameraSource instance.
- In src/platform.ts, store camera source references (e.g., in a Map<string, JuneCameraSource>) and iterate them on plugin shutdown, calling source.destroy() on each.
| const left = valueOf(probes.find((p: any) => p?.id === 'left')) ?? valueOf(probes.find((p: any) => valueOf(p) !== undefined)); | ||
| const right = valueOf(probes.find((p: any) => p?.id === 'right')); |
There was a problem hiding this comment.
🟠 Probe telemetry cross-contamination: disconnected left probe inherits right probe's temperature (bug)
In parseProbeTelemetry (src/june-client.ts:58), the left-probe fallback chain uses a nullish coalesce with a find over any probe that has a numeric value. When both probes are reported in the sensor_data.probe array but the left probe's value is undefined/null/absent (indicating a disconnected or malfunctioning probe), the fallback captures the right probe's temperature and assigns it to probeLeftC. Meanwhile the right probe's value is correctly assigned to probeRightC via the direct find at line 59. Result: both probeLeftC and probeRightC reflect the right probe's temperature — a cross-contamination that hides the left probe's disconnected state from the user, which could lead to food safety issues if a probe silently reads the wrong temperature.
💡 Suggestion: Remove the right-hand side of the ?? fallback for the left probe. If no left probe with a valid numeric value is found, probeLeftC should remain undefined. The fallback was intended for single-probe ovens (which always report id='left'), but it backfires dangerously when a dual-probe oven has a disconnected left probe.
📋 Prompt for AI Agents
In src/june-client.ts, line 58, replace:
const left = valueOf(probes.find((p: any) => p?.id === 'left')) ?? valueOf(probes.find((p: any) => valueOf(p) !== undefined));
with:
const left = valueOf(probes.find((p: any) => p?.id === 'left'));
This removes the dangerous fallback. The single-probe case is already handled because single-probe ovens report [{id:'left', value:...}] — the direct find matches. If future protocol versions omit the 'left' id for single probes, a safer alternative is: probes.length === 1 ? valueOf(probes[0]) : valueOf(probes.find(p => p?.id === 'left')).
| private update(telemetry: JuneTelemetry): void { | ||
| const triggers = this.client.config.doorbell.triggers; | ||
| if ((triggers.done && telemetry.done) || (triggers.ready && telemetry.ready)) { | ||
| this.press(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Doorbell rings repeatedly during ready state instead of once per event (bug)
The doorbell accessory (src/accessories/doorbell.ts) calls press() on every telemetry update where the done or ready trigger is true (line 28), without edge detection or debouncing. The ready field is recomputed from live telemetry data on each 10013 frame (cook_state_data.progress >= 0.995 at june-client.ts:256, refined by temperature check at june-client.ts:293-295). When the oven stays in the ready state, every 10013 frame (~1/s during a cook) re-emits ready: true, causing the doorbell notification to fire repeatedly. For comparison, the existing JuneOccupancySensorAccessory (sensors.ts:30-33) has a similar pattern for ready/done but is resilient because it uses a 30-second timeout to reset OccupancyDetected; the doorbell has no such dampening.
💡 Suggestion: Track the previous state and only call press() when transitioning from false→true. Add private lastReady = false; private lastDone = false; to the class, then in update() check for edge transitions before firing. Alternatively, add a cooldown timer similar to the occupancy sensor's 30-second debounce.
📋 Prompt for AI Agents
In src/accessories/doorbell.ts, modify the JuneDoorbellAccessory class:
- Add private fields:
private lastReady = false;andprivate lastDone = false; - In the
update()method (lines 26-31), replace the current condition with edge-detection:
const readyEdge = triggers.ready && telemetry.ready && !this.lastReady;
const doneEdge = triggers.done && telemetry.done && !this.lastDone;
if (readyEdge || doneEdge) {
this.press();
}
this.lastReady = telemetry.ready ?? false;
this.lastDone = telemetry.done ?? false;
| if (!snapshot) { | ||
| this.platform.log.warn('June camera has no frame yet (no active cook) — cannot start live stream.'); | ||
| callback(new Error('No camera frame available')); | ||
| return; | ||
| } | ||
| const { video } = request; | ||
| const args = [ | ||
| '-loglevel', 'error', | ||
| '-loop', '1', '-re', '-i', snapshot.url, | ||
| '-an', '-sn', '-dn', | ||
| '-codec:v', 'libx264', '-pix_fmt', 'yuv420p', '-profile:v', 'baseline', | ||
| '-preset', 'ultrafast', '-tune', 'zerolatency', | ||
| '-r', String(video.fps), | ||
| '-vf', `scale=${video.width}:${video.height}`, | ||
| '-b:v', `${video.max_bit_rate}k`, '-bufsize', `${2 * video.max_bit_rate}k`, '-maxrate', `${video.max_bit_rate}k`, | ||
| '-payload_type', String(video.pt), | ||
| '-ssrc', String(session.videoSsrc), | ||
| '-f', 'rtp', | ||
| '-srtp_out_suite', 'AES_CM_128_HMAC_SHA1_80', | ||
| '-srtp_out_params', session.videoSrtp, | ||
| `srtp://${session.targetAddress}:${session.videoPort}?rtcpport=${session.videoPort}&pkt_size=1316`, | ||
| ]; | ||
|
|
||
| const ffmpegPath = this.client.config.camera.ffmpegPath; | ||
| let proc: ChildProcess; | ||
| try { | ||
| proc = spawn(ffmpegPath, args, { env: process.env }); | ||
| } catch (error) { | ||
| this.platform.log.error(`June camera: failed to spawn ffmpeg ("${ffmpegPath}"): ${(error as Error).message}`); | ||
| callback(new Error('ffmpeg not available')); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🟡 UDP socket leak on stream start failure — session not cleaned up (bug)
In startStream() (src/accessories/camera.ts:137-187), two early-return paths leave the session (with its bound UDP socket created in prepareStream) in this.sessions without cleanup: (1) no snapshot available at lines 144-147, and (2) ffmpeg spawn throws at lines 169-174. In both cases, callback(new Error(...)) is called and the method returns without calling stopSession(request.sessionID). If the HomeKit controller does not send a STOP request after receiving a START error, the socket stays bound indefinitely and the session entry leaks. Over repeated attempts, multiple UDP sockets accumulate.
💡 Suggestion: Call this.stopSession(request.sessionID) before returning the error callback in both early-return paths of startStream. This ensures the bound UDP socket is closed and the session is removed from the map regardless of why streaming fails.
📋 Prompt for AI Agents
In src/accessories/camera.ts, in startStream():
- At line 145 (after the log.warn), add
this.stopSession(request.sessionID);before the callback on line 146. - At line 172 (after the log.error), add
this.stopSession(request.sessionID);before the callback on line 173.
This ensures the bound UDP socket from prepareStream is always cleaned up on start failure.
| audio: { | ||
| codecs: [ | ||
| { type: hap.AudioStreamingCodecType.AAC_ELD, samplerate: hap.AudioStreamingSamplerate.KHZ_16 }, | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| public async handleSnapshotRequest(_request: SnapshotRequest, callback: SnapshotRequestCallback): Promise<void> { | ||
| const snapshot = this.client.latestSnapshot; |
There was a problem hiding this comment.
🟠 Camera advertises audio codecs but never negotiates or streams audio (bug)
The JuneCameraSource streaming options declare AAC-ELD audio at 16 kHz (camera.ts:70-75), but the prepareStream callback only returns a video configuration object — no audio port, SSRC, or SRTP parameters (camera.ts:113-120). The ffmpeg command in startStream uses -an to disable audio (camera.ts:153). Per the HAP CameraRTPStreamManagement specification, if audio codecs appear in SupportedRTPConfiguration, the SelectedRTPConfiguration response must include negotiated audio parameters. HomeKit's streaming controller may reject the session when audio is advertised but not provided in the negotiation response, causing live-stream tap-to-view to fail silently on affected controllers.
💡 Suggestion: Remove the audio codec block from streamingOptions (simplest, since this camera has no audio source). This is a still-image-based camera with no microphone input.
📋 Prompt for AI Agents
In src/accessories/camera.ts, remove lines 70-75 (the audio: block from streamingOptions). This still-image-based camera source has no audio input, so advertising audio codecs causes a protocol mismatch with HomeKit controllers. The resulting streamingOptions should only contain supportedCryptoSuites and video. If audio support is added later, implement full audio negotiation in prepareStream (return audio: {port, ssrc, srtp_key, srtp_salt} matching the request) and add audio encoding to the ffmpeg command in startStream.
| this.left = this.accessory.getServiceById(Service.TemperatureSensor, 'probe-left') | ||
| || this.accessory.addService(Service.TemperatureSensor, cfg.leftName, 'probe-left'); | ||
| this.left.setCharacteristic(Characteristic.Name, cfg.leftName); | ||
| this.right = this.accessory.getServiceById(Service.TemperatureSensor, 'probe-right') | ||
| || this.accessory.addService(Service.TemperatureSensor, cfg.rightName, 'probe-right'); | ||
| this.right.setCharacteristic(Characteristic.Name, cfg.rightName); |
There was a problem hiding this comment.
🟠 Probe temperature sensors may exceed HAP CurrentTemperature valid range (0–100°C) (bug)
The probe-sensor.ts accessory sets CurrentTemperature on TemperatureSensor services without adjusting the characteristic's valid value range (probe-sensor.ts:21-26). The HAP specification defines CurrentTemperature with default minValue: 0 and maxValue: 100 (°C). The thermostat accessory in the same codebase explicitly widens this to maxValue: 300 via setProps (thermostat.ts:22), but probe-sensor.ts does not. If a food probe reads above 100°C — common when a probe is left dangling in a hot oven cavity or during a sensor fault — the value would be clamped or rejected, displaying incorrect temperature in the Home app.
💡 Suggestion: After creating the TemperatureSensor services, call .getCharacteristic(Characteristic.CurrentTemperature).setProps({minValue: -20, maxValue: 300}) on both left and right probe services, matching the thermostat's approach.
📋 Prompt for AI Agents
In src/accessories/probe-sensor.ts, after line 23 (this.left.setCharacteristic(...)) add:
this.left.getCharacteristic(Characteristic.CurrentTemperature).setProps({minValue: -20, maxValue: 300});
After line 26 (this.right.setCharacteristic(...)) add:
this.right.getCharacteristic(Characteristic.CurrentTemperature).setProps({minValue: -20, maxValue: 300});
This widens the HAP valid range to accommodate food probes reading cavity air or sensor extremes. The -20°C floor handles frozen food; the 300°C ceiling matches the thermostat.
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | ||
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | ||
| socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`)); | ||
| socket.bind(() => { | ||
| const localPort = socket.address().port; | ||
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | ||
| this.sessions.set(request.sessionID, { | ||
| socket, | ||
| targetAddress: request.targetAddress, | ||
| videoPort: request.video.port, | ||
| videoSsrc: ssrc, | ||
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | ||
| }); | ||
| callback(undefined, { | ||
| video: { | ||
| port: localPort, | ||
| ssrc, | ||
| srtp_key: request.video.srtp_key, | ||
| srtp_salt: request.video.srtp_salt, | ||
| }, | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 Missing error callback: prepareStream never calls back when socket bind fails (bug)
In JuneCameraSource.prepareStream (camera.ts:100-122), socket.bind() is called with a success callback (lines 103-121) that invokes the HomeKit-provided callback. If createSocket() throws synchronously or socket.bind() fails (emitting 'error'), the HomeKit callback is never invoked. The error handler at line 102 only logs a warning. This causes the HomeKit streaming session to hang indefinitely with no timeout or recovery. The HomeKit callback MUST always be invoked (either with success or error) to complete the streaming negotiation handshake.
💡 Suggestion: Wrap createSocket in try/catch and call callback(err) on failure. In the socket 'error' handler (line 102), also call callback(new Error(...)) after logging. Wrap the bind success callback body in try/catch so any exception also results in callback being called with an error.
📋 Prompt for AI Agents
In src/accessories/camera.ts, modify prepareStream (lines 100-122):
- Wrap
createSocketin try/catch: if it throws, callcallback(err)and return. - Change the socket 'error' handler at line 102 to also call
callback(new Error(...))after logging. - Wrap the body of the bind success callback (lines 104-121) in try/catch so any exception also calls
callback(err).
This ensures the HomeKit streaming negotiation always completes (success or error) rather than hanging indefinitely.
| const response = await fetch(snapshot.url, { signal: controller.signal }); | ||
| clearTimeout(timer); | ||
| if (!response.ok) { | ||
| throw new Error(`snapshot fetch ${response.status}`); | ||
| } | ||
| callback(undefined, Buffer.from(await response.arrayBuffer())); | ||
| } catch (error) { | ||
| this.platform.log.warn(`June camera snapshot failed: ${(error as Error).message}`); | ||
| callback(undefined, PLACEHOLDER_JPEG); | ||
| } | ||
| } | ||
|
|
||
| public prepareStream(request: PrepareStreamRequest, callback: PrepareStreamCallback): void { | ||
| const socket = createSocket(request.addressVersion === 'ipv6' ? 'udp6' : 'udp4'); | ||
| socket.on('error', error => this.platform.log.warn(`June camera RTCP socket error: ${error.message}`)); | ||
| socket.bind(() => { | ||
| const localPort = socket.address().port; | ||
| const ssrc = this.platform.api.hap.CameraController.generateSynchronisationSource(); | ||
| this.sessions.set(request.sessionID, { | ||
| socket, | ||
| targetAddress: request.targetAddress, | ||
| videoPort: request.video.port, | ||
| videoSsrc: ssrc, | ||
| videoSrtp: Buffer.concat([request.video.srtp_key, request.video.srtp_salt]).toString('base64'), | ||
| }); | ||
| callback(undefined, { | ||
| video: { | ||
| port: localPort, | ||
| ssrc, | ||
| srtp_key: request.video.srtp_key, | ||
| srtp_salt: request.video.srtp_salt, | ||
| }, | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| public handleStreamRequest(request: StreamingRequest, callback: StreamRequestCallback): void { | ||
| const { StreamRequestTypes } = this.platform.api.hap; | ||
| if (request.type === StreamRequestTypes.START) { | ||
| this.startStream(request, callback); | ||
| return; | ||
| } | ||
| if (request.type === StreamRequestTypes.STOP) { | ||
| this.stopSession(request.sessionID); | ||
| } | ||
| // RECONFIGURE is a no-op for a still-based source. | ||
| callback(); | ||
| } | ||
|
|
||
| private startStream(request: StartStreamRequest, callback: StreamRequestCallback): void { | ||
| const session = this.sessions.get(request.sessionID); | ||
| const snapshot = this.client.latestSnapshot; | ||
| if (!session) { | ||
| callback(new Error('No prepared session')); | ||
| return; | ||
| } | ||
| if (!snapshot) { | ||
| this.platform.log.warn('June camera has no frame yet (no active cook) — cannot start live stream.'); | ||
| callback(new Error('No camera frame available')); | ||
| return; | ||
| } | ||
| const { video } = request; | ||
| const args = [ | ||
| '-loglevel', 'error', | ||
| '-loop', '1', '-re', '-i', snapshot.url, |
There was a problem hiding this comment.
🟡 SSRF via unvalidated snapshot URL from WebSocket data (security)
In src/accessories/camera.ts, the snapshot.url value extracted from June WebSocket frames (message_code 10011 via parseCameraFrame in src/june-client.ts) is used without any validation in two dangerous sinks: (1) fetch(snapshot.url) at line 88 to retrieve the snapshot image, and (2) '-i', snapshot.url at line 152 passed to ffmpeg as input. Since the URL comes from external WebSocket data, an attacker who compromises that data stream could inject URLs targeting internal services (e.g., http://169.254.169.254/latest/meta-data/ for AWS metadata), local files (file:///etc/passwd), or internal network hosts. Neither the scheme nor the host are validated against expected June CDN/S3 domains.
💡 Suggestion: Validate the snapshot URL before use: ensure it uses the https:// scheme and its hostname matches an expected June image CDN domain or S3 bucket. Reject any URL with unexpected schemes (file://, http:// on non-local networks) or suspicious hostnames (localhost, 169.254.x.x, 10.x.x.x, etc.). The most targeted fix is to add validation in parseCameraFrame in src/june-client.ts so both call sites are protected.
📋 Prompt for AI Agents
In src/june-client.ts, modify the parseCameraFrame function (around lines 39-46) to validate the extracted URL before returning it: import the url module from Node.js, parse the candidate URL, reject on non-https schemes, and reject hostnames that resolve to private/localhost/multicast IPs. If validation fails, return null (which the camera will handle via the existing placeholder fallback). This protects both the fetch() sink in camera.ts:88 and the ffmpeg -i sink in camera.ts:152 with a single check at the parsing boundary.
| private async setOn(subtype: string, value: CharacteristicValue): Promise<void> { | ||
| const entry = this.services.get(subtype); | ||
| if (!entry) { | ||
| return; | ||
| } | ||
| const status = value | ||
| ? await this.client.startMode(entry.mode.primitiveType, entry.mode.tempF) | ||
| : await this.client.cancel(); | ||
| if (status !== 'success') { | ||
| entry.service.updateCharacteristic(this.platform.Characteristic.On, !value); | ||
| this.platform.log.warn(`June rejected ${entry.mode.label} command: ${status || 'no ack'}`); | ||
| return; | ||
| } | ||
| if (value) { | ||
| this.setAllOff(subtype); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Race condition between telemetry-driven setAllOff() and user-driven setOn() in mode switches (bug)
JuneModeSwitchAccessory registers handlers for both telemetry events (line 29) and HomeKit characteristic onSet callbacks (line 26). The setOn() method is async and awaits client.startMode() or client.cancel() at lines 52-53, yielding to the event loop. While suspended, a telemetry event carrying active===false (e.g., from a stale 10018 device-state frame) can fire synchronously via EventEmitter, invoking update() (line 32) which calls setAllOff() (line 34), writing Characteristic.On=false to every mode switch. When setOn() resumes after the command ack, the switch state it expects may have already been mutated by the telemetry handler. In particular: if the user turns ON mode A, telemetry sets it OFF during the await, then setOn succeeds and calls setAllOff(subtype) — restoring other switches but leaving A OFF even though the cook is active.
💡 Suggestion: Add a per-accessory guard flag to serialize the update() and setOn() critical sections. For example, use a commandInFlight boolean that setOn() sets before the await and clears in a finally block; update() checks this flag and defers state changes until the command completes.
📋 Prompt for AI Agents
In src/accessories/mode-switch.ts, protect the shared switch state:
- Add private fields:
private commandInFlight = false;andprivate pendingActiveState: boolean | undefined; - In
setOn(), wrap the async body: setcommandInFlight = truebefore the await, and in afinallyblock setcommandInFlight = falsethen apply any stashedpendingActiveState. - In
update(), whencommandInFlightis true, stashtelemetry.activeinpendingActiveStateinstead of callingsetAllOff().
This serializes access without blocking the event loop.
08a4c5f to
7f55f04
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7f55f04 to
57f6138
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
… the custom Config UI The plugin uses a custom Config UI (customUi:true), so the schema 'form' is ignored — the new opt-in features were not editable in the UI. Adds per-oven controls (doorbell + triggers, camera + ffmpeg path, probe sensor names, and a dynamic cook-mode list) matching the existing card pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@homebridge-ui/public/index.html`:
- Around line 288-297: The temperature field in modeRowHtml is missing
client-side bounds even though the config schema for tempF enforces a 100–550
range. Update the field-mode-temp input to include matching min and max
attributes so the UI validates the same constraints as the schema. Use the
modeRowHtml helper and the field-mode-temp control as the reference points when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a885586-c4f3-4a93-976c-5f7ab2273426
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
homebridge-ui/public/index.htmlpackage.json
✅ Files skipped from review due to trivial changes (1)
- package.json
| function modeRowHtml(mode, modeIndex) { | ||
| return ` | ||
| <div class="mode-row" data-mode-index="${modeIndex}" style="display:flex; gap:0.5rem; margin-bottom:0.5rem;"> | ||
| <input type="text" class="form-control field-mode-label" placeholder="Name (e.g. Broil)" value="${escapeHtml(mode.label ?? '')}"> | ||
| <input type="text" class="form-control field-mode-type" placeholder="Mode id (e.g. broil)" value="${escapeHtml(mode.primitiveType ?? '')}"> | ||
| <input type="number" class="form-control field-mode-temp" placeholder="°F" style="max-width:6rem;" value="${mode.tempF ?? 350}"> | ||
| <button class="btn btn-sm btn-outline-danger remove-mode" type="button">×</button> | ||
| </div>`; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add min/max attributes to the temperature input to match schema bounds.
The config schema defines tempF with "minimum": 100, "maximum": 550, but the field-mode-temp input has no min/max attributes, allowing out-of-range values. Adding them provides client-side validation consistent with the schema contract.
🛡️ Proposed fix
<div class="mode-row" data-mode-index="${modeIndex}" style="display:flex; gap:0.5rem; margin-bottom:0.5rem;">
<input type="text" class="form-control field-mode-label" placeholder="Name (e.g. Broil)" value="${escapeHtml(mode.label ?? '')}">
<input type="text" class="form-control field-mode-type" placeholder="Mode id (e.g. broil)" value="${escapeHtml(mode.primitiveType ?? '')}">
- <input type="number" class="form-control field-mode-temp" placeholder="°F" style="max-width:6rem;" value="${mode.tempF ?? 350}">
+ <input type="number" class="form-control field-mode-temp" placeholder="°F" min="100" max="550" style="max-width:6rem;" value="${mode.tempF ?? 350}">
<button class="btn btn-sm btn-outline-danger remove-mode" type="button">×</button>📝 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.
| function modeRowHtml(mode, modeIndex) { | |
| return ` | |
| <div class="mode-row" data-mode-index="${modeIndex}" style="display:flex; gap:0.5rem; margin-bottom:0.5rem;"> | |
| <input type="text" class="form-control field-mode-label" placeholder="Name (e.g. Broil)" value="${escapeHtml(mode.label ?? '')}"> | |
| <input type="text" class="form-control field-mode-type" placeholder="Mode id (e.g. broil)" value="${escapeHtml(mode.primitiveType ?? '')}"> | |
| <input type="number" class="form-control field-mode-temp" placeholder="°F" style="max-width:6rem;" value="${mode.tempF ?? 350}"> | |
| <button class="btn btn-sm btn-outline-danger remove-mode" type="button">×</button> | |
| </div>`; | |
| } | |
| function modeRowHtml(mode, modeIndex) { | |
| return ` | |
| <div class="mode-row" data-mode-index="${modeIndex}" style="display:flex; gap:0.5rem; margin-bottom:0.5rem;"> | |
| <input type="text" class="form-control field-mode-label" placeholder="Name (e.g. Broil)" value="${escapeHtml(mode.label ?? '')}"> | |
| <input type="text" class="form-control field-mode-type" placeholder="Mode id (e.g. broil)" value="${escapeHtml(mode.primitiveType ?? '')}"> | |
| <input type="number" class="form-control field-mode-temp" placeholder="°F" min="100" max="550" style="max-width:6rem;" value="${mode.tempF ?? 350}"> | |
| <button class="btn btn-sm btn-outline-danger remove-mode" type="button">×</button> | |
| </div>`; | |
| } |
🤖 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 `@homebridge-ui/public/index.html` around lines 288 - 297, The temperature
field in modeRowHtml is missing client-side bounds even though the config schema
for tempF enforces a 100–550 range. Update the field-mode-temp input to include
matching min and max attributes so the UI validates the same constraints as the
schema. Use the modeRowHtml helper and the field-mode-temp control as the
reference points when making the change.
CameraController advertised an AAC-ELD codec but prepareStream returned no audio block, so HAP threw 'Audio was enabled but not supplied in PrepareStreamResponse' on every stream request, crash-looping the child bridge. The oven cam is video-only; audio is optional in CameraStreamingOptions, so drop it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Previously ffmpeg looped the single still captured at stream-start, so the tapped-in live view was frozen. Now feed the latest 10011 frame into ffmpeg via image2pipe as new frames arrive (~1/s during a cook), so the live view updates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
The June oven has one probe. The telemetry array's {id:'left'} label implied a
possible 'right', but there's only one — the right sensor was dead weight.
Collapse to a single 'Food Probe' temperature sensor across client, accessory,
config, schema, and custom UI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/accessories/probe-sensor.ts (1)
29-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStill missing
setPropsto widenCurrentTemperaturevalid range.The previous review flagged that
CurrentTemperaturedefaults to 0–100°C per HAP, but food probes can exceed this (oven air, high-temp cooking, sensor faults). The refactor to a single sensor didn't add thesetPropscall. Values above 100°C will be clamped or rejected by HomeKit.🛡️ Proposed fix: widen CurrentTemperature range
this.service = this.accessory.getService(Service.TemperatureSensor) || this.accessory.addService(Service.TemperatureSensor); + this.service.getCharacteristic(Characteristic.CurrentTemperature).setProps({ minValue: -20, maxValue: 300 }); this.service.setCharacteristic(Characteristic.Name, cfg.name);🤖 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/accessories/probe-sensor.ts` around lines 29 - 36, The `ProbeSensor` setup still needs to widen the HomeKit `CurrentTemperature` range before publishing readings. In the constructor where `this.service` is created and `Characteristic.Name` is set, add the missing `setProps` call on `Characteristic.CurrentTemperature` so the `update` path can accept probe temperatures above the default 0–100°C range. Use the existing `this.service` initialization and `this.platform.Characteristic.CurrentTemperature` reference to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/accessories/probe-sensor.ts`:
- Around line 29-36: The `ProbeSensor` setup still needs to widen the HomeKit
`CurrentTemperature` range before publishing readings. In the constructor where
`this.service` is created and `Characteristic.Name` is set, add the missing
`setProps` call on `Characteristic.CurrentTemperature` so the `update` path can
accept probe temperatures above the default 0–100°C range. Use the existing
`this.service` initialization and
`this.platform.Characteristic.CurrentTemperature` reference to locate the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d99c45aa-21d2-4c50-a438-793131bd059a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
config.schema.jsonhomebridge-ui/public/index.htmlpackage.jsonsrc/accessories/camera.tssrc/accessories/probe-sensor.tssrc/june-client.test.tssrc/june-client.tssrc/protocol.test.tssrc/protocol.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/protocol.test.ts
- src/june-client.test.ts
- src/protocol.ts
- src/accessories/camera.ts
- homebridge-ui/public/index.html
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
omg.. founder of June here.. this is awesome! Somehow found this by accident right now running last30days on June Oven. THANKS FOR MAKING THIS I'm @mvanhorn on x |
|
you made it possible for me to make a printing press skill for this (claude code / hermes / open claw) THANK YOU! mvanhorn/printing-press-library#1475 |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-08-doorbell-probe-polish.md`:
- Line 13: Change the “Task 1: Remove Unsupported Door-Open Trigger” heading
from level 3 to level 2 so it follows the document’s H1 title sequentially and
satisfies MD001.
In `@docs/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md`:
- Around line 14-16: Rename the “Cook-done doorbell” feature throughout the
specification, including the related section at lines 88-92, to reflect that it
supports both ready and done triggers; use an oven/cook-event doorbell name or
explicitly identify the ready trigger as a preheat notification.
- Around line 122-127: Update the probe sensor specification to match the
confirmed left/right payload contract: define separate left and right sensors,
configuration names such as probeSensors.leftName and probeSensors.rightName,
and per-side telemetry fields and accessory subscriptions. If aggregation is
intended instead, explicitly document that behavior and its naming and telemetry
semantics rather than using singular probe 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba43c5f4-4e0a-4697-ad86-b18c839ba236
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdconfig.schema.jsondocs/superpowers/plans/2026-07-08-doorbell-probe-polish.mddocs/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.mdpackage.jsonsrc/accessories/doorbell.tssrc/platform.test.tssrc/platform.tssrc/protocol.test.tssrc/protocol.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src/protocol.test.ts
- src/platform.ts
- config.schema.json
- src/accessories/doorbell.ts
- src/protocol.ts
|
|
||
| --- | ||
|
|
||
| ### Task 1: Remove Unsupported Door-Open Trigger |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the Markdown heading hierarchy sequential.
### Task 1 skips directly from the H1 title to H3. Change task headings to ## (or add an H2 section) to satisfy MD001 and preserve document navigation.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 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/superpowers/plans/2026-07-08-doorbell-probe-polish.md` at line 13,
Change the “Task 1: Remove Unsupported Door-Open Trigger” heading from level 3
to level 2 so it follows the document’s H1 title sequentially and satisfies
MD001.
Source: Linters/SAST tools
| 1. **Cook-done doorbell** — ships as a Doorbell, and becomes a Video Doorbell when the interior | ||
| camera is also enabled. Offered as an option alongside the existing ready/done sensors; users can | ||
| enable either, both, or neither. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Rename the feature to reflect both triggers.
This is called “Cook-done doorbell,” but ready is also a first-class trigger and fires before cooking completes. Rename it to an oven/cook-event doorbell or explicitly describe the ready trigger as a preheat notification.
Also applies to: 88-92
🤖 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/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md`
around lines 14 - 16, Rename the “Cook-done doorbell” feature throughout the
specification, including the related section at lines 88-92, to reflect that it
supports both ready and done triggers; use an oven/cook-event doorbell name or
explicitly identify the ready trigger as a preheat notification.
| - Source: `10013` telemetry `sensor_data.probe` array from Spike B. The June oven has a single | ||
| food probe; the accessory keeps the last reported value between updates. | ||
| - Opt-in. Config (per oven): `probeSensors.enabled` (bool, default false). Optionally a | ||
| `probeSensors.name` display name. | ||
| - New `JuneTelemetry` fields (`probeC`, `probePresent`) populated in | ||
| `JuneClient.handleMessage` for `10013`; a `JuneProbeSensorAccessory` subscribes to telemetry. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document separate left/right probe sensors.
The confirmed payload is keyed by id: "left"|"right" and the PR objective calls for left/right HomeKit temperature sensors, but this section describes one probe, one probeSensors.name, and singular probeC/probePresent fields. Align the spec with the per-side contract (leftName/rightName and per-side telemetry), or explicitly document an intentional aggregation. This overlaps the earlier probe-configuration review comment.
🤖 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/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md`
around lines 122 - 127, Update the probe sensor specification to match the
confirmed left/right payload contract: define separate left and right sensors,
configuration names such as probeSensors.leftName and probeSensors.rightName,
and per-side telemetry fields and accessory subscriptions. If aggregation is
intended instead, explicitly document that behavior and its naming and telemetry
semantics rather than using singular probe fields.
Adds five opt-in HomeKit features (all default off — no behavior change for existing installs), driven from the Config UI. Protocol details for each were confirmed against a real oven during development.
Features
10011, ~1 fps, 640×480 JPEG). Snapshot works with no extra setup; tap-to-view live streaming spawns systemffmpeg(configurable path) when present and degrades gracefully otherwise.done/readytriggers. (doorOpentoggle exists but is a no-op — no confirmed oven signal.)sensor_data.probe[]as HomeKit Temperature Sensors, for "notify at X°F" automations.primitive_type+ temp); any mode the oven accepts. Mutually exclusive.Plus a README "Not exposed (and why)" section for the intentionally-omitted timer and cook-progress %.
Verified live against a real oven
startMode/cancelcommand round-trips → acksuccess10011→latestSnapshot→ HTTP 200 → valid JPEG10011cadence/format (1 fps still JPEG) and probe telemetry format (sensor_data.probe[]) both captured and matched by the parsersKnown limitations
Design + plan:
docs/superpowers/specs/2026-07-08-june-expanded-homekit-features-design.md,docs/superpowers/plans/2026-07-08-expanded-homekit-features.md. Lint clean; 16 tests passing.🤖 Generated with Claude Code
Summary by CodeRabbit