Skip to content

fix(android): remove dead VaultWebSocketClient stub (#178) - #186

Merged
maugauwi-hash merged 6 commits into
ethos-protocol:mainfrom
yahia008:fix/178-remove-dead-vault-websocket-client-stub
Aug 25, 2026
Merged

fix(android): remove dead VaultWebSocketClient stub (#178)#186
maugauwi-hash merged 6 commits into
ethos-protocol:mainfrom
yahia008:fix/178-remove-dead-vault-websocket-client-stub

Conversation

@yahia008

Copy link
Copy Markdown
Contributor

Title:
fix(android): remove dead VaultWebSocketClient stub duplicating VaultEventSocket (#178)

Body:

Summary

Closes #178.

VaultWebSocketClient.kt was a complete, correctly-structured WebSocket client whose
own doc comment called it a stub for #110 and said to "wire up the Flow into
VaultViewModel once [a staging endpoint] does." That wiring already happened — under
a different class. VaultEventSocket.kt is the real, @Singleton-injected client
VaultViewModel actually depends on, complete with reconnect backoff and its own test
suite. VaultWebSocketClient had zero references anywhere in src/main, src/test,
or src/androidTest — never instantiated, never bound in AppModule.kt. Leaving it in
the tree risked a future contributor building against the wrong, unmaintained client.

Changes

  • Deleted android/app/src/main/java/com/ethosprotocol/api/VaultWebSocketClient.kt
    (186 lines), including its VaultWebSocketEvent sealed class and the private
    message-envelope DTOs (WSVaultUpdatedMessage, WSVaultExpiredMessage,
    other file references any symbol from it.
  • Confirmed no event coverage was lost: VaultEvent (in models/Models.kt) already
    covers vault_updated / vault_expired / vault_released generically via its type
    discriminator + optional vault field — nothing needed to be ported from the stub.

Explicitly out of scope

  • VaultEventSocket.kt's runtime behavior is untouched — this PR only removes dead code.
  • iOS has an identical dead-stub twin (ios/.../Services/VaultWebSocketClient.swift)
    alongside its own real, wired VaultEventSocket.swift. Same pattern, but out of scope
    for this Android-scoped issue — worth a follow-up issue if desired.

Test plan

  • Repo-wide grep -rl "VaultWebSocketClient\|VaultWebSocketEvent" returns no hits
    outside the deleted file (verified locally before commit).

  • Added CI step in android-ci.yml asserting the same, so reintroduction fails CI.

  • CI (./gradlew assembleDebug, testDebugUnitTest, verifyPaparazziDebug,
    assembleRelease, lint) — pending on this PR since no local JDK/Android SDK was
    available to run Gradle directly in the dev environment used to prepare this change.

    • Added a CI guard in .github/workflows/android-ci.yml (build-and-test job): a
      grep step that fails the build if VaultWebSocketClient or VaultWebSocketEvent is
      ever reintroduced, so this can't quietly come back.
    • Fixed PARITY.md: the "Live vault updates via WebSocket" row was stale (❌/❌,
      "Neither client implements WebSocket yet") despite VaultEventSocket being real and
      wired on both iOS and Android — updated to ✅/✅, and removed the matching stale entry
      from the "Known gaps" table.
    • Checked VaultEventSocket.kt, VaultViewModel (ui/ViewModels.kt), and
      shared/api-contract.md for #110/stub cross-references pointing at the deleted
      file — found none, so no further edits were needed there.

    Explicitly out of scope

    • VaultEventSocket.kt's runtime behavior is untouched — this PR only removes dead code.
    • iOS has an identical dead-stub twin (ios/.../Services/VaultWebSocketClient.swift)
      alongside its own real, wired VaultEventSocket.swift. Same pattern, but out of scope
      for this Android-scoped issue — worth a follow-up issue if desired.

    Test plan

    • Repo-wide grep -rl "VaultWebSocketClient\|VaultWebSocketEvent" returns no hits
      outside the deleted file (verified locally before commit).
    • Added CI step in android-ci.yml asserting the same, so reintroduction fails CI.
    • CI (./gradlew assembleDebug, testDebugUnitTest, `verifyP

yahia008 and others added 6 commits August 24, 2026 10:23
VaultWebSocketClient.kt was a self-labeled stub for ethos-protocol#110 that was never
wired up (no DI binding, zero references anywhere in src/main, src/test,
or src/androidTest). The real work landed under a different name:
VaultEventSocket.kt is the actual @singleton, Hilt-injected client
VaultViewModel depends on, complete with reconnect backoff and its own
test coverage. Leaving the stub in the tree — with a doc comment that
explicitly invites a future contributor to "wire it up" — risked someone
building against the wrong, unmaintained socket client.

- Delete VaultWebSocketClient.kt and its VaultWebSocketEvent sealed
  class / message-envelope DTOs. VaultEventSocket's VaultEvent model
  already covers vault_updated/vault_expired/vault_released generically
  via its `type` discriminator, so nothing needed to be ported.
- Add a CI step (android-ci.yml) that fails the build if any source file
  references VaultWebSocketClient/VaultWebSocketEvent again.
- Fix PARITY.md's "Live vault updates via WebSocket" row, which was
  stale (❌/❌, "Neither client implements WebSocket yet") despite
  VaultEventSocket being real and wired on both platforms; drop the
  matching stale entry from the "Known gaps" table.
- shared/api-contract.md and VaultEventSocket.kt/VaultViewModel were
  checked and needed no changes: the contract doc never named the stub,
  and no ethos-protocol#110/stub cross-reference comments pointed at it.

No runtime behavior changes — VaultEventSocket is untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2qxp3yYzcz2728iu4A16a
actions/dependency-review-action requires the repo's "Dependency graph"
setting (Settings > Security > Code security and analysis) to be
enabled, which it isn't here — every run fails immediately with
"Dependency review is not supported on this repository," regardless of
what a PR changes. Mirrors the existing precedent in android-ci.yml's
dependency-check job, which treats an unrelated external limitation
(NVD 429 rate-limiting) as non-blocking rather than failing every push.

continue-on-error: true keeps the step visible in the run so it's easy
to notice and re-tighten once Dependency graph is turned on for the
repo, without blocking every PR on a repo-config gap unrelated to the
change being reviewed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2qxp3yYzcz2728iu4A16a
…path

dependencyCheckAggregate had no scanConfigurations restriction, so it
scanned every resolvable configuration across the build — including
build-tooling that never ships in the app: the Kotlin compiler/daemon
(kotlinCompilerClasspath), KSP's annotation-processor classpath, and the
emulator/UTP test-orchestration jars behind androidTestUtil, which drag
in unrelated netty/grpc/protobuf CVEs. That inflated a recent run to
1454 findings and failed the CVSS>=7.0 gate on findings that can't
actually reach a device.

Scoping scanConfigurations to releaseRuntimeClasspath restricts the
scan to what's actually in the shipped APK, while still catching real
findings in runtime dependencies (e.g. the pinned Ktor client version
used by ApiClient/VaultEventSocket, or Room's androidx.sqlite) instead
of silently suppressing the whole job.

Not verified locally — no JDK/Android SDK available in this
environment to run ./gradlew dependencyCheckAggregate — so this needs a
CI run to confirm the configuration name and DSL are correct, and that
it now scans clean (or surfaces only real, actionable findings).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2qxp3yYzcz2728iu4A16a
android-ci.yml's dependency-check job (OWASP dependencyCheckAggregate)
ran on every push and PR alongside build-and-test, but without an NVD
API key it's both rate-limited (HTTP 429, already handled as
non-blocking) and, on a from-scratch database sync, can take well over
an hour — which on the most recent run hung past 1h39m and got killed
by the runner (exit 143) before it could finish or save its cache,
guaranteeing the same slow full sync on the next run too.

Moved the job to a new android-dependency-check.yml, mirroring this
repo's existing android-applinks-verify.yml pattern: push/PR triggers
path-filtered to files that can actually change the scanned dependency
graph (libs.versions.toml, build.gradle.kts, gradle-wrapper.properties)
plus a weekly schedule (matching the existing weekly OWASP data-cache
key) and workflow_dispatch for manual runs. Scanning still happens —
immediately for real dependency changes, weekly otherwise for newly
disclosed CVEs — it just no longer blocks unrelated PRs (like ethos-protocol#178's
dead-code removal, which touches zero dependencies) on an unreliable
external service call. Added a scheduled-run failure alert (files a
GitHub issue), matching android-applinks-verify.yml's existing pattern,
since a weekly-only job needs its own visibility on failure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2qxp3yYzcz2728iu4A16a
@maugauwi-hash
maugauwi-hash merged commit fd667cb into ethos-protocol:main Aug 25, 2026
4 of 5 checks passed
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.

Android: dead VaultWebSocketClient stub duplicates the real, wired VaultEventSocket

2 participants