Skip to content

chore: fix all 16 code-scan findings (scan-6bcbf3cf) - #690

Merged
streamer45 merged 17 commits into
mainfrom
devin/1789828230-code-scan-fixes
Sep 19, 2026
Merged

streamer45 merged 17 commits into
mainfrom
devin/1789828230-code-scan-fixes

Conversation

@staging-devin-ai-integration

@staging-devin-ai-integration staging-devin-ai-integration Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Resolves all 16 findings from code scan scan-6bcbf3cf61894b29b2737f185f5a800a, one signed commit per finding (six cherry-picked from the earlier terminated-session branches, ten implemented here).
  • The auth sweep replaces stale #[allow(dead_code)] suppressions with real conditions: MoQ-only request/claim types are now #[cfg(feature = "moq")], never-constructed AuthError/StoreError variants and the dead MaybeAuth/jti/sub/root/total_success/pipeline_path/permission-helper items are deleted, and the MoQ gateway now logs the unused session_id instead of carrying a suppression.
  • Three new shared homes end the copy-paste drift: streamkit_core::text hosts the SentenceSplitter/extract_chunk used by all five TTS plugins and TextChunkerNode (pocket-tts picks up the CJK boundaries it had lost); plugins/native/common hosts the sherpa-onnx FFI bindings plus an owning OfflineTtsConfig builder for piper/kokoro/matcha, and a feature-gated silero_vad module for whisper/sensevoice/parakeet whose process_chunk now returns (probability, is_speech) so callers stop re-implementing the threshold comparison.
  • plugins/native/common is a support crate, not a plugin: the two marketplace enumeration scripts skip it, and just lint-plugins/fix-plugins lint it with --all-targets --all-features.

Review & Validation

  • just lint passes (including the new common crate).
  • cargo test --workspace -- --skip gpu_tests:: and cargo test -p streamkit-server --features moq|mcp pass.
  • Skim the common FFI struct layouts against sherpa-onnx/c-api.h — layout must stay byte-identical; the builder only changes who writes the fields.
  • VAD callers now destructure (probability, is_speech) — spot-check that whisper still uses probability for telemetry and that vad_threshold config plumbing is unchanged.

Notes

  • Commit 5298909 bumps the workspace Cargo.lock to rustls 0.23.45 — a newly published advisory (RUSTSEC-2026-0285) was failing cargo deny advisories on main's lockfile too; not tied to a finding.
  • just test-ui fails on this box with two unhandled WebSocket errors from useCompositorLayers.{perf,render-perf}.test.ts (they open a real ws://localhost:3000/api/v1/control connection; no server is running). All 1371 UI tests pass and ui/ is byte-identical to main, so this is environmental, not from this change.
  • The plugins' own Cargo.lock files were regenerated by cargo after the dependency moves.

Finding → commit map

Finding Commit
sfind-7a1e043a6e8549dabb24dab4a8c7ced9 b919738
sfind-9e70b738d4e5489fa91bbdf6964dde56 ee4a619
sfind-ee00315bd4674501a4c8a8b382a07a91 644f617
sfind-7c55087087d64d6981515c6b2da97a39 314c68d
sfind-019e0e6339164e9781ca104bc0332f9b aa3d8fe
sfind-5181780016d34e709f62754881c66c71 ee9b363
sfind-617012a3955b47ef907efbfe01c97e56 0777cb8
sfind-b043e0c55e5c4904a6f42d3d19fad606 026638a
sfind-e25bbcea436842b288fe795e4c3d4d02 b8b8c8f
sfind-1ef5a2836ffa4bd1b5b0f20ac21ed4fe dc6f926
sfind-8e7f1fc36e6243489e51ccf99bf6a0bc 1bd3b2e
sfind-d13d8c628fda46b68b380cf234a6da25 c8c35d9
sfind-b7c3ad500f7347f9b139c5011b9b4c43 6c38b88
sfind-91ebd1390a7d4d9284c7f904df0c480f d1feb1f
sfind-a95f7f7cb1474fc2ac6f00113d750f59 2826ead
sfind-96154dc806254f089ee9bf7bbde0b0e3 fb5f9ef

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

Status Commit
🟢 Reviewed 5298909

Devin Review (Staging)

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>
@staging-devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✅ Devin addressed Devin Review findings on 5298909

Devin did not fix any findings.

Code quality to review (1): available in Devin Review only.

View all findings in Devin Review

Devin Review (Staging)

@codecov

codecov Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.30137% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.29%. Comparing base (64dcfda) to head (5298909).

Files with missing lines Patch % Lines
apps/skit/src/moq_gateway.rs 0.00% 9 Missing ⚠️
apps/skit/src/server/mod.rs 0.00% 1 Missing ⚠️
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              
Flag Coverage Δ
backend 85.38% <86.30%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 86.03% <100.00%> (+0.24%) ⬆️
engine 83.76% <ø> (ø)
api 91.14% <ø> (ø)
nodes 84.94% <100.00%> (-0.02%) ⬇️
server 85.31% <16.66%> (+0.05%) ⬆️
plugin-native 84.79% <ø> (ø)
plugin-wasm 95.41% <ø> (ø)
ui-services 86.29% <ø> (ø)
ui-components 68.94% <ø> (ø)
Files with missing lines Coverage Δ
apps/skit/src/auth/claims.rs 98.75% <100.00%> (ø)
apps/skit/src/auth/extractor.rs 91.17% <ø> (+9.49%) ⬆️
apps/skit/src/auth/handlers.rs 93.17% <ø> (ø)
apps/skit/src/auth/mod.rs 99.33% <ø> (ø)
apps/skit/src/auth/moq.rs 100.00% <100.00%> (ø)
apps/skit/src/auth/stores/mod.rs 80.00% <ø> (ø)
apps/skit/src/permissions.rs 98.70% <ø> (+0.95%) ⬆️
crates/core/src/text.rs 100.00% <100.00%> (ø)
crates/nodes/src/core/script.rs 77.49% <ø> (ø)
crates/nodes/src/core/text_chunker.rs 98.67% <100.00%> (+0.40%) ⬆️
... and 3 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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
staging-devin-ai-integration Bot force-pushed the devin/1789828230-code-scan-fixes branch from 97f1005 to 5298909 Compare September 19, 2026 15:48
@streamer45
streamer45 merged commit 1371c1a into main Sep 19, 2026
30 checks passed
@streamer45
streamer45 deleted the devin/1789828230-code-scan-fixes branch September 19, 2026 17:26
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