Skip to content

release: prepare v11.0.0-alpha.2#102

Merged
gaelic-ghost merged 2 commits into
mainfrom
output/lan-audio-receiver
Jun 1, 2026
Merged

release: prepare v11.0.0-alpha.2#102
gaelic-ghost merged 2 commits into
mainfrom
output/lan-audio-receiver

Conversation

@gaelic-ghost
Copy link
Copy Markdown
Owner

@gaelic-ghost gaelic-ghost commented Jun 1, 2026

Release

  • prepares v11.0.0-alpha.2 from branch output/lan-audio-receiver
  • keeps protected main updates behind pull request review and CI
  • release tag v11.0.0-alpha.2 will be created after CI and the review-comment gate pass, so failed or still-discussed release candidates do not get tagged

Review Loop

Before merge and tagging, scripts/repo-maintenance/release.sh watches CI and stops on review comments unless the maintainer has already addressed or resolved them and reruns with --review-comments-addressed.

Summary by CodeRabbit

  • New Features

    • Added optional LAN audio receiver for network audio streaming (disabled by default)
    • Configurable via TCP port, Bonjour service name, and shared-token authentication
    • Active stream count reporting in transport snapshots
  • Chores

    • Bumped dependency version to 11.0.0-alpha.2
    • Updated default configuration and templates
  • Documentation

    • Added LAN audio receiver configuration and enabling instructions
    • Updated operator documentation with new transport capability details

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces a network audio receiver feature that allows the SpeakSwiftly server to accept LAN-based audio streams via TCP with Bonjour advertisement and shared-token handshakes. The receiver is optional and disabled by default, with full configuration support, lifecycle management, transport state tracking, comprehensive tests, and documentation updates including a dependency version bump to 11.0.0-alpha.2.

Changes

Network Audio Receiver Feature

Layer / File(s) Summary
Network Audio Receiver Configuration Model
Sources/SSSCore/Config/NetworkAudioReceiverConfig.swift, Sources/SSSCore/Config/AppConfig.swift, Sources/SSSCore/Config/ServerConfigStore.swift, Sources/SSSCore/Config/ServerConfigPersistence.swift, Sources/SSSCore/Resources/default-server.yaml
NetworkAudioReceiverConfig struct models enabled/disabled state, service name, port, and optional shared token with validated port ranges and formatted .local advertised addresses. Integrated into AppConfig, defaults store, YAML templates, and default configuration file.
Transport State and Stream Count Tracking
Sources/SSSCore/Host/EmbeddedServerSnapshots.swift, Sources/SSSCore/Host/ServerHost+EventMapping.swift, Sources/SSSCore/Host/ServerHost+RuntimeLifecycle.swift
TransportStatusSnapshot adds activeStreamCount: Int? field serialized as active_stream_count. updateTransportStatus accepts optional port and activeStreamCount parameters. New markTransportListening and markTransportActiveStreamCount helpers update transport state and trigger immediate publish.
ServerHost Configuration and Snapshot Integration
Sources/SSSCore/Host/ServerHost.swift, Sources/SSSCore/Host/ServerHost+Snapshots.swift
ServerHost actor stores networkAudioReceiverConfig with optional initialization. Transport snapshot initialization and collection include receiver transport. Restart detection checks for receiver configuration changes (enabled, serviceName, port, sharedToken).
Network Audio Receiver Lifecycle Service
Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift
NetworkAudioReceiverLifecycleService manages SpeakSwiftly.NetworkAudioStreamListener, startup coordination with port binding and timeout, listener-state monitoring, concurrent inbound stream consumption, active stream tracking via NetworkAudioReceiverStreamCounter actor, transport state reporting, and default chunk playback through local player with cancellation-aware error handling.
Embedded Server Bootstrap Integration
Sources/SpeakSwiftlyServer/EmbeddedLifecycleServices.swift, Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift
withEmbeddedShutdownBarrier widened to package access. embeddedServerLiveBootstrap conditionally increments shutdown barrier, marks receiver transport starting/stopped/failed, and appends NetworkAudioReceiverLifecycleService when receiver is enabled.
Configuration Validation Tests
Tests/SSSCoreTests/ConfigTests.swift
Configuration tests verify networkAudioReceiver defaults, environment-variable parsing via APP_NETWORK_AUDIO_RECEIVER_* keys, YAML configuration loading, and error cases (missing shared token when enabled, invalid port).
Network Audio Receiver Embedding Tests
Tests/SpeakSwiftlyServerEmbeddingTests/HostLifecycleTests.swift
Comprehensive embedding test boots receiver service, sends loopback audio streams with shared-token authentication, captures and validates chunks via probe actor, verifies state transitions and active stream counts, and triggers graceful shutdown. Configuration reload variants and polling utilities included.
Dependency and Test Fixture Updates
Package.swift, Package.resolved, Tests/SpeakSwiftlyServerTestSupport/SpeakSwiftlyServerTestFixtures.swift
SpeakSwiftly bumped to 11.0.0-alpha.2 in package manifest and lock file. SpeakSwiftlyServerEmbeddingTests target adds SpeakSwiftly product dependency. Test fixtures extended with audioFormat and contentType fields.
Documentation and Version Updates
API.md, Sources/SpeakSwiftlyServer/SpeakSwiftlyServer.docc/Articles/*, .codex-plugin/plugin.json, hooks/hooks.json, docs/codex-hooks-tts.md, scripts/repo-maintenance/version-bump.sh, skills/speak-swiftly-codex-hooks/SKILL.md
API documentation describes LAN receiver behavior, Bonjour, shared-token handshake, and transport snapshots. Operator surfaces and LaunchAgent workflow updated. Plugin version and hook commands bumped to 11.0.0-alpha.2. Version-bump script enhanced for pre-release suffixes.

Sequence Diagram(s)

sequenceDiagram
    participant ServerHost
    participant EmbeddedServerSession
    participant NetworkAudioReceiverLifecycleService
    participant NetworkAudioStreamListener
    participant RemoteAudioSender
    participant StreamCounter
    participant LocalAudioPlayer
    
    EmbeddedServerSession->>ServerHost: markTransportStarting("network_audio_receiver")
    EmbeddedServerSession->>NetworkAudioReceiverLifecycleService: run()
    NetworkAudioReceiverLifecycleService->>NetworkAudioStreamListener: start listener
    NetworkAudioStreamListener-->>NetworkAudioReceiverLifecycleService: listening on port
    NetworkAudioReceiverLifecycleService->>ServerHost: markTransportListening("network_audio_receiver", port)
    RemoteAudioSender->>NetworkAudioStreamListener: connect + shared-token handshake
    NetworkAudioStreamListener-->>RemoteAudioSender: authenticated
    RemoteAudioSender->>NetworkAudioStreamListener: stream audio chunks
    NetworkAudioReceiverLifecycleService->>StreamCounter: start(requestID)
    StreamCounter-->>NetworkAudioReceiverLifecycleService: activeCount=1
    NetworkAudioReceiverLifecycleService->>ServerHost: markTransportActiveStreamCount("network_audio_receiver", 1)
    NetworkAudioReceiverLifecycleService->>LocalAudioPlayer: playInboundStream(chunks)
    LocalAudioPlayer-->>NetworkAudioReceiverLifecycleService: playback complete
    NetworkAudioReceiverLifecycleService->>StreamCounter: finish(requestID)
    StreamCounter-->>NetworkAudioReceiverLifecycleService: activeCount=0
    NetworkAudioReceiverLifecycleService->>ServerHost: markTransportActiveStreamCount("network_audio_receiver", 0)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • gaelic-ghost/SpeakSwiftlyServer#92: Both PRs update the Codex plugin version in .codex-plugin/plugin.json and synchronize Socket hook command targets across manifests and documentation, establishing a direct dependency chain for consistent versioning.

Suggested labels

enhancement

Poem

🐰 A listener arrives with a port and a song,
Bonjour announces where audio belongs,
Tokens shake hands, streams flow with care,
While actors count pulses floating through air!
The server now hears from near and afar—
That's quite the receiver, no matter the bar! 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'release: prepare v11.0.0-alpha.2' clearly and concisely summarizes the main objective of the pull request, which is to prepare the repository for the v11.0.0-alpha.2 release by updating version metadata and integrating the LAN audio receiver service.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch output/lan-audio-receiver

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2dd6690b79

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

enabled: \(appConfig.networkAudioReceiver.enabled ? "true" : "false")
serviceName: '\(appConfig.networkAudioReceiver.serviceName)'
port: \(appConfig.networkAudioReceiver.port)
sharedToken: ''
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve the receiver token when saving config

When the LAN receiver is enabled from YAML, any runtime-config save path (saveRuntimeConfiguration via HTTP/MCP/default-voice changes) reloads the current app config and rewrites the whole file through this renderer, but this line always replaces the valid sharedToken with an empty string. Because NetworkAudioReceiverConfig rejects enabled: true without a non-empty token, the saved server.yaml becomes invalid for config reloads and the next process restart cannot load the operator's config. Please carry forward appConfig.networkAudioReceiver.sharedToken instead of blanking it when the receiver is enabled.

Useful? React with 👍 / 👎.

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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
scripts/repo-maintenance/version-bump.sh (1)

6-8: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden release version validation in scripts/repo-maintenance/version-bump.sh (lines 6-8).

The current case patterns use [0-9]*, which matches letters after the initial digit (so malformed inputs like 1foo.2.3 / 1.2bar.3 are treated as valid). Tighten validation to require digits-only MAJOR.MINOR.PATCH with an optional prerelease suffix.

Suggested change: replace the case glob check with an anchored regex, e.g.

  • ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$
    and fail (as the script already does) when it doesn’t match.
🤖 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 `@scripts/repo-maintenance/version-bump.sh` around lines 6 - 8, The
release_version glob in the case statement is too permissive (patterns using
[0-9]* allow trailing letters) — update the validation to enforce digits-only
MAJOR.MINOR.PATCH with an optional prerelease by replacing the case check on
release_version with a POSIX-compliant regex match (e.g., use [[ ]] or grep -E)
that implements ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ and keep the existing
failure path when it doesn’t match; target the release_version variable and the
current case ... esac block when making the change.
Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift (1)

251-264: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Transport stop/fail is marked twice; the generic message overwrites the specific one.

NetworkAudioReceiverLifecycleService.run() already calls markTransportStopped/markTransportFailed with a receiver-specific message before the service group returns. This runTask then marks the same transport again, so on failure the specific "could not start the LAN audio receiver…" message is replaced by the generic Hummingbird message. Consider scoping these bootstrap-level marks to the Hummingbird transports (http/mcp) and letting the lifecycle service own the receiver's state.

🤖 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 `@Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift` around lines 251 -
264, The catch block in EmbeddedServerSession.runTask is re-marking the network
audio receiver transport and overwriting the receiver-specific error set by
NetworkAudioReceiverLifecycleService.run; update the catch to only call
host.markTransportFailed/Stopped for the Hummingbird transports ("http" and
"mcp") and remove or skip the calls for NetworkAudioReceiverConfig.transportName
so the lifecycle service retains ownership of the receiver's state (refer to
EmbeddedServerSession.runTask and NetworkAudioReceiverLifecycleService.run and
the markTransportStopped/markTransportFailed calls).
🧹 Nitpick comments (2)
Tests/SpeakSwiftlyServerEmbeddingTests/HostLifecycleTests.swift (1)

190-244: ⚡ Quick win

Guarantee runTask teardown on early throw.

runTask runs serviceGroup.run() as an unstructured task. If any try await between Lines 194 and 231 throws (e.g. the 5s waitForNetworkAudioReceiverPort timeout, or sender.send), the function exits before Line 242, leaving the service group and its bound network listener running. That leaked task can cause cross-test flakiness or hangs. Consider triggering shutdown/cancellation via defer.

♻️ Proposed cleanup
     let runTask = Task<Void, Error> {
         try await serviceGroup.run()
     }
+    defer {
+        Task { await serviceGroup.triggerGracefulShutdown() }
+        runTask.cancel()
+    }
🤖 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 `@Tests/SpeakSwiftlyServerEmbeddingTests/HostLifecycleTests.swift` around lines
190 - 244, The test spawns runTask to run serviceGroup.run() but can exit early
on throws (e.g., waitForNetworkAudioReceiverPort or sender.send), leaking the
service and listener; wrap the rest of the test after creating runTask in a
defer that always triggers shutdown/cancellation and awaits the runTask
completion so the serviceGroup is stopped on any early return. Specifically, add
a defer block after creating runTask that calls
serviceGroup.triggerGracefulShutdown() (or cancels runTask) and then awaits
runTask.value (or absorbs errors) to ensure runTask and the NetworkAudioReceiver
are torn down even if waitForNetworkAudioReceiverPort or sender.send throws.
Sources/SSSCore/Config/NetworkAudioReceiverConfig.swift (1)

116-119: ⚡ Quick win

Consider using typed error checking instead of string matching.

The current implementation detects missing config values by checking if the error description contains a specific message string. If swift-configuration provides a typed error (e.g., a specific error case or type), prefer matching against that type instead of the description string.

🤖 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 `@Sources/SSSCore/Config/NetworkAudioReceiverConfig.swift` around lines 116 -
119, Replace the fragile string-based check in isMissingRequiredConfigValue(_
error: any Error) with a typed pattern match: attempt to cast the error to the
concrete error type provided by swift-configuration (e.g., ConfigurationError or
its actual type) and check for the specific case/enum value that represents a
missing required config (e.g., .missingRequiredValue(key:) or similar); if the
concrete type/case is not available, keep a conservative fallback to the
existing String(describing: error).contains(...) check so behavior remains safe.
Ensure you reference and match the actual enum/case name from the
swift-configuration API in the implementation.
🤖 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 `@Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift`:
- Around line 204-215: The NetworkAudioReceiverLifecycleService is appended
without explicit termination behaviors which may let a receiver failure bring
down the whole embedded session; update the service registration where
services.append(...) is called to set ServiceConfiguration (or the equivalent
initializer) for NetworkAudioReceiverLifecycleService with
successTerminationBehavior: .ignore and failureTerminationBehavior: .ignore
(mirroring ConfigWatchService) so receiver failures are non-fatal, or
document/justify the intended fatal contract if you want host shutdown on
receiver errors.

In `@Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift`:
- Around line 42-47: playInboundStream creates a fresh
SpeakSwiftly.LocalGeneratedAudioPlayer per stream and consumeInboundStreams
launches unbounded concurrent tasks from listener.inboundStreams(), allowing
overlapping playback; serialize playback by using a single shared player or a
bounded concurrency gate. Change playInboundStream(_:) to either accept an
injected SpeakSwiftly.LocalGeneratedAudioPlayer or call a shared/actor-scoped
player instance, and update consumeInboundStreams to acquire a permit (e.g.,
AsyncSemaphore, actor, or a TaskGroup with maxConcurrentWorkers) before creating
the Task so only one stream plays at a time; release the permit after
player.play completes and ensure player.play runs on `@MainActor` as currently
done.

In `@Sources/SSSCore/Config/ServerConfigPersistence.swift`:
- Line 55: The YAML output for serviceName uses single quotes but doesn't escape
embedded single quotes, so values like "Bob's Receiver" break parsing; update
the code that emits serviceName (the interpolated
appConfig.networkAudioReceiver.serviceName) to escape single quotes by doubling
them (replace every ' with '' ) before interpolation so the generated YAML uses
proper single-quoted scalars.
- Around line 53-57: The YAML renderer in ServerConfigPersistence.swift
currently hard-codes sharedToken: '' which wipes runtime tokens; update the
saveRuntimeConfiguration/renderYAML logic to persist the actual
appConfig.networkAudioReceiver.sharedToken when networkAudioReceiver.enabled is
true (or omit the sharedToken field entirely when enabled is false) instead of
writing an empty string. Locate the block that renders networkAudioReceiver
(used by renderYAML/saveRuntimeConfiguration after loadAppConfig) and replace
the fixed literal with conditional output that uses
appConfig.networkAudioReceiver.sharedToken (or skips the line when not enabled)
so NetworkAudioReceiverConfig.init will receive the correct value on reload.

---

Outside diff comments:
In `@scripts/repo-maintenance/version-bump.sh`:
- Around line 6-8: The release_version glob in the case statement is too
permissive (patterns using [0-9]* allow trailing letters) — update the
validation to enforce digits-only MAJOR.MINOR.PATCH with an optional prerelease
by replacing the case check on release_version with a POSIX-compliant regex
match (e.g., use [[ ]] or grep -E) that implements
^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ and keep the existing failure path
when it doesn’t match; target the release_version variable and the current case
... esac block when making the change.

In `@Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift`:
- Around line 251-264: The catch block in EmbeddedServerSession.runTask is
re-marking the network audio receiver transport and overwriting the
receiver-specific error set by NetworkAudioReceiverLifecycleService.run; update
the catch to only call host.markTransportFailed/Stopped for the Hummingbird
transports ("http" and "mcp") and remove or skip the calls for
NetworkAudioReceiverConfig.transportName so the lifecycle service retains
ownership of the receiver's state (refer to EmbeddedServerSession.runTask and
NetworkAudioReceiverLifecycleService.run and the
markTransportStopped/markTransportFailed calls).

---

Nitpick comments:
In `@Sources/SSSCore/Config/NetworkAudioReceiverConfig.swift`:
- Around line 116-119: Replace the fragile string-based check in
isMissingRequiredConfigValue(_ error: any Error) with a typed pattern match:
attempt to cast the error to the concrete error type provided by
swift-configuration (e.g., ConfigurationError or its actual type) and check for
the specific case/enum value that represents a missing required config (e.g.,
.missingRequiredValue(key:) or similar); if the concrete type/case is not
available, keep a conservative fallback to the existing String(describing:
error).contains(...) check so behavior remains safe. Ensure you reference and
match the actual enum/case name from the swift-configuration API in the
implementation.

In `@Tests/SpeakSwiftlyServerEmbeddingTests/HostLifecycleTests.swift`:
- Around line 190-244: The test spawns runTask to run serviceGroup.run() but can
exit early on throws (e.g., waitForNetworkAudioReceiverPort or sender.send),
leaking the service and listener; wrap the rest of the test after creating
runTask in a defer that always triggers shutdown/cancellation and awaits the
runTask completion so the serviceGroup is stopped on any early return.
Specifically, add a defer block after creating runTask that calls
serviceGroup.triggerGracefulShutdown() (or cancels runTask) and then awaits
runTask.value (or absorbs errors) to ensure runTask and the NetworkAudioReceiver
are torn down even if waitForNetworkAudioReceiverPort or sender.send throws.
🪄 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 Plus

Run ID: b04fde67-ccf3-426a-a564-253f8d2c26be

📥 Commits

Reviewing files that changed from the base of the PR and between 888a72c and 2dd6690.

📒 Files selected for processing (26)
  • .codex-plugin/plugin.json
  • API.md
  • Package.resolved
  • Package.swift
  • Sources/SSSCore/Config/AppConfig.swift
  • Sources/SSSCore/Config/NetworkAudioReceiverConfig.swift
  • Sources/SSSCore/Config/ServerConfigPersistence.swift
  • Sources/SSSCore/Config/ServerConfigStore.swift
  • Sources/SSSCore/Host/EmbeddedServerSnapshots.swift
  • Sources/SSSCore/Host/ServerHost+EventMapping.swift
  • Sources/SSSCore/Host/ServerHost+RuntimeLifecycle.swift
  • Sources/SSSCore/Host/ServerHost+Snapshots.swift
  • Sources/SSSCore/Host/ServerHost.swift
  • Sources/SSSCore/Resources/default-server.yaml
  • Sources/SpeakSwiftlyServer/EmbeddedLifecycleServices.swift
  • Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift
  • Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift
  • Sources/SpeakSwiftlyServer/SpeakSwiftlyServer.docc/Articles/LaunchAgent-Workflow.md
  • Sources/SpeakSwiftlyServer/SpeakSwiftlyServer.docc/Articles/Operator-Surfaces.md
  • Tests/SSSCoreTests/ConfigTests.swift
  • Tests/SpeakSwiftlyServerEmbeddingTests/HostLifecycleTests.swift
  • Tests/SpeakSwiftlyServerTestSupport/SpeakSwiftlyServerTestFixtures.swift
  • docs/codex-hooks-tts.md
  • hooks/hooks.json
  • scripts/repo-maintenance/version-bump.sh
  • skills/speak-swiftly-codex-hooks/SKILL.md

Comment on lines +204 to +215
if config.networkAudioReceiver.enabled {
services.append(
.init(
service: NetworkAudioReceiverLifecycleService(
host: host,
config: config.networkAudioReceiver,
shutdownBarrier: shutdownBarrier,
),
serviceName: "NetworkAudioReceiverLifecycleService",
),
)
}
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 | 🟠 Major | ⚡ Quick win

❓ Verification inconclusive

🌐 Web query:

swift-service-lifecycle ServiceGroupConfiguration ServiceConfiguration default successTerminationBehavior failureTerminationBehavior

💡 Result:

I found authoritative docs for swift-service-lifecycle’s ServiceGroupConfiguration/ServiceConfiguration fields, but I could not retrieve the specific “default” values for successTerminationBehavior and failureTerminationBehavior from the sources surfaced in this run. The available results clearly show that these parameters exist and can be configured (e.g., setting successTerminationBehavior and failureTerminationBehavior to behaviors like.ignore /.shutdownGracefully), but they do not state the defaults explicitly in the snippets I accessed. What I can state with evidence: 1) ServiceConfiguration includes successTerminationBehavior and failureTerminationBehavior and they control how the service group terminates when a service finishes/throws. 2) successTerminationBehavior can be set to.ignore (shown in a configuration example), and failureTerminationBehavior can be set to.shutdownGracefully (shown in examples). 3) The library’s configuration is customizable per service via ServiceGroup(configuration:.init(services:...)). If you want, I can do one more targeted search specifically for the doc pages/Swift interface that show the default enum cases or default initializers for ServiceGroupConfiguration.ServiceConfiguration (e.g., the “init” signature or property default in current docs).

Citations:


Make NetworkAudioReceiverLifecycleService failure behavior explicitly non-fatal for the embedded server (or justify the contract).

NetworkAudioReceiverLifecycleService is registered without setting successTerminationBehavior / failureTerminationBehavior. If swift-service-lifecycle’s defaults are group-fatal when a service throws (e.g., LAN listener socket loss), an optional receiver failure could shut down the entire embedded server session; ConfigWatchService uses .ignore for both specifically to avoid non-core failures terminating the host. Consider setting the receiver to non-fatal termination behaviors (mirroring ConfigWatchService) unless taking down the host on receiver failure is the intended contract.

swift-service-lifecycle ServiceConfiguration successTerminationBehavior failureTerminationBehavior default values what happens when a service throws without specifying failureTerminationBehavior
🤖 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 `@Sources/SpeakSwiftlyServer/EmbeddedServerSession.swift` around lines 204 -
215, The NetworkAudioReceiverLifecycleService is appended without explicit
termination behaviors which may let a receiver failure bring down the whole
embedded session; update the service registration where services.append(...) is
called to set ServiceConfiguration (or the equivalent initializer) for
NetworkAudioReceiverLifecycleService with successTerminationBehavior: .ignore
and failureTerminationBehavior: .ignore (mirroring ConfigWatchService) so
receiver failures are non-fatal, or document/justify the intended fatal contract
if you want host shutdown on receiver errors.

Comment on lines +42 to +47
private static func playInboundStream(_ inboundStream: SpeakSwiftly.NetworkAudioInboundStream) async throws {
try await Task { @MainActor in
let player = SpeakSwiftly.LocalGeneratedAudioPlayer()
try await player.play(chunks: inboundStream.chunks)
}.value
}
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 | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how LocalGeneratedAudioPlayer / inboundStreams handle concurrent playback
rg -nP -C3 'inboundStreams|LocalGeneratedAudioPlayer' --type=swift

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 1804


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift"
nl -ba "$FILE" | sed -n '1,120p'
nl -ba "$FILE" | sed -n '120,220p'

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 118


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift"
echo "== consumeInboundStreams context =="
rg -n "consumeInboundStreams|withTaskGroup|addTask|playbackSink" "$FILE" -n

echo
echo "== playInboundStream lines (around 35-70) =="
awk 'NR>=35 && NR<=80 {printf "%4d:%s\n", NR, $0}' "$FILE"

echo
echo "== task group body (around 130-220) =="
awk 'NR>=120 && NR<=220 {printf "%4d:%s\n", NR, $0}' "$FILE"

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 5828


Cap/serialize inbound playback to avoid per-stream concurrent players

playInboundStream creates a new SpeakSwiftly.LocalGeneratedAudioPlayer() for each inbound stream, and consumeInboundStreams spawns a new withTaskGroup task for every value yielded by listener.inboundStreams() (no concurrency limit). This means multiple inbound streams can trigger concurrent playback through independent players; if only one stream should play at a time, gate concurrency (e.g., shared/serialized player or bounded semaphore).

[Also applies to: 144-179]

🤖 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 `@Sources/SpeakSwiftlyServer/NetworkAudioReceiverLifecycleService.swift` around
lines 42 - 47, playInboundStream creates a fresh
SpeakSwiftly.LocalGeneratedAudioPlayer per stream and consumeInboundStreams
launches unbounded concurrent tasks from listener.inboundStreams(), allowing
overlapping playback; serialize playback by using a single shared player or a
bounded concurrency gate. Change playInboundStream(_:) to either accept an
injected SpeakSwiftly.LocalGeneratedAudioPlayer or call a shared/actor-scoped
player instance, and update consumeInboundStreams to acquire a permit (e.g.,
AsyncSemaphore, actor, or a TaskGroup with maxConcurrentWorkers) before creating
the Task so only one stream plays at a time; release the permit after
player.play completes and ensure player.play runs on `@MainActor` as currently
done.

Comment on lines +53 to +57
networkAudioReceiver:
enabled: \(appConfig.networkAudioReceiver.enabled ? "true" : "false")
serviceName: '\(appConfig.networkAudioReceiver.serviceName)'
port: \(appConfig.networkAudioReceiver.port)
sharedToken: ''
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 | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -t f 'NetworkAudioReceiverConfig.swift' --exec cat {}
rg -nP -C3 'sharedToken' --type=swift

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 16172


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "saveRuntimeConfiguration|renderYAML" Sources/SSSCore/Config -S
echo "----"
sed -n '1,220p' Sources/SSSCore/Config/ServerConfigPersistence.swift
echo "----"
sed -n '220,520p' Sources/SSSCore/Config/ServerConfigPersistence.swift

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 7873


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "saveRuntimeConfiguration|renderYAML" Sources/SSSCore/Config -S
echo "----"
sed -n '1,220p' Sources/SSSCore/Config/ServerConfigPersistence.swift
echo "----"
sed -n '220,520p' Sources/SSSCore/Config/ServerConfigPersistence.swift

Repository: gaelic-ghost/SpeakSwiftlyServer

Length of output: 7873


Fix runtime persistence of app.networkAudioReceiver.sharedToken (hard-coded empty string).

In Sources/SSSCore/Config/ServerConfigPersistence.swift (rendered block around lines 53-57), sharedToken is always written as sharedToken: ''. Since saveRuntimeConfiguration does loadAppConfig() and then rewrites the entire YAML via renderYAML, any runtime save wipes a configured token from disk. On the next load, NetworkAudioReceiverConfig.init throws when enabled is true and sharedToken is empty, causing reload failure. Persist the actual appConfig.networkAudioReceiver.sharedToken (or omit the field when enabled is false) instead of hard-coding ''.

sharedToken: ''
🤖 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 `@Sources/SSSCore/Config/ServerConfigPersistence.swift` around lines 53 - 57,
The YAML renderer in ServerConfigPersistence.swift currently hard-codes
sharedToken: '' which wipes runtime tokens; update the
saveRuntimeConfiguration/renderYAML logic to persist the actual
appConfig.networkAudioReceiver.sharedToken when networkAudioReceiver.enabled is
true (or omit the sharedToken field entirely when enabled is false) instead of
writing an empty string. Locate the block that renders networkAudioReceiver
(used by renderYAML/saveRuntimeConfiguration after loadAppConfig) and replace
the fixed literal with conditional output that uses
appConfig.networkAudioReceiver.sharedToken (or skips the line when not enabled)
so NetworkAudioReceiverConfig.init will receive the correct value on reload.

title: \(appConfig.mcp.title)
networkAudioReceiver:
enabled: \(appConfig.networkAudioReceiver.enabled ? "true" : "false")
serviceName: '\(appConfig.networkAudioReceiver.serviceName)'
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 | ⚡ Quick win

Escape single quotes in serviceName.

serviceName is wrapped in single quotes but inner ' characters aren't doubled, so a name like Bob's Receiver produces invalid YAML and breaks config loading. YAML single-quoted scalars escape a quote by doubling it.

🛡️ Proposed fix
-            serviceName: '\(appConfig.networkAudioReceiver.serviceName)'
+            serviceName: '\(appConfig.networkAudioReceiver.serviceName.replacingOccurrences(of: "'", with: "''"))'
📝 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
serviceName: '\(appConfig.networkAudioReceiver.serviceName)'
serviceName: '\(appConfig.networkAudioReceiver.serviceName.replacingOccurrences(of: "'", with: "''"))'
🤖 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 `@Sources/SSSCore/Config/ServerConfigPersistence.swift` at line 55, The YAML
output for serviceName uses single quotes but doesn't escape embedded single
quotes, so values like "Bob's Receiver" break parsing; update the code that
emits serviceName (the interpolated appConfig.networkAudioReceiver.serviceName)
to escape single quotes by doubling them (replace every ' with '' ) before
interpolation so the generated YAML uses proper single-quoted scalars.

@gaelic-ghost gaelic-ghost merged commit f3ffaeb into main Jun 1, 2026
2 checks passed
@gaelic-ghost gaelic-ghost deleted the output/lan-audio-receiver branch June 1, 2026 15:16
@coderabbitai coderabbitai Bot mentioned this pull request Jun 5, 2026
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.

1 participant