chore: fix all 16 code-scan findings (scan-6bcbf3cf) - #690
Merged
Merged
Conversation
Finding: sfind-7a1e043a6e8549dabb24dab4a8c7ced9 (cherry picked from commit 53f57fa) Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-9e70b738d4e5489fa91bbdf6964dde56 (cherry picked from commit 65d0892) Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-ee00315bd4674501a4c8a8b382a07a91 (cherry picked from commit 261d1d5) Signed-off-by: streamkit-devin <devin@streamkit.dev>
get_default had no callers and can_accept_oneshot was never consulted; the oneshot limit is enforced by the tower ConcurrencyLimitLayer. Finding: sfind-7c55087087d64d6981515c6b2da97a39 (cherry picked from commit fd1ec6a) Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-019e0e6339164e9781ca104bc0332f9b (cherry picked from commit e6dc2b5) Signed-off-by: streamkit-devin <devin@streamkit.dev>
Drops the rationale-free #[allow(dead_code)] by emitting the owning session id in connection routing and route unregistration traces. Finding: sfind-5181780016d34e709f62754881c66c71 (cherry picked from commit 9a5d053) Signed-off-by: streamkit-devin <devin@streamkit.dev>
The request DTO is only used by the moq-gated token handler and CLI arm; replace the stale #[allow(dead_code)] with the real compilation condition. Finding: sfind-617012a3955b47ef907efbfe01c97e56 Signed-off-by: streamkit-devin <devin@streamkit.dev>
AUD_MOQ, MoqClaims, MoqClaims::validate, and the MissingRoot error variant are only reachable under the moq feature; gate them like the rest of the MoQ surface in auth/mod.rs and gate test_moq_claims_validation to match. Finding: sfind-b043e0c55e5c4904a6f42d3d19fad606 Signed-off-by: streamkit-devin <devin@streamkit.dev>
- Drop the blanket allow on AuthError and remove the never-constructed Revoked/Expired/InvalidAudience variants (those states already surface through the Jwt/Claims wrappers or the extractor's status tuples). - is_revoked is only called from moq-gated code, so gate it with #[cfg(feature = "moq")] instead of an allow. - key_provider is used by the JWKS handler; the allow was stale. - Wire should_enable into start_server's auth decision instead of duplicating the match there, which also drops its dead-code allow. Finding: sfind-e25bbcea436842b288fe795e4c3d4d02 Signed-off-by: streamkit-devin <devin@streamkit.dev>
AuthContext.permissions is read by the websocket auth path, so its allow was stale. MaybeAuth and AuthContext::jti/sub had no production caller and existed only for their own unit test; delete them. Finding: sfind-1ef5a2836ffa4bd1b5b0f20ac21ed4fe Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-8e7f1fc36e6243489e51ccf99bf6a0bc Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-d13d8c628fda46b68b380cf234a6da25 Signed-off-by: streamkit-devin <devin@streamkit.dev>
Finding: sfind-b7c3ad500f7347f9b139c5011b9b4c43 Signed-off-by: streamkit-devin <devin@streamkit.dev>
Five TTS plugins carried copies of the same incremental sentence splitter (piper/kokoro/matcha/supertonic identical; pocket-tts had drifted to English-only boundaries), and TextChunkerNode duplicated the same scan logic again for sentence and clause modes. - streamkit_core::text gains a generic extract_chunk primitive plus a SentenceSplitter wrapper with the shared English+CJK boundary tables; plugins reach it through the plugin SDK's streamkit_core re-export. - TextChunkerNode delegates both modes to extract_chunk with its own clause boundary tables; pocket-tts picks up CJK boundaries. - The dead flush() helper and per-plugin copies are removed. Finding: sfind-91ebd1390a7d4d9284c7f904df0c480f Signed-off-by: streamkit-devin <devin@streamkit.dev>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Contributor
Author
There was a problem hiding this comment.
✅ Devin addressed Devin Review findings on 5298909
Devin did not fix any findings.
Code quality to review (1): available in Devin Review only.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #690 +/- ##
==========================================
+ Coverage 85.26% 85.29% +0.02%
==========================================
Files 251 252 +1
Lines 76246 76210 -36
Branches 2485 2485
==========================================
- Hits 65014 65002 -12
+ Misses 11226 11202 -24
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
piper, kokoro, and matcha each carried ~100 lines of identical sherpa-onnx C-API struct bindings plus ~90 lines of near-identical OfflineTtsConfig assembly (the three plugins differ only in which model-family sub-config they populate and in debug/provider flags). - New plugins/native/common crate (streamkit-plugin-native-common) hosts the repr(C) bindings, a path_to_cstring helper, and an owning OfflineTtsConfig builder that fills unused model families with the null placeholders the C API expects; the unsafe create call moves inside the crate so the node code is safe. - Removes the never-used SherpaOnnxGeneratedAudioCallbackWithArg alias and piper's module-wide #![allow(dead_code)]. - The 'common' dir is skipped in marketplace plugin enumeration and linted via just lint-plugins. Finding: sfind-a95f7f7cb1474fc2ac6f00113d750f59 Signed-off-by: streamkit-devin <devin@streamkit.dev>
whisper, sensevoice, and parakeet each carried a copy of the same SileroVAD ort wrapper (new + process_chunk byte-identical; sensevoice and parakeet had drifted on dead helpers and constness). - plugins/native/common gains a silero_vad module behind the silero-vad feature so the heavy ort/ndarray deps stay optional. - process_chunk now returns (probability, is_speech) so callers stop re-implementing the threshold comparison inline; whisper keeps the probability for telemetry. - Dead helpers is_speech, reset, and threshold() are dropped; parakeet's non-const set_threshold (and its missing_const_for_fn allow) converge on whisper's const fn. - The plugins drop their own ort/ndarray/once_cell deps and their vad.rs copies; whisper's VAD tests move with the module. Finding: sfind-96154dc806254f089ee9bf7bbde0b0e3 Signed-off-by: streamkit-devin <devin@streamkit.dev>
cargo deny advisories fails on rustls 0.23.43 (TLS 1.3 handshake messages accepted across encryption level boundaries). Patch bump, no code changes; lockfile-only update via cargo update -p rustls. Signed-off-by: streamkit-devin <devin@streamkit.dev>
staging-devin-ai-integration
Bot
force-pushed
the
devin/1789828230-code-scan-fixes
branch
from
September 19, 2026 15:48
97f1005 to
5298909
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scan-6bcbf3cf61894b29b2737f185f5a800a, one signed commit per finding (six cherry-picked from the earlier terminated-session branches, ten implemented here).#[allow(dead_code)]suppressions with real conditions: MoQ-only request/claim types are now#[cfg(feature = "moq")], never-constructedAuthError/StoreErrorvariants and the deadMaybeAuth/jti/sub/root/total_success/pipeline_path/permission-helper items are deleted, and the MoQ gateway now logs the unusedsession_idinstead of carrying a suppression.streamkit_core::texthosts theSentenceSplitter/extract_chunkused by all five TTS plugins andTextChunkerNode(pocket-tts picks up the CJK boundaries it had lost);plugins/native/commonhosts the sherpa-onnx FFI bindings plus an owningOfflineTtsConfigbuilder for piper/kokoro/matcha, and a feature-gatedsilero_vadmodule for whisper/sensevoice/parakeet whoseprocess_chunknow returns(probability, is_speech)so callers stop re-implementing the threshold comparison.plugins/native/commonis a support crate, not a plugin: the two marketplace enumeration scripts skip it, andjust lint-plugins/fix-pluginslint it with--all-targets --all-features.Review & Validation
just lintpasses (including the newcommoncrate).cargo test --workspace -- --skip gpu_tests::andcargo test -p streamkit-server --features moq|mcppass.commonFFI struct layouts againstsherpa-onnx/c-api.h— layout must stay byte-identical; the builder only changes who writes the fields.(probability, is_speech)— spot-check that whisper still usesprobabilityfor telemetry and thatvad_thresholdconfig plumbing is unchanged.Notes
Cargo.lockto rustls 0.23.45 — a newly published advisory (RUSTSEC-2026-0285) was failingcargo deny advisorieson main's lockfile too; not tied to a finding.just test-uifails on this box with two unhandled WebSocket errors fromuseCompositorLayers.{perf,render-perf}.test.ts(they open a realws://localhost:3000/api/v1/controlconnection; no server is running). All 1371 UI tests pass andui/is byte-identical tomain, so this is environmental, not from this change.Finding → commit map
Link to Devin session: https://staging.itsdev.in/sessions/9563616525554c70bfb5a643c31511cd
Open in Devin Desktop: https://staging.itsdev.in/desktop/session/9563616525554c70bfb5a643c31511cd?variant=devin-insiders
Requested by: @streamer45
Devin Review
5298909