Skip to content

Add a github workflow to build and test the container#81

Open
dthaler wants to merge 6 commits into
orcasound:mainfrom
dthaler:build-container-image
Open

Add a github workflow to build and test the container#81
dthaler wants to merge 6 commits into
orcasound:mainfrom
dthaler:build-container-image

Conversation

@dthaler
Copy link
Copy Markdown
Contributor

@dthaler dthaler commented Apr 13, 2026

  • Add a github workflow to build the container and run startup tests using NODE_TYPE=dev-virt-s3 and NODE_TYPE=hls-only
  • Use a self-contained approach rather than a two-stage container build, since the image can now be built in github and downloaded
  • Update README.md to match current code: self-contained Dockerfile, correct env var names, node/ compose path, boto3 instead of rsync/s3fs
  • Add .vs to .gitignore for those using VS as an IDE
  • README.md said to use syslog+tls: but all current nodes use syslog: so updated README.md since TLS has not been tested recently. Changing all nodes to use TLS can be tracked in a separate issue.
  • Create a .env.example file that can be copied and edited
  • Fix typo in /etc/security/limits.conf filename in stream.sh that prevented using high priority

Summary by CodeRabbit

  • New Features

    • Added automated container build & test pipeline and multi-architecture image publishing
    • Added a comprehensive example environment template for deployment
  • Bug Fixes

    • Improved shell script variable handling and quoting
    • Fixed debug logging format and added runtime capability detection for process prioritization
  • Documentation

    • Updated installation, local test, and deployment instructions; clarified env vars and audio deps
  • Chores

    • Expanded ignore rules for env and editor files and added container ignore patterns

Fix CI: build base image from ./base/amd64 before building node image
Updated dockerfile
Use a self-contained approach
Add a container startup test
Update README.md to match current code: self-contained Dockerfile, correct env var names, node/ compose path, boto3 instead of rsync/s3fs

Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

Adds a container CI workflow (build/test/push), introduces a scoped Node Docker image and .env example, updates stream.sh startup robustness, small logging fixes, and repository ignore adjustments.

Changes

Cohort / File(s) Summary
CI Workflow
.github/workflows/build-container.yml
New GitHub Actions workflow: containerized tests on push/PR/tag/dispatch and a dependent multi-arch build-and-push job (linux/amd64, linux/arm/v7, linux/arm64) to GHCR with metadata tagging and GHA cache.
Node image & runtime
node/Dockerfile, node/stream.sh, node/.env.example, node/.gitignore
New scoped container built from python:3.11-slim-bookworm; installs ffmpeg/jack/alsa, copies node/ and selective base/ scripts, installs boto3/inotify; adds default env/CMD. stream.sh now uses quoted var tests, conditional defaulting for SAMPLE_RATE/STREAM_RATE, probes CAP_SYS_NICE to set nice/JACK RT flags, and applies conditional nice/JACK invocation.
Repository ignores & dockerignore
.gitignore, .dockerignore
Add .vs to .gitignore; add comprehensive .dockerignore excluding secrets, VCS, deps, editor files; node/.gitignore now ignores .env but keeps !.env.example.
Docs
README.md
Update install/test instructions to build from node/Dockerfile, instruct .env creation from node/.env.example, rename AWS env vars to AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, update syslog example and dependency list (JACK, boto3).
Logging tweaks
base/upload_s3.py, base/upload_flac_s3.py
Change debug logging calls to use format-string substitution (%s) instead of passing values as separate args.

Sequence Diagram(s)

sequenceDiagram
    participant GH as GitHub (Push/PR/Tag/Dispatch)
    participant Test as Test Job
    participant Build as Build-and-Push Job
    participant Container as Container (build/run)
    participant GHCR as GHCR Registry

    GH->>Test: Trigger workflow (push/PR/tag/dispatch)
    Test->>Container: Build image from `node/Dockerfile`
    Container->>Container: Run verification (Python, FFmpeg, JACK, deps, `/app/stream.sh`)
    Test->>Container: Create `test.env`, start container detached
    Container-->>Test: Return status / exit code
    alt status running OR exit code 0
        Test-->>Build: proceed to build-and-push
        Build->>Build: Setup QEMU & Buildx, generate tags/labels
        Build->>GHCR: Build & push multi-arch images
    else
        Test-->>GH: fail workflow
    end
Loading
sequenceDiagram
    participant Init as stream.sh Init
    participant Env as Environment Vars
    participant Sys as Host Capability
    participant JACK as jackd
    participant FF as ffmpeg/live-stream

    Init->>Env: Check STREAM_RATE / SAMPLE_RATE
    alt unset
        Env->>Env: set defaults and log
    end
    Init->>Sys: run `nice -n -1 true`
    alt succeeds (CAP_SYS_NICE)
        Sys->>Init: set NICE_HIGH, NICE_MED, JACKD_RT="-P 75"
    else
        Sys->>Init: leave NICE/JACKD_RT empty, log notice
    end
    Init->>JACK: start `jackd` with conditional $JACKD_RT
    Init->>FF: start ffmpeg/live-stream using $NICE_HIGH/$NICE_MED
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • scottveirs
  • paulcretu

Poem

🐰 I hopped into Docker's den,
Tested containers now and then,
JACK drums and FFmpeg sing,
Actions push each multi-arch thing,
Stream.sh sniffs caps — then we begin!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a GitHub Actions workflow to build and test the container image.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the container build/deploy flow by introducing a GitHub Actions workflow that builds/tests (and on main/tags pushes) a self-contained node container image, while aligning documentation and configuration templates with the current runtime behavior.

Changes:

  • Add a GitHub Actions workflow to build, smoke-test, and publish a multi-arch container image to GHCR.
  • Replace the previous “base image” approach with a self-contained node/Dockerfile and update runtime scripts for env defaults/priority handling.
  • Update docs and templates: README refresh, add node/.env.example, and extend .gitignore entries.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
node/stream.sh Adds env defaults, safer string comparisons, and conditional process priority handling.
node/Dockerfile Switches to a self-contained python-slim image with OS + pip dependencies and runs stream.sh.
node/.gitignore Ignores secrets .env while keeping .env.example tracked.
node/.env.example Adds a copy-ready configuration template for deployments.
base/upload_s3.py Fixes logging formatting for bucket debug output.
base/upload_flac_s3.py Fixes logging formatting for bucket debug output.
README.md Updates setup/run instructions and dependency descriptions to match current architecture.
.gitignore Adds .vs ignore (and keeps .env ignored).
.github/workflows/build-container.yml Adds CI build/test and main/tag build-and-push to GHCR with caching and metadata tagging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread node/stream.sh Outdated
Comment thread .github/workflows/build-container.yml Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
node/stream.sh (1)

57-59: Make the limits.conf edits idempotent.

Every restart appends the same two lines again, so long-lived containers accumulate duplicates in /etc/security/limits.conf. Guard the writes so the startup path stays stable across restarts.

Suggested shell change
-echo `@audio` - memlock 256000 >> /etc/security/limits.conf
-echo `@audio` - rtprio 75 >> /etc/security/limits.conf
+grep -qxF '@audio - memlock 256000' /etc/security/limits.conf || echo '@audio - memlock 256000' >> /etc/security/limits.conf
+grep -qxF '@audio - rtprio 75' /etc/security/limits.conf || echo '@audio - rtprio 75' >> /etc/security/limits.conf
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/stream.sh` around lines 57 - 59, The two echo lines appending "@audio -
memlock 256000" and "@audio - rtprio 75" to /etc/security/limits.conf are not
idempotent and will duplicate on every restart; update the startup script
(node/stream.sh) to first check for the exact lines (e.g., using grep -qF or
grep -qxF) and only append each line if it is not already present, so the
entries are added once and not duplicated across restarts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@node/.env.example`:
- Around line 15-18: The .env.example advertises BUCKET_TYPE but omits the
required variables read by base/upload_s3.py and base/upload_flac_s3.py; add
BUCKET_STREAMING and BUCKET_ARCHIVE entries to the template (with placeholder
values and a short comment) so when BUCKET_TYPE=custom the container won't fail
due to missing env vars—include both variable names (BUCKET_STREAMING,
BUCKET_ARCHIVE) near the BUCKET_TYPE section and show example placeholders for
dev/prod/custom usage.
- Around line 20-23: The .env.example lists NODE_TYPE=offline even though
node/stream.sh does not handle "offline"; remove or comment out the "offline"
option in node/.env.example (leave only supported values like hls-only,
research, debug, dev-virt-s3) and update the accompanying comment to warn not to
enable unsupported modes; only re-add "offline" once node/stream.sh implements
branching for NODE_TYPE "offline".

In `@node/Dockerfile`:
- Around line 30-35: The Dockerfile's COPY node/ ./ instruction will include
sensitive files like node/.env into the image; add a root .dockerignore (placed
at repository root) to exclude deployment secrets and unnecessary files (e.g.,
node/.env, base/*.env, .git, node/node_modules, **/*.pem, **/*.key, .env) so
those files are not sent to the Docker daemon or baked into image layers; ensure
the .dockerignore is committed and then rebuild to verify COPY node/ ./ no
longer includes excluded files.

---

Nitpick comments:
In `@node/stream.sh`:
- Around line 57-59: The two echo lines appending "@audio - memlock 256000" and
"@audio - rtprio 75" to /etc/security/limits.conf are not idempotent and will
duplicate on every restart; update the startup script (node/stream.sh) to first
check for the exact lines (e.g., using grep -qF or grep -qxF) and only append
each line if it is not already present, so the entries are added once and not
duplicated across restarts.
🪄 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: 6bbcd81a-266e-472f-96d7-9fa63160f548

📥 Commits

Reviewing files that changed from the base of the PR and between 2cd6cc1 and 3f1519f.

📒 Files selected for processing (9)
  • .github/workflows/build-container.yml
  • .gitignore
  • README.md
  • base/upload_flac_s3.py
  • base/upload_s3.py
  • node/.env.example
  • node/.gitignore
  • node/Dockerfile
  • node/stream.sh

Comment thread node/.env.example
Comment thread node/.env.example Outdated
Comment thread node/Dockerfile
dthaler added 2 commits April 13, 2026 12:18
Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
node/Dockerfile (1)

30-31: ⚠️ Potential issue | 🟠 Major

Keep deployment secrets out of the image build context.

Line 31 still copies the whole node/ directory, so node/.env will be baked into the image if a root .dockerignore does not exclude it. .gitignore does not protect Docker builds.

Suggested `.dockerignore`
+# Keep deployment secrets out of Docker build context
+node/.env
+!node/.env.example
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/Dockerfile` around lines 30 - 31, The Dockerfile currently uses COPY
node/ ./ which can bake deployment secrets like node/.env into the image; stop
including secrets by adding a .dockerignore that excludes files such as .env,
.env.* and other secret/config files, and ensure the build context does not
contain secrets before running the build; alternatively change the Dockerfile
workflow to COPY only the required artifacts (e.g., package.json,
package-lock.json, src/) instead of copying the whole directory (refer to the
COPY node/ ./ instruction) so secrets are not included in the image.
node/.env.example (1)

15-18: ⚠️ Potential issue | 🟠 Major

Add the missing custom-bucket variables to the template.

This still advertises BUCKET_TYPE=custom in the file docs, but the template never defines BUCKET_STREAMING or BUCKET_ARCHIVE. Copy-pasting custom will fail at startup when the upload scripts read those env vars.

Suggested template update
 # === S3 Bucket Selection ===
 BUCKET_TYPE=dev
 `#BUCKET_TYPE`=prod
+#BUCKET_TYPE=custom
+#BUCKET_STREAMING=your-streaming-bucket
+#BUCKET_ARCHIVE=your-archive-bucket
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/.env.example` around lines 15 - 18, The template advertises a custom
BUCKET_TYPE but doesn't define the required env vars, causing startup failures;
update the .env template to include the missing BUCKET_STREAMING and
BUCKET_ARCHIVE entries (and any related custom-bucket keys the upload scripts
expect) alongside the existing BUCKET_TYPE setting so that when
BUCKET_TYPE=custom is used the upload/init code can read BUCKET_STREAMING and
BUCKET_ARCHIVE (look for BUCKET_TYPE, BUCKET_STREAMING, BUCKET_ARCHIVE in the
repo to ensure you add exactly the keys the upload scripts reference).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@node/stream.sh`:
- Around line 56-57: The script currently starts jackd unconditionally
(JACK_NO_AUDIO_RESERVATION=1 jackd ... -d hw:$AUDIO_HW_ID ...) which binds to
real hardware and prevents dev-virt-s3 overrides; change it so jackd (and
subsequent jack_connect calls) only run for hardware-backed NODE_TYPEs by gating
the jackd startup and the later jack_connect block behind a condition that
checks NODE_TYPE or the USE_JACK_INPUT flag. Locate the jackd invocation and the
jack_connect sequence and wrap them in the same conditional (e.g., if [
"$NODE_TYPE" = "dev-hw" ] or if [ "$USE_JACK_INPUT" = "1" ]), ensuring
SAMPLE_RATE/STREAM_RATE overrides for dev-virt-s3 occur before any jackd start
when in virtual mode.

---

Duplicate comments:
In `@node/.env.example`:
- Around line 15-18: The template advertises a custom BUCKET_TYPE but doesn't
define the required env vars, causing startup failures; update the .env template
to include the missing BUCKET_STREAMING and BUCKET_ARCHIVE entries (and any
related custom-bucket keys the upload scripts expect) alongside the existing
BUCKET_TYPE setting so that when BUCKET_TYPE=custom is used the upload/init code
can read BUCKET_STREAMING and BUCKET_ARCHIVE (look for BUCKET_TYPE,
BUCKET_STREAMING, BUCKET_ARCHIVE in the repo to ensure you add exactly the keys
the upload scripts reference).

In `@node/Dockerfile`:
- Around line 30-31: The Dockerfile currently uses COPY node/ ./ which can bake
deployment secrets like node/.env into the image; stop including secrets by
adding a .dockerignore that excludes files such as .env, .env.* and other
secret/config files, and ensure the build context does not contain secrets
before running the build; alternatively change the Dockerfile workflow to COPY
only the required artifacts (e.g., package.json, package-lock.json, src/)
instead of copying the whole directory (refer to the COPY node/ ./ instruction)
so secrets are not included in the image.
🪄 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: 645d84d6-0149-4ef8-9fa3-256d898e389e

📥 Commits

Reviewing files that changed from the base of the PR and between 3f1519f and 0b5eb2b.

📒 Files selected for processing (4)
  • .github/workflows/build-container.yml
  • node/.env.example
  • node/Dockerfile
  • node/stream.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/build-container.yml

Comment thread node/stream.sh Outdated
Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
@dthaler dthaler changed the title Add a github workflow to build the container Add a github workflow to build and test the container Apr 13, 2026
Create a WAV file to test with

Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
@dthaler dthaler force-pushed the build-container-image branch from 20226af to 95a646e Compare April 13, 2026 20:02
Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
node/stream.sh (2)

56-60: Quote $AUDIO_HW_ID and $SAMPLE_RATE to prevent word splitting.

While $JACKD_RT is intentionally unquoted (to disappear when empty), $AUDIO_HW_ID and $SAMPLE_RATE should be quoted to handle edge cases.

🔧 Proposed fix
 if [ "$NODE_TYPE" != "dev-virt-s3" ]; then
     echo "Starting Jack audio server for hardware audio capture..."
-    JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 $JACKD_RT -d alsa -d hw:$AUDIO_HW_ID -r $SAMPLE_RATE -p 1024 -n 10 -s &
+    JACK_NO_AUDIO_RESERVATION=1 jackd -t 2000 $JACKD_RT -d alsa -d "hw:$AUDIO_HW_ID" -r "$SAMPLE_RATE" -p 1024 -n 10 -s &
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/stream.sh` around lines 56 - 60, The jackd invocation may suffer
word-splitting because $AUDIO_HW_ID and $SAMPLE_RATE are unquoted; update the
jackd command in the block that sets JACK_NO_AUDIO_RESERVATION and calls jackd
(the line with "JACK_NO_AUDIO_RESERVATION=1 jackd ... -d hw:$AUDIO_HW_ID -r
$SAMPLE_RATE ... &") to quote those two expansions (e.g., use -d
"hw:$AUDIO_HW_ID" and -r "$SAMPLE_RATE") while leaving $JACKD_RT unquoted so it
still disappears when empty.

94-102: Inconsistent indentation in dev-virt-s3 block.

Lines 97 and 100 use 2-space indentation while surrounding lines use 4 spaces. This doesn't affect functionality but impacts readability.

🔧 Proposed fix for consistent indentation
 elif [ "$NODE_TYPE" = "dev-virt-s3" ]; then
     SAMPLE_RATE=48000
     STREAM_RATE=48000
-  echo "Sampling from virtual WAV file at $SAMPLE_RATE Hz..."
+    echo "Sampling from virtual WAV file at $SAMPLE_RATE Hz..."
     echo "Asking ffmpeg to stream only HLS segments at $STREAM_RATE Hz......" 
     ## Streaming HLS only via mpegts
-  $NICE_HIGH ffmpeg -re -fflags +genpts -stream_loop -1 -i "samples/haro-strait_2005.wav" \
+    $NICE_HIGH ffmpeg -re -fflags +genpts -stream_loop -1 -i "samples/haro-strait_2005.wav" \
     -f segment -segment_list "/tmp/$NODE_NAME/hls/$timestamp/live.m3u8" -segment_list_flags +live -segment_time $SEGMENT_DURATION -segment_format mpegts \
     -ar $STREAM_RATE -ac $CHANNELS -threads 3 -acodec aac "/tmp/$NODE_NAME/hls/$timestamp/live%03d.ts" &
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/stream.sh` around lines 94 - 102, The dev-virt-s3 case has inconsistent
indentation: normalize the indentation to match the surrounding 4-space style
for the SAMPLE_RATE and STREAM_RATE assignments and the ffmpeg-related echo
lines; specifically adjust the two echo lines ("Sampling from virtual WAV..."
and "Asking ffmpeg to stream...") and the ffmpeg invocation so they use the same
4-space indent as the other statements in the NODE_TYPE = "dev-virt-s3" block
(look for the NODE_TYPE check, SAMPLE_RATE, STREAM_RATE, and the ffmpeg command)
to improve readability.
.github/workflows/build-container.yml (1)

89-128: The hls-only test will always encounter JACK failures in CI.

Since hls-only mode starts JACK (which requires audio hardware), this test will always fail to produce actual HLS output. The test currently accepts this ("audio hardware errors expected" on line 118), but consider whether this provides meaningful validation beyond verifying the container starts.

If the goal is just smoke-testing that the script doesn't crash on startup, this is fine. Otherwise, consider adding a comment clarifying the test's limited scope or skipping verification of HLS output for this mode.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-container.yml around lines 89 - 128, The hls-only
test (NODE_TYPE=hls-only) will always trigger JACK/hardware errors in CI and
thus doesn't validate real HLS output; update the test around the docker run for
orcanode-test-hw and test.env to either (A) add an explicit comment stating this
is only a startup/smoke test and not an HLS functional test (mention
NODE_TYPE=hls-only and JACK), or (B) change the verification to only assert the
container started/exited cleanly (check STATUS/EXIT_CODE) and skip any
HLS/output verification for this run; implement one of these fixes so the intent
is clear and the test doesn't attempt to validate hardware-dependent audio
output.
🤖 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-container.yml:
- Around line 155-170: Update the docker/metadata-action step (id: meta) to use
a simplified image name under the images key: change images: ghcr.io/${{
github.repository }}/orcanode to images: ghcr.io/${{ github.repository }} so the
resulting image path becomes ghcr.io/orcasound/orcanode instead of the redundant
ghcr.io/orcasound/orcanode/orcanode; keep the rest of the metadata-action
configuration (tags block and type/raw latest enable) unchanged.

---

Nitpick comments:
In @.github/workflows/build-container.yml:
- Around line 89-128: The hls-only test (NODE_TYPE=hls-only) will always trigger
JACK/hardware errors in CI and thus doesn't validate real HLS output; update the
test around the docker run for orcanode-test-hw and test.env to either (A) add
an explicit comment stating this is only a startup/smoke test and not an HLS
functional test (mention NODE_TYPE=hls-only and JACK), or (B) change the
verification to only assert the container started/exited cleanly (check
STATUS/EXIT_CODE) and skip any HLS/output verification for this run; implement
one of these fixes so the intent is clear and the test doesn't attempt to
validate hardware-dependent audio output.

In `@node/stream.sh`:
- Around line 56-60: The jackd invocation may suffer word-splitting because
$AUDIO_HW_ID and $SAMPLE_RATE are unquoted; update the jackd command in the
block that sets JACK_NO_AUDIO_RESERVATION and calls jackd (the line with
"JACK_NO_AUDIO_RESERVATION=1 jackd ... -d hw:$AUDIO_HW_ID -r $SAMPLE_RATE ...
&") to quote those two expansions (e.g., use -d "hw:$AUDIO_HW_ID" and -r
"$SAMPLE_RATE") while leaving $JACKD_RT unquoted so it still disappears when
empty.
- Around line 94-102: The dev-virt-s3 case has inconsistent indentation:
normalize the indentation to match the surrounding 4-space style for the
SAMPLE_RATE and STREAM_RATE assignments and the ffmpeg-related echo lines;
specifically adjust the two echo lines ("Sampling from virtual WAV..." and
"Asking ffmpeg to stream...") and the ffmpeg invocation so they use the same
4-space indent as the other statements in the NODE_TYPE = "dev-virt-s3" block
(look for the NODE_TYPE check, SAMPLE_RATE, STREAM_RATE, and the ffmpeg command)
to improve readability.
🪄 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: 1d786679-465e-4fa7-b2f5-dee010a60162

📥 Commits

Reviewing files that changed from the base of the PR and between 0b5eb2b and 77fd62c.

📒 Files selected for processing (3)
  • .dockerignore
  • .github/workflows/build-container.yml
  • node/stream.sh
✅ Files skipped from review due to trivial changes (1)
  • .dockerignore

Comment on lines +155 to +170
- name: Extract metadata
id: meta
uses: docker/metadata-action@8e5442c4ef9f78752691e2d8f8d19755c6f78e81 # v5.5.1
with:
images: ghcr.io/${{ github.repository }}/orcanode
tags: |
# Semver tags on release
type=semver,pattern={{version}}
type=semver,pattern={{major}}.{{minor}}
type=semver,pattern={{major}}
type=ref,event=tag
# Branch-based tags
type=ref,event=branch
type=sha
# Latest tag for main branch
type=raw,value=latest,enable={{is_default_branch}}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the repository name to confirm the resulting image path
echo "Repository: orcasound/orcanode"
echo "Resulting image path: ghcr.io/orcasound/orcanode/orcanode"
echo ""
echo "If this is unintended, the image path should be: ghcr.io/orcasound/orcanode"

Repository: orcasound/orcanode

Length of output: 227


🏁 Script executed:

# Check if there are other container images or references in the workflow
rg "images:" .github/workflows/ -A 2

Repository: orcasound/orcanode

Length of output: 290


🏁 Script executed:

# Look for any documentation about container image naming in the repo
fd -e md -e txt | xargs rg -l "container|image|ghcr" 2>/dev/null | head -20

Repository: orcasound/orcanode

Length of output: 87


🏁 Script executed:

# Check for other docker/metadata-action uses in the repository
rg "docker/metadata-action" .github/workflows/

Repository: orcasound/orcanode

Length of output: 186


🏁 Script executed:

# Look at the broader workflow context to see if there are multiple services/images
cat -n .github/workflows/build-container.yml | head -50

Repository: orcasound/orcanode

Length of output: 2092


🏁 Script executed:

# Check the README files for any documentation about container image naming
cat README.md | grep -i "container\|image\|ghcr" -A 2 -B 2

Repository: orcasound/orcanode

Length of output: 1719


🏁 Script executed:

# Also check the full build-container.yml workflow to understand if there's any reason for the /orcanode suffix
cat -n .github/workflows/build-container.yml | sed -n '130,182p'

Repository: orcasound/orcanode

Length of output: 2446


Simplify the image path by removing the redundant /orcanode suffix.

The image path ghcr.io/${{ github.repository }}/orcanode resolves to ghcr.io/orcasound/orcanode/orcanode, which is redundant since only a single container image is built in this workflow. Use:

images: ghcr.io/${{ github.repository }}

This produces the simpler and more conventional image path ghcr.io/orcasound/orcanode.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-container.yml around lines 155 - 170, Update the
docker/metadata-action step (id: meta) to use a simplified image name under the
images key: change images: ghcr.io/${{ github.repository }}/orcanode to images:
ghcr.io/${{ github.repository }} so the resulting image path becomes
ghcr.io/orcasound/orcanode instead of the redundant
ghcr.io/orcasound/orcanode/orcanode; keep the rest of the metadata-action
configuration (tags block and type/raw latest enable) unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants