feat: environment sensors, WiFi status, and RAW file management#162
Merged
Conversation
Parse bedTemp/frzTemp CBOR records from RAW files into new biometrics DB tables, expose via tRPC with unit conversion. Add WiFi signal strength endpoint and RAW file list/download/delete API for disk management. - New DB tables: bed_temp (zone temps, humidity), freezer_temp (water temps) - Python module: environment-monitor (60s downsampled writes, 32MB/10% CPU) - Environment router: date-range queries, latest, summary with F/C conversion - System router: wifiStatus via nmcli with dev fallback - Raw router: file listing, safe delete (protects active file), disk usage - Next.js route: streaming binary download with path traversal protection - Deploy: install script updated for environment-monitor module
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an environment-monitor Python service that ingests CBOR RAW sensor files into biometrics.db, new DB schema/migrations and temp utilities, Next/TS API routes and TRPC routers for environment and raw-file management, a systemd unit and install updates, and a Next.js app route to stream raw files. Changes
Sequence Diagram(s)sequenceDiagram
participant Python as "Environment Monitor\n(Python)"
participant Files as "/persistent\nRAW Files"
participant DB as "biometrics.db\n(SQLite)"
participant Health as "sleepypod.db\n(System Health)"
Files->>Python: RawFileFollower streams CBOR records
Python->>Python: filter by type, apply 60s cadence
Python->>DB: write_bed_temp / write_freezer_temp (transaction)
DB-->>Python: insert ok
Python->>Health: report_health(status)
Health-->>Python: ack
sequenceDiagram
actor Client
participant API as "TRPC API / Next App"
participant FS as "Filesystem (/persistent)"
participant Exec as "Local cmd (df/nmcli)"
participant DB as "biometrics.db"
Client->>API: GET /raw/files
API->>FS: readdir + stat *.RAW
FS-->>API: file list & sizes
API-->>Client: sorted files
Client->>API: POST /raw/files/delete {filename}
API->>API: validate SAFE_FILENAME & prevent traversal
API->>FS: unlink file
FS-->>API: success/error
API-->>Client: {deleted, message}
Client->>API: GET /raw/disk-usage
API->>Exec: df -B1 RAW_DIR
Exec-->>API: total/used/avail
API->>FS: sum RAW file sizes
API-->>Client: usage object
Client->>API: GET /environment/bed-temp
API->>DB: query bed_temp (timestamp range)
DB-->>API: rows
API->>API: convert temps/humidity
API-->>Client: converted readings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/raw/`[filename]/route.ts:
- Around line 21-23: The path check only validates the resolved string but
doesn't prevent symlinks; before calling createReadStream() in this route
handler, call fs.promises.lstat(resolved) and reject if lstat.isSymbolicLink()
is true, and also call fs.promises.realpath(resolved) and
fs.promises.realpath(RAW_DIR) and ensure the canonical file path startsWith the
canonical RAW_DIR; if either check fails, return the 400 "Path traversal
detected" response. Update the logic around the current usage of resolved /
RAW_DIR / filename and perform these checks prior to opening the stream.
- Around line 39-40: The catch block that currently returns NextResponse.json({
error: 'File not found' }, { status: 404 }) unconditionally should distinguish
ENOENT from other I/O errors: inside the catch for the file read in the route
handler (the try/await that reads the file), examine the caught error's code
(e.g., err.code === 'ENOENT') and only return a 404 for ENOENT; for all other
errors return a 500 (NextResponse.json with status 500) and include the actual
error message/details in the response or log so storage/permission faults are
surfaced (reference the catch block and the NextResponse.json calls in
route.ts).
In `@modules/environment-monitor/main.py`:
- Around line 160-181: The follower currently resets last_bed_write and
last_frz_write to 0 on restart causing historical records to be reinserted;
before entering the for record in follower.read_records() loop, query the DB
(using db_conn from open_biometrics_db) for the most recent timestamp per stream
(e.g., SELECT MAX(ts) FROM bed_temp and freezer_temp) and seed last_bed_write
and last_frz_write with those values (or with time.time() if no rows), so
RawFileFollower/ DOWNSAMPLE_INTERVAL_S logic and functions
write_bed_temp/write_freezer_temp will skip already-ingested samples; place this
seeding immediately after report_health(...) and before creating/consuming
follower.read_records() to make restarts safe.
In `@modules/environment-monitor/sleepypod-environment-monitor.service`:
- Line 9: The service unit currently runs as root (line containing "User=root");
change the unit to run under a dedicated non-root account (e.g., create or
reference a service user like sleepypod-monitor and replace "User=root" with
that username in sleepypod-environment-monitor.service), ensure that the account
exists (systemd --user or system user via useradd/systemd-sysusers), and set
ownership/permissions so the user has read on /persistent and read/write on
/persistent/sleepypod-data (chown/chmod as needed). Also validate the service
can start under the new user and update any ExecStart or post-start hooks that
assume root privileges.
In `@scripts/install`:
- Around line 502-504: The update flow currently only installs
"environment-monitor" during full install but not during the sp-update path;
modify the sp-update branch in scripts/install to mirror the full-install steps:
ensure the modules directory is synced (the logic that updates
/opt/sleepypod/modules), call install_module "environment-monitor" (same
function used for "piezo-processor" etc.) so its venv gets created, and
enable/start the sleepypod-environment-monitor.service (same service enablement
pattern used for other modules) so in-place updates receive the new monitor.
In `@src/db/biometrics-migrations/0002_mushy_loki.sql`:
- Around line 1-25: The schema allows duplicate timestamps because `bed_temp`
and `freezer_temp` only have non-unique indexes and the writer
(modules/environment-monitor/main.py using last_bed_write/last_frz_write and
RawFileFollower) can replay old samples on restart; fix by making timestamp
unique or using upsert semantics: add a UNIQUE constraint on the `timestamp`
column for both `bed_temp` and `freezer_temp` (e.g. ALTER TABLE / create table
with UNIQUE(timestamp)), or change the writer to use deterministic upsert logic
(INSERT ... ON CONFLICT(timestamp) DO UPDATE ... or SQLite’s INSERT OR REPLACE)
when inserting into `bed_temp` and `freezer_temp` so duplicate timestamps are
de-duplicated instead of inserted (referencing tables `bed_temp`, `freezer_temp`
and index `idx_bed_temp_timestamp` / `idx_freezer_temp_timestamp` for location).
In `@src/server/routers/raw.ts`:
- Around line 58-60: The deleteFile mutation (deleteFile) is currently
registered with publicProcedure and meta openapi protect: false, which exposes
file deletion of RAW_DATA_DIR to unauthenticated clients; change its
registration to use the authenticated/admin middleware instead of
publicProcedure (e.g., replace publicProcedure with the appropriate
adminProcedure or protectedProcedure used elsewhere), update the meta openapi
protect flag to true (or remove protect:false), and ensure the input validation
remains the same so only authorized users can call deleteFile against
RAW_DATA_DIR.
In `@src/server/routers/system.ts`:
- Around line 166-175: The nmcli parsing using active.split(':') misparses
escaped colons in SSIDs; update the parsing around the execFileAsync call and
the active/parts handling to split on unescaped colons and then unescape
sequences (turn '\:' → ':' and '\\' → '\') for each field before returning;
implement a small delimiter-aware parser that iterates the active string
building fields when a non-escaped ':' is seen (or use a CSV-style parser
configured with delimiter ':' and escape '\\'), then set ssid =
unescapedField[1] and signal = Number(unescapedField[2]) || null to preserve the
declared return types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f1ccda2-b25a-499f-b3f6-ab5596807a0f
📒 Files selected for processing (16)
app/api/raw/[filename]/route.tsmodules/environment-monitor/main.pymodules/environment-monitor/manifest.jsonmodules/environment-monitor/requirements.txtmodules/environment-monitor/sleepypod-environment-monitor.servicescripts/installsrc/db/biometrics-migrations/0002_mushy_loki.sqlsrc/db/biometrics-migrations/meta/0002_snapshot.jsonsrc/db/biometrics-migrations/meta/_journal.jsonsrc/db/biometrics-schema.tssrc/lib/tempUtils.tssrc/server/openapi.tssrc/server/routers/app.tssrc/server/routers/environment.tssrc/server/routers/raw.tssrc/server/routers/system.ts
|
|
||
| [Service] | ||
| Type=simple | ||
| User=root |
There was a problem hiding this comment.
Run the monitor under a non-root identity.
This process only needs read access to /persistent and write access to /persistent/sleepypod-data. Keeping it on User=root means any bug in the CBOR ingestion path lands with full-root privileges on the box instead of a narrowly scoped service account.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/environment-monitor/sleepypod-environment-monitor.service` at line 9,
The service unit currently runs as root (line containing "User=root"); change
the unit to run under a dedicated non-root account (e.g., create or reference a
service user like sleepypod-monitor and replace "User=root" with that username
in sleepypod-environment-monitor.service), ensure that the account exists
(systemd --user or system user via useradd/systemd-sysusers), and set
ownership/permissions so the user has read on /persistent and read/write on
/persistent/sleepypod-data (chown/chmod as needed). Also validate the service
can start under the new user and update any ExecStart or post-start hooks that
assume root privileges.
Comment on lines
+58
to
+60
| deleteFile: publicProcedure | ||
| .meta({ openapi: { method: 'POST', path: '/raw/files/delete', protect: false, tags: ['Raw'] } }) | ||
| .input(z.object({ filename: z.string() }).strict()) |
There was a problem hiding this comment.
Do not expose file deletion as a public procedure.
This mutation deletes files from RAW_DATA_DIR, yet it is mounted under publicProcedure with protect: false. Combined with the public RAW listing/download surface, any reachable client can remove historical sensor data with no auth. Please put deleteFile behind admin/auth middleware instead of shipping it as a public endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/routers/raw.ts` around lines 58 - 60, The deleteFile mutation
(deleteFile) is currently registered with publicProcedure and meta openapi
protect: false, which exposes file deletion of RAW_DATA_DIR to unauthenticated
clients; change its registration to use the authenticated/admin middleware
instead of publicProcedure (e.g., replace publicProcedure with the appropriate
adminProcedure or protectedProcedure used elsewhere), update the meta openapi
protect flag to true (or remove protect:false), and ensure the input validation
remains the same so only authorized users can call deleteFile against
RAW_DATA_DIR.
- nmcli parser: handle escaped colons in SSIDs - raw download: reject symlinks, verify canonical path - raw delete: mark as protected endpoint - Python module: INSERT OR IGNORE + seed cursors from DB on restart - Schema: unique indexes on timestamp (dedup guarantee) - Regenerate clean 0002 migration - CI build workflow: trigger on all branches, not just main - Service: run as dac user instead of root
- UNIQUE indexes on bed_temp.timestamp and freezer_temp.timestamp (migration SQL + drizzle schema) to prevent duplicate rows - Sync biometrics modules in sp-update flow so OTA updates install/restart piezo-processor, sleep-detector, environment-monitor - (Already in PR) symlink check + realpath validation in RAW route - (Already in PR) ENOENT vs 500 error differentiation - (Already in PR) INSERT OR IGNORE + DB-seeded cursors for restart safety - (Already in PR) deleteFile protect:true, nmcli escape-aware parsing - (Already in PR) User=dac in environment-monitor service Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit 6f928db.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/server/routers/raw.ts (1)
58-60:⚠️ Potential issue | 🟠 Major
deleteFileis still exposed through a public procedure.
protect: truein OpenAPI metadata does not enforce runtime auth. Because this mutation is onpublicProcedure, unauthenticated callers can still delete RAW files. Move this to your authenticated/admin procedure middleware.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routers/raw.ts` around lines 58 - 60, The deleteFile procedure is defined with publicProcedure which allows unauthenticated callers despite openapi meta protect: change the procedure factory from publicProcedure to your authenticated middleware (e.g. protectedProcedure or adminProcedure used in the codebase) for the deleteFile export so runtime auth is enforced; keep the same .meta({...}), .input(z.object({ filename: z.string() }).strict()), and handler body, but replace the publicProcedure reference for the deleteFile symbol so only authenticated/admin users can invoke it.
🧹 Nitpick comments (2)
src/server/routers/raw.ts (1)
67-70: Harden delete path validation to canonical-path checks (consistent with download route).The
startsWith(path.resolve(RAW_DIR))guard is brittle as a long-term boundary check. Use canonical path verification (and reject symlinks) likeapp/api/raw/[filename]/route.tsfor consistency and defense-in-depth.Suggested hardening patch
-import { readdir, stat, unlink } from 'node:fs/promises' +import { lstat, readdir, realpath, stat, unlink } from 'node:fs/promises' ... const resolved = path.resolve(RAW_DIR, input.filename) - if (!resolved.startsWith(path.resolve(RAW_DIR))) { - throw new TRPCError({ code: 'BAD_REQUEST', message: 'Path traversal detected' }) - } + const lstats = await lstat(resolved) + if (lstats.isSymbolicLink()) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'Path traversal detected' }) + } + + const canonicalFile = await realpath(resolved) + const canonicalDir = await realpath(RAW_DIR) + const rel = path.relative(canonicalDir, canonicalFile) + if (rel.startsWith('..') || path.isAbsolute(rel)) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'Path traversal detected' }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routers/raw.ts` around lines 67 - 70, Replace the brittle startsWith check in the delete logic (where you compute resolved = path.resolve(RAW_DIR, input.filename)) with the same canonical-path and symlink rejection used by app/api/raw/[filename]/route.ts: resolve both RAW_DIR and the target via fs.realpath (or fs.promises.realpath), ensure the real target path starts with the real RAW_DIR, and use fs.lstat to reject symbolic links (isSymbolicLink) before proceeding; update the TRPCError to trigger on failure of these canonical checks so deletions cannot escape the RAW_DIR via symlinks or crafted paths.modules/environment-monitor/main.py (1)
144-145: Narrow the exception catch to database-specific errors.This function writes to a SQLite database. Catching
Exceptionmasks unexpected errors and weakens diagnostics. Usesqlite3.Errorto catch only database-related failures.Affected code
except Exception as e: log.warning("Could not write health status: %s", e)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/environment-monitor/main.py` around lines 144 - 145, The generic except in the block that writes health status should be narrowed to database errors: replace "except Exception as e:" with "except sqlite3.Error as e:" (and add "import sqlite3" at top if missing) so only SQLite-related failures are caught and logged via log.warning("Could not write health status: %s", e); do not swallow other exceptions—allow them to propagate or handle them separately if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 3-10: The workflow currently grants write scope globally via the
top-level "permissions: contents: write"; change the top-level permissions to
the least-privileged setting (e.g., "permissions: contents: read" or remove the
write blanket) and move the write privilege into a dedicated "release" job that
runs only for tagged pushes (use the existing condition "if:
startsWith(github.ref, 'refs/tags/v')" on that job). Ensure normal build and PR
jobs (triggered by "on: push" and "on: pull_request") run with read-only content
permissions while the release job declares "permissions: contents: write" so
only the release job receives a write-scoped GITHUB_TOKEN.
In `@modules/environment-monitor/main.py`:
- Around line 160-171: The startup opens the DB and seeds cursors
(open_biometrics_db, RawFileFollower, last_bed_write/last_frz_write) outside the
main try, so failures can skip report_health("down", ...) and the finally
cleanup; move the DB open, RawFileFollower instantiation, and cursor seeding
into the existing try block that contains the main run logic, ensure
report_health("healthy", ...) remains after successful init, and in the finally
block guard closure with a check like "if db_conn is not None" before closing so
a partially-initialized failure won't raise on cleanup; also ensure
RawFileFollower is cleaned up only if it was successfully created.
- Around line 174-186: The loop over follower.read_records assumes each record
is a dict with a numeric "ts" which lets malformed CBOR crash the monitor; add
defensive validation inside the loop so malformed records are skipped: check
that record is a dict, that record.get("type") returns a string, and that "ts"
exists or can be parsed to a float (use try/except around float(...) and fall
back to time.time()), and only then proceed to the existing branches that call
write_bed_temp or write_freezer_temp and update last_bed_write/last_frz_write;
on validation failure log a warning and continue to the next record rather than
letting the exception propagate.
In `@src/db/biometrics-migrations/0002_fuzzy_strong_guy.sql`:
- Around line 1-25: The migration file 0002_fuzzy_strong_guy.sql duplicates
objects already created by 0002_mushy_loki.sql (tables bed_temp, freezer_temp
and indexes idx_bed_temp_timestamp, idx_freezer_temp_timestamp), causing
conflicts; either remove the CREATE TABLE/CREATE INDEX statements from
0002_fuzzy_strong_guy.sql and transform it into an ALTER/index-only migration
that applies only necessary deltas, or delete/merge this duplicate so only one
canonical 0002 migration defines bed_temp/freezer_temp and their indexes; after
choosing the canonical path, update the migration journal/snapshot to reflect
the single 0002 migration.
In `@src/server/routers/raw.ts`:
- Around line 101-103: The call to execFileAsync('df', ['-B1', RAW_DIR]) can
hang; update the execFileAsync invocation in the same block (where dfOut, dfLine
and dfParts are produced) to pass explicit options including a timeout (e.g.
5s–10s) and a maxBuffer (e.g. 10MB) like execFileAsync('df', ['-B1', RAW_DIR], {
timeout: 5000, maxBuffer: 10 * 1024 * 1024 }), and ensure the surrounding
try/catch correctly handles a timeout error from execFileAsync so
dfOut/dfLine/dfParts are not used when the subprocess times out.
In `@src/server/routers/system.ts`:
- Around line 187-191: The code returns signal: fields[2] ? Number(fields[2]) :
null which can produce NaN and violate the z.number().nullable() contract;
change it to parse and validate the numeric value (e.g., const parsed =
Number(fields[2]); use Number.isFinite(parsed) ? parsed : null) so that
non-numeric tokens yield null instead of NaN; update the return object (the
signal field) accordingly where connected/ssid/signal are constructed.
---
Duplicate comments:
In `@src/server/routers/raw.ts`:
- Around line 58-60: The deleteFile procedure is defined with publicProcedure
which allows unauthenticated callers despite openapi meta protect: change the
procedure factory from publicProcedure to your authenticated middleware (e.g.
protectedProcedure or adminProcedure used in the codebase) for the deleteFile
export so runtime auth is enforced; keep the same .meta({...}),
.input(z.object({ filename: z.string() }).strict()), and handler body, but
replace the publicProcedure reference for the deleteFile symbol so only
authenticated/admin users can invoke it.
---
Nitpick comments:
In `@modules/environment-monitor/main.py`:
- Around line 144-145: The generic except in the block that writes health status
should be narrowed to database errors: replace "except Exception as e:" with
"except sqlite3.Error as e:" (and add "import sqlite3" at top if missing) so
only SQLite-related failures are caught and logged via log.warning("Could not
write health status: %s", e); do not swallow other exceptions—allow them to
propagate or handle them separately if necessary.
In `@src/server/routers/raw.ts`:
- Around line 67-70: Replace the brittle startsWith check in the delete logic
(where you compute resolved = path.resolve(RAW_DIR, input.filename)) with the
same canonical-path and symlink rejection used by
app/api/raw/[filename]/route.ts: resolve both RAW_DIR and the target via
fs.realpath (or fs.promises.realpath), ensure the real target path starts with
the real RAW_DIR, and use fs.lstat to reject symbolic links (isSymbolicLink)
before proceeding; update the TRPCError to trigger on failure of these canonical
checks so deletions cannot escape the RAW_DIR via symlinks or crafted paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 554400e9-4aaa-4e22-a743-7e4f5fea924b
📒 Files selected for processing (10)
.github/workflows/build.ymlapp/api/raw/[filename]/route.tsmodules/environment-monitor/main.pymodules/environment-monitor/sleepypod-environment-monitor.servicesrc/db/biometrics-migrations/0002_fuzzy_strong_guy.sqlsrc/db/biometrics-migrations/meta/0002_snapshot.jsonsrc/db/biometrics-migrations/meta/_journal.jsonsrc/db/biometrics-schema.tssrc/server/routers/raw.tssrc/server/routers/system.ts
✅ Files skipped from review due to trivial changes (1)
- src/db/biometrics-migrations/meta/0002_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/db/biometrics-migrations/meta/_journal.json
- modules/environment-monitor/sleepypod-environment-monitor.service
- app/api/raw/[filename]/route.ts
Comment on lines
3
to
10
| on: | ||
| push: | ||
| branches: [main] | ||
| branches: ['**'] | ||
| tags: ['v*'] | ||
| pull_request: | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/build.ymlRepository: sleepypod/core
Length of output: 1949
Reduce token scope—write permission unnecessary for most builds.
The workflow currently has permissions: contents: write at the workflow level, which grants write-capable GITHUB_TOKEN to all steps across all branch pushes and PRs to main/dev. Although the release step is conditionally gated to tags with if: startsWith(github.ref, 'refs/tags/v'), the write token is still unnecessarily available to all build steps. This violates least-privilege—release is the only operation requiring write access.
🔒 Suggested hardening (split release into a write-scoped job)
name: Build & Package
on:
push:
branches: ['**']
tags: ['v*']
pull_request:
+ branches: [main, dev]
permissions:
- contents: write
+ contents: read
jobs:
build:
runs-on: ubuntu-latest
steps:
@@
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: sleepypod-core
path: sleepypod-core.tar.gz
retention-days: 30
- # Create GitHub Release with deploy tarball on tags
- - name: Create Release
- if: startsWith(github.ref, 'refs/tags/v')
- uses: softprops/action-gh-release@v2
- with:
- files: sleepypod-core.tar.gz
- generate_release_notes: true
+ release:
+ if: startsWith(github.ref, 'refs/tags/v')
+ needs: build
+ runs-on: ubuntu-latest
+ permissions:
+ contents: write
+ steps:
+ - name: Download artifact
+ uses: actions/download-artifact@v4
+ with:
+ name: sleepypod-core
+
+ - name: Create Release
+ uses: softprops/action-gh-release@v2
+ with:
+ files: sleepypod-core.tar.gz
+ generate_release_notes: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build.yml around lines 3 - 10, The workflow currently
grants write scope globally via the top-level "permissions: contents: write";
change the top-level permissions to the least-privileged setting (e.g.,
"permissions: contents: read" or remove the write blanket) and move the write
privilege into a dedicated "release" job that runs only for tagged pushes (use
the existing condition "if: startsWith(github.ref, 'refs/tags/v')" on that job).
Ensure normal build and PR jobs (triggered by "on: push" and "on: pull_request")
run with read-only content permissions while the release job declares
"permissions: contents: write" so only the release job receives a write-scoped
GITHUB_TOKEN.
Comment on lines
+160
to
+171
| db_conn = open_biometrics_db() | ||
| follower = RawFileFollower(RAW_DATA_DIR, _shutdown, poll_interval=0.5) | ||
|
|
||
| # Seed cursors from DB so restarts don't replay already-ingested samples | ||
| last_bed_write = float( | ||
| db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM bed_temp").fetchone()[0] or 0 | ||
| ) | ||
| last_frz_write = float( | ||
| db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM freezer_temp").fetchone()[0] or 0 | ||
| ) | ||
|
|
||
| report_health("healthy", "environment-monitor started") |
There was a problem hiding this comment.
Startup failures can bypass health-down reporting and deterministic cleanup.
open_biometrics_db() and cursor seeding run before the try block. If either fails, the module exits without report_health("down", ...), and cleanup logic in finally is skipped. Wrap initialization in the main try and guard connection close.
Suggested restructuring
def main() -> None:
@@
- db_conn = open_biometrics_db()
- follower = RawFileFollower(RAW_DATA_DIR, _shutdown, poll_interval=0.5)
-
- # Seed cursors from DB so restarts don't replay already-ingested samples
- last_bed_write = float(
- db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM bed_temp").fetchone()[0] or 0
- )
- last_frz_write = float(
- db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM freezer_temp").fetchone()[0] or 0
- )
-
- report_health("healthy", "environment-monitor started")
-
- try:
+ db_conn: sqlite3.Connection | None = None
+ try:
+ db_conn = open_biometrics_db()
+ follower = RawFileFollower(RAW_DATA_DIR, _shutdown, poll_interval=0.5)
+
+ last_bed_write = float(
+ db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM bed_temp").fetchone()[0] or 0
+ )
+ last_frz_write = float(
+ db_conn.execute("SELECT COALESCE(MAX(timestamp), 0) FROM freezer_temp").fetchone()[0] or 0
+ )
+
+ report_health("healthy", "environment-monitor started")
for record in follower.read_records():
@@
except Exception as e:
log.exception("Fatal error in main loop: %s", e)
report_health("down", str(e))
sys.exit(1)
finally:
- db_conn.close()
+ if db_conn is not None:
+ db_conn.close()
log.info("Shutdown complete")Also applies to: 188-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/environment-monitor/main.py` around lines 160 - 171, The startup
opens the DB and seeds cursors (open_biometrics_db, RawFileFollower,
last_bed_write/last_frz_write) outside the main try, so failures can skip
report_health("down", ...) and the finally cleanup; move the DB open,
RawFileFollower instantiation, and cursor seeding into the existing try block
that contains the main run logic, ensure report_health("healthy", ...) remains
after successful init, and in the finally block guard closure with a check like
"if db_conn is not None" before closing so a partially-initialized failure won't
raise on cleanup; also ensure RawFileFollower is cleaned up only if it was
successfully created.
Comment on lines
+1
to
+25
| CREATE TABLE `bed_temp` ( | ||
| `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL, | ||
| `timestamp` integer NOT NULL, | ||
| `ambient_temp` integer, | ||
| `mcu_temp` integer, | ||
| `humidity` integer, | ||
| `left_outer_temp` integer, | ||
| `left_center_temp` integer, | ||
| `left_inner_temp` integer, | ||
| `right_outer_temp` integer, | ||
| `right_center_temp` integer, | ||
| `right_inner_temp` integer | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE UNIQUE INDEX `idx_bed_temp_timestamp` ON `bed_temp` (`timestamp`);--> statement-breakpoint | ||
| CREATE TABLE `freezer_temp` ( | ||
| `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL, | ||
| `timestamp` integer NOT NULL, | ||
| `ambient_temp` integer, | ||
| `heatsink_temp` integer, | ||
| `left_water_temp` integer, | ||
| `right_water_temp` integer | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE UNIQUE INDEX `idx_freezer_temp_timestamp` ON `freezer_temp` (`timestamp`); No newline at end of file |
There was a problem hiding this comment.
Resolve duplicate 0002 migration that recreates existing tables.
This migration recreates bed_temp/freezer_temp, but src/db/biometrics-migrations/0002_mushy_loki.sql already creates the same tables (and same index names) without guards. Running both will break migration application with existing-object errors. Keep a single canonical 0002 path (and update journal/snapshot accordingly), or convert one path into an alter/index-mutation migration only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/biometrics-migrations/0002_fuzzy_strong_guy.sql` around lines 1 - 25,
The migration file 0002_fuzzy_strong_guy.sql duplicates objects already created
by 0002_mushy_loki.sql (tables bed_temp, freezer_temp and indexes
idx_bed_temp_timestamp, idx_freezer_temp_timestamp), causing conflicts; either
remove the CREATE TABLE/CREATE INDEX statements from 0002_fuzzy_strong_guy.sql
and transform it into an ALTER/index-only migration that applies only necessary
deltas, or delete/merge this duplicate so only one canonical 0002 migration
defines bed_temp/freezer_temp and their indexes; after choosing the canonical
path, update the migration journal/snapshot to reflect the single 0002
migration.
Comment on lines
+101
to
+103
| const { stdout: dfOut } = await execFileAsync('df', ['-B1', RAW_DIR]) | ||
| const dfLine = dfOut.trim().split('\n')[1] | ||
| const dfParts = dfLine?.split(/\s+/) ?? [] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the current call site lacks timeout/maxBuffer options.
rg -nP "execFileAsync\\(\\s*'df'\\s*,\\s*\\['-B1',\\s*RAW_DIR\\]\\s*\\)" src/server/routers/raw.tsRepository: sleepypod/core
Length of output: 139
🏁 Script executed:
# Check the context around line 101 in raw.ts
sed -n '95,110p' src/server/routers/raw.tsRepository: sleepypod/core
Length of output: 689
🏁 Script executed:
# Check if there are other execFileAsync calls in the file
rg -n "execFileAsync" src/server/routers/raw.tsRepository: sleepypod/core
Length of output: 183
🏁 Script executed:
# Search for execFileAsync calls across the codebase to see how timeouts are typically handled
rg -B2 -A2 "execFileAsync.*timeout" --type ts --type jsRepository: sleepypod/core
Length of output: 40
🏁 Script executed:
# Check the import statement for execFileAsync
rg -n "execFileAsync|import.*exec" src/server/routers/raw.ts | head -20Repository: sleepypod/core
Length of output: 231
🏁 Script executed:
# Find RAW_DIR definition
rg -n "RAW_DIR\s*=" src/server/routers/raw.tsRepository: sleepypod/core
Length of output: 118
🏁 Script executed:
# Check if there are server-level timeouts or middleware
rg -n "timeout|setTimeout" src/server/routers/raw.ts | head -10Repository: sleepypod/core
Length of output: 40
🏁 Script executed:
# Look for server configuration and middleware
fd "server\.(ts|js)" src/server/ | head -5Repository: sleepypod/core
Length of output: 40
Add timeout and buffer limits to the df subprocess call.
Line 101 executes an external filesystem command without timeout protection. If the df call stalls (e.g., NFS issues, hung filesystem), the request will hang indefinitely and consume server resources. The try-catch block only handles errors, not timeouts.
Suggested reliability patch
- const { stdout: dfOut } = await execFileAsync('df', ['-B1', RAW_DIR])
+ const { stdout: dfOut } = await execFileAsync('df', ['-B1', RAW_DIR], {
+ timeout: 2000,
+ maxBuffer: 1024 * 1024,
+ })📝 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 { stdout: dfOut } = await execFileAsync('df', ['-B1', RAW_DIR]) | |
| const dfLine = dfOut.trim().split('\n')[1] | |
| const dfParts = dfLine?.split(/\s+/) ?? [] | |
| const { stdout: dfOut } = await execFileAsync('df', ['-B1', RAW_DIR], { | |
| timeout: 2000, | |
| maxBuffer: 1024 * 1024, | |
| }) | |
| const dfLine = dfOut.trim().split('\n')[1] | |
| const dfParts = dfLine?.split(/\s+/) ?? [] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/routers/raw.ts` around lines 101 - 103, The call to
execFileAsync('df', ['-B1', RAW_DIR]) can hang; update the execFileAsync
invocation in the same block (where dfOut, dfLine and dfParts are produced) to
pass explicit options including a timeout (e.g. 5s–10s) and a maxBuffer (e.g.
10MB) like execFileAsync('df', ['-B1', RAW_DIR], { timeout: 5000, maxBuffer: 10
* 1024 * 1024 }), and ensure the surrounding try/catch correctly handles a
timeout error from execFileAsync so dfOut/dfLine/dfParts are not used when the
subprocess times out.
- raw delete: canonical path + symlink checks (matches download route) - raw diskUsage: 5s timeout on df subprocess - wifiStatus: NaN guard on signal parsing - Python module: narrow except to sqlite3.Error, skip malformed records
2 tasks
ng
added a commit
that referenced
this pull request
Mar 15, 2026
## Summary Follow-up to #162 — addresses findings from dual-model adversarial code review. - Delete orphaned migration `0002_mushy_loki.sql` (not in drizzle journal) - Re-throw `TRPCError` in `deleteFile` catch block (was swallowed as 500 instead of 400) - Fix `limit` schema: default(1440) exceeded max(1000) — raised max to 1440 - Parallelize `getSummary` queries with `Promise.all` - Add dev fallback for `df -B1` in `diskUsage` (GNU-only, fails on macOS) - Use `SAFE_FILENAME` filter in `listRawFiles` for consistency with download/delete - Add 18 unit tests for `tempUtils` (centiDegrees conversions, toF/toC, ensureF, mapToEightSleepScale) ## Test plan - [x] 947 tests pass (18 new) - [x] TypeScript compiles clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added environment monitoring to track bed zone and freezer temperatures with ambient, MCU, humidity, and water temperature sensors. * Added APIs for retrieving temperature data with configurable date ranges and unit conversion (Celsius/Fahrenheit). * Added raw file management endpoints for listing, deleting, and monitoring disk usage. * Added WiFi status endpoint to display connectivity and signal strength. * **Tests** * Added comprehensive unit tests for temperature conversion utilities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
8 tasks
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bedTemp+frzTempCBOR records from RAW files into newbed_tempandfreezer_tempbiometrics DB tables. Expose via 5 tRPC endpoints with F/C unit conversion (centidegrees stored raw, converted in API layer).wifiStatusendpoint on system router usingnmcliwith graceful dev fallback.environment-monitor— lightweight RAW file follower with 60s downsampling, 32MB memory / 10% CPU limits, cbor2 only (no numpy).New endpoints
/environment/bed-temp/environment/freezer-temp/environment/bed-temp/latest/environment/freezer-temp/latest/environment/summary/system/wifi-status/raw/files/raw/files/delete/raw/disk-usage/api/raw/{filename}Files changed
src/db/biometrics-schema.ts—bedTemp,freezerTemptablessrc/db/biometrics-migrations/0002_mushy_loki.sql— generated migrationmodules/environment-monitor/— Python module (main.py, manifest, service, requirements)src/lib/tempUtils.ts— centidegree/centipercent conversion helperssrc/server/routers/environment.ts— new environment data routersrc/server/routers/raw.ts— new RAW file management routersrc/server/routers/system.ts— wifiStatus proceduresrc/server/routers/app.ts— mount environment + raw routerssrc/server/openapi.ts— Environment + Raw tagsapp/api/raw/[filename]/route.ts— streaming binary downloadscripts/install— install environment-monitor moduleRelated: #161 (automated RAW file cleanup — future work)
Test plan
npx tsc --noEmit— type check passespnpm lint— no lint errorspnpm test— all 551 tests pass/api/openapi.jsonincludes new endpoints/panelshows environment + raw routerssystemctl status sleepypod-environment-monitorrunningbed_temp/freezer_temptables populating after 60sSummary by CodeRabbit
New Features
Chores