Skip to content

Document in-app feedback sheet design and implementation plan#467

Merged
rainxchzed merged 23 commits intomainfrom
feat/in-app-feedback
Apr 29, 2026
Merged

Document in-app feedback sheet design and implementation plan#467
rainxchzed merged 23 commits intomainfrom
feat/in-app-feedback

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented Apr 29, 2026

Summary by CodeRabbit

  • New Features

    • In-app "Send feedback" sheet under Tweaks/About: structured feedback (category/topic, title, description, conditional fields), optional diagnostics preview, and send via Email or GitHub with success/error snackbars.
    • New localized strings for all feedback UI labels and messages.
  • Bug Fixes

    • Safer browser launch: failures when opening send URLs now surface a retryable error.
  • Documentation

    • Added rollout plan and design spec for the feedback feature.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7712499c-89e2-408c-9f94-38ecc963c87e

📥 Commits

Reviewing files that changed from the base of the PR and between dd1b36b and d5461f0.

📒 Files selected for processing (4)
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/utils/AndroidBrowserHelper.kt
  • docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/TopicSelector.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/util/FeedbackComposer.kt

Walkthrough

Adds a complete in-app "Send feedback" feature: platform helpers for OS/locale, MVI (state/action/event/viewmodel), feedback models/enums, a FeedbackComposer utility, Compose UI bottom sheet and components, DI registration, i18n strings, build dependency, and documentation.

Changes

Cohort / File(s) Summary
Platform Abstractions
core/domain/src/commonMain/kotlin/.../Platform.kt, core/domain/src/androidMain/kotlin/.../Platform.android.kt, core/domain/src/jvmMain/kotlin/.../Platform.jvm.kt
Added expect APIs getOsVersion() and getSystemLocaleTag() and platform actual implementations for Android and JVM.
DI Registration
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/di/ViewModelsModule.kt
Registered FeedbackViewModel in the Koin viewModels module.
Feedback Models & Enums
feature/tweaks/presentation/src/commonMain/kotlin/.../feedback/model/*
Added FeedbackCategory, FeedbackTopic, FeedbackChannel enums and DiagnosticsInfo data class for diagnostics metadata and GitHub labels.
Feedback MVI
feature/tweaks/presentation/src/commonMain/kotlin/.../feedback/FeedbackState.kt, .../FeedbackAction.kt, .../FeedbackEvent.kt, .../FeedbackViewModel.kt
New MVI pieces: FeedbackState (fields, canSend), FeedbackAction (field updates, toggle, send/dismiss), FeedbackEvent (OnSent/OnSendError), and FeedbackViewModel (diagnostics collection, compose+open URL, event channel).
Compose UI: Bottom Sheet & Components
feature/tweaks/presentation/src/commonMain/kotlin/.../feedback/components/*
Added FeedbackBottomSheet and composables: CategorySelector, TopicSelector, ConditionalFields, DiagnosticsPreview, SendActions wiring to viewmodel actions/events.
Feedback Composer Utility
feature/tweaks/presentation/src/commonMain/kotlin/.../feedback/util/FeedbackComposer.kt
New composer to build Markdown body, truncate to 7,500 chars, and generate mailto: or GitHub issue creation URLs with labels.
Tweaks Integration
feature/tweaks/presentation/src/commonMain/kotlin/.../TweaksAction.kt, .../TweaksState.kt, .../TweaksViewModel.kt, .../TweaksRoot.kt, .../components/sections/About.kt
Added feedback open/dismiss actions and state, show bottom sheet in TweaksRoot, replace About "help/support" row with "Send feedback", and handle snackbar messages on result.
Strings & Build
core/presentation/src/commonMain/composeResources/values/strings.xml, feature/tweaks/presentation/build.gradle.kts
Added ~30+ feedback-related i18n strings; added ktor-client-core dependency to tweaks presentation module.
Docs
docs/superpowers/plans/2026-04-29-tweaks-feedback.md, docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md
Added implementation plan and design spec detailing feature behavior, diagnostics rules, composer contract, and wiring checklist.
Android Browser Helper
core/data/src/androidMain/kotlin/.../AndroidBrowserHelper.kt
openUrl now catches exceptions when starting the intent and forwards an error message to onFailure.

Sequence Diagram

sequenceDiagram
    actor User
    participant UI as FeedbackBottomSheet
    participant FVM as FeedbackViewModel
    participant Composer as FeedbackComposer
    participant BH as BrowserHelper
    participant Browser as External Browser/Email

    User->>UI: Fill form & tap Send
    UI->>FVM: FeedbackAction.OnSendViaEmail/Github
    FVM->>Composer: composeUrl(state, channel)
    Composer->>Composer: build body + truncate
    Composer-->>FVM: return URL
    FVM->>BH: openUrl(url, onFailure)
    BH->>Browser: launch intent / mailto
    Browser-->>BH: success / exception
    BH-->>FVM: success / onFailure(message)
    FVM->>FVM: emit FeedbackEvent.OnSent / OnSendError
    FVM-->>UI: event observed -> call onSent/onError
    UI->>User: show snackbar / error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I stitched a sheet for users to say,
With categories, topics, and diagnostics on display,
A composer hums, the viewmodel sings,
Links fly out on feathered wings —
Hop, click, send: the feedback finds its way.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The pull request title accurately describes the primary change: comprehensive documentation for in-app feedback feature design and implementation plan. However, the changeset includes substantial implementation code (ViewModels, UI components, models, enums) beyond just documentation files. Clarify whether the title should emphasize the complete feature implementation (design, models, ViewModels, UI components) or just the documentation aspects. Consider a more specific title like 'Implement in-app feedback sheet with design and plan docs' if implementation is primary.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 feat/in-app-feedback

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

Copy link
Copy Markdown
Contributor

@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: 5

🧹 Nitpick comments (4)
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/TopicSelector.kt (1)

34-47: Add selection-group semantics for topic chips.

Line 34 defines a single-choice topic group; adding selectableGroup() improves accessibility semantics for assistive tech.

Proposed diff
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.selection.selectableGroup
@@
         FlowRow(
             modifier = Modifier
                 .fillMaxWidth()
+                .selectableGroup()
                 .padding(top = 8.dp),
             horizontalArrangement = Arrangement.spacedBy(8.dp),
             verticalArrangement = Arrangement.spacedBy(8.dp),
         ) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/TopicSelector.kt`
around lines 34 - 47, Wrap the FlowRow used to render the topic chips with
selectableGroup() to expose single-selection semantics to assistive tech: locate
the FlowRow in TopicSelector.kt that iterates FeedbackTopic.entries and add the
selectableGroup() modifier on its Modifier chain (the one currently calling
fillMaxWidth() and padding()). This ensures the FilterChip group (each
FilterChip using selected and onSelected) is announced as a selectable
single-choice group to accessibility services.
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/FeedbackBottomSheet.kt (1)

63-66: Deduplicate sheet-dismiss behavior.

Both dismiss paths execute identical logic. Extract a local dismissSheet() lambda to avoid drift between the system dismiss and icon-button dismiss flows.

Also applies to: 89-92

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

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/FeedbackBottomSheet.kt`
around lines 63 - 66, Duplicate dismiss logic exists in the sheet's
onDismissRequest and the icon-button dismiss flow: both call
viewModel.onAction(FeedbackAction.OnDismiss) then onDismiss(). Extract a local
dismissSheet() lambda (or val) that calls
viewModel.onAction(FeedbackAction.OnDismiss) and onDismiss(), then replace the
body of onDismissRequest and the icon-button's onClick/handler with a call to
dismissSheet() to ensure a single source of truth.
docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md (1)

39-59: Add language identifiers to fenced code blocks.

These fences currently violate markdownlint MD040 and should be tagged (for example, text, kotlin, or mermaid where appropriate).

Also applies to: 63-70, 167-196, 226-246

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

In `@docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md` around lines 39
- 59, The markdown in the spec file contains fenced code blocks without language
identifiers (violating MD040); update each fence shown (the file tree and the
other blocks at lines noted) to include an appropriate language tag (e.g., use
"text" for the directory tree, "kotlin" for filenames/snippets like
FeedbackViewModel.kt, FeedbackComposer.kt, FeedbackState.kt, and "mermaid" where
diagrams apply) so all fenced blocks are tagged accordingly and linting passes.
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/DiagnosticsPreview.kt (1)

78-88: Avoid duplicated diagnostics formatting logic.

formatDiagnostics duplicates composer-side diagnostics rendering, so preview and actual payload can diverge over time. Consider moving this to a shared formatter used by both DiagnosticsPreview and FeedbackComposer.

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

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/DiagnosticsPreview.kt`
around lines 78 - 88, formatDiagnostics in DiagnosticsPreview duplicates
rendering logic used by FeedbackComposer; extract this logic into a single
shared formatter (e.g., a top-level function or a DiagnosticsFormatter class) in
a common/shared module and have both DiagnosticsPreview (the current
formatDiagnostics) and FeedbackComposer call that shared formatter;
specifically, move the StringBuilder construction and field concatenation
(appVersion, platform, osVersion, locale, installerType, githubUsername
conditional on FeedbackChannel.GITHUB) into the shared function (e.g.,
formatDiagnosticsShared or DiagnosticsFormatter.format), replace the current
private formatDiagnostics in DiagnosticsPreview with a call to that shared
function, and update FeedbackComposer to use the same shared API so there is a
single source of truth for diagnostics formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/superpowers/plans/2026-04-29-tweaks-feedback.md`:
- Around line 1590-1591: Standardize the spelling of "prefilled/pre-filled"
across the plan by choosing one form (e.g., "prefilled") and updating both
occurrences in the two list items (Step 5: "Type a title and description, tap
'Send Email' → your default mail client opens with a prefilled draft to
`hello@github-store.org`" and Step 6: "Reopen the sheet, tap 'Open as GitHub
Issue' → browser opens to a pre-filled `OpenHub-Store/GitHub-Store` new-issue
page with labels") so both use the same chosen spelling consistently.

In `@docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md`:
- Around line 155-158: The FeedbackEvent sealed interface currently declares a
channel-agnostic success object (FeedbackEvent.OnSent) which conflicts with the
implementation's channel-specific success handling; update the spec to make the
success event carry the channel (e.g., replace data object OnSent with a data
class OnSent(val channel: FeedbackChannel) : FeedbackEvent or similar) and
ensure the spec still includes OnSendError(message: String) so implementers can
wire channel-aware success messaging consistently with the code paths that
dispatch per-channel success.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackViewModel.kt`:
- Around line 37-40: In the init block where viewModelScope.launch updates
_state with diagnostics, guard the call to collectDiagnostics() using
runCatching so any exception doesn’t cancel initialization; on success set
diagnostics to the collected value, and on failure set diagnostics = null (i.e.,
_state.update { it.copy(diagnostics = result.getOrNull()) } or use
runCatching(...).getOrNull()), ensuring collectDiagnostics() failures are
swallowed and diagnostics stays null rather than propagating.
- Around line 73-95: The send() coroutine should be made exception-safe: wrap
the work after setting _state.update { isSending = true } in a try/finally so
_state.update { isSending = false } always runs, catch exceptions from
FeedbackComposer.composeUrl(...) and browserHelper.openUrl(...) and route them
to _events.send(FeedbackEvent.OnSendError(...)) and set a local failed flag so
the OnSent path is not taken; ensure you still call resetForm() only on success.
Also harden AndroidBrowserHelper so its startActivity/openUrl path invokes the
provided onFailure callback when startActivity throws or fails (and surface that
error back to the FeedbackViewModel via the same onFailure callback used by
browserHelper.openUrl).

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/util/FeedbackComposer.kt`:
- Around line 67-70: The truncateToCap extension currently slices to
BODY_MAX_CHARS and then appends the suffix, exceeding the cap; change
String.truncateToCap to compute the suffix (e.g., "\n\n…[truncated]") and, when
length > BODY_MAX_CHARS, return either suffix.take(BODY_MAX_CHARS) if
BODY_MAX_CHARS <= suffix.length or substring(0, BODY_MAX_CHARS - suffix.length)
+ suffix otherwise, so the final result from truncateToCap never exceeds
BODY_MAX_CHARS.

---

Nitpick comments:
In `@docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md`:
- Around line 39-59: The markdown in the spec file contains fenced code blocks
without language identifiers (violating MD040); update each fence shown (the
file tree and the other blocks at lines noted) to include an appropriate
language tag (e.g., use "text" for the directory tree, "kotlin" for
filenames/snippets like FeedbackViewModel.kt, FeedbackComposer.kt,
FeedbackState.kt, and "mermaid" where diagrams apply) so all fenced blocks are
tagged accordingly and linting passes.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/DiagnosticsPreview.kt`:
- Around line 78-88: formatDiagnostics in DiagnosticsPreview duplicates
rendering logic used by FeedbackComposer; extract this logic into a single
shared formatter (e.g., a top-level function or a DiagnosticsFormatter class) in
a common/shared module and have both DiagnosticsPreview (the current
formatDiagnostics) and FeedbackComposer call that shared formatter;
specifically, move the StringBuilder construction and field concatenation
(appVersion, platform, osVersion, locale, installerType, githubUsername
conditional on FeedbackChannel.GITHUB) into the shared function (e.g.,
formatDiagnosticsShared or DiagnosticsFormatter.format), replace the current
private formatDiagnostics in DiagnosticsPreview with a call to that shared
function, and update FeedbackComposer to use the same shared API so there is a
single source of truth for diagnostics formatting.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/FeedbackBottomSheet.kt`:
- Around line 63-66: Duplicate dismiss logic exists in the sheet's
onDismissRequest and the icon-button dismiss flow: both call
viewModel.onAction(FeedbackAction.OnDismiss) then onDismiss(). Extract a local
dismissSheet() lambda (or val) that calls
viewModel.onAction(FeedbackAction.OnDismiss) and onDismiss(), then replace the
body of onDismissRequest and the icon-button's onClick/handler with a call to
dismissSheet() to ensure a single source of truth.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/TopicSelector.kt`:
- Around line 34-47: Wrap the FlowRow used to render the topic chips with
selectableGroup() to expose single-selection semantics to assistive tech: locate
the FlowRow in TopicSelector.kt that iterates FeedbackTopic.entries and add the
selectableGroup() modifier on its Modifier chain (the one currently calling
fillMaxWidth() and padding()). This ensures the FilterChip group (each
FilterChip using selected and onSelected) is announced as a selectable
single-choice group to accessibility services.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a15514e7-044b-4f51-b7ad-78eb25a76e50

📥 Commits

Reviewing files that changed from the base of the PR and between 852d002 and dd1b36b.

📒 Files selected for processing (28)
  • composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/di/ViewModelsModule.kt
  • core/domain/src/androidMain/kotlin/zed/rainxch/core/domain/Platform.android.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/Platform.kt
  • core/domain/src/jvmMain/kotlin/zed/rainxch/core/domain/Platform.jvm.kt
  • core/presentation/src/commonMain/composeResources/values/strings.xml
  • docs/superpowers/plans/2026-04-29-tweaks-feedback.md
  • docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md
  • feature/tweaks/presentation/build.gradle.kts
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksRoot.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/About.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackAction.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackEvent.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackState.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackViewModel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/CategorySelector.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/ConditionalFields.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/DiagnosticsPreview.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/FeedbackBottomSheet.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/SendActions.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/components/TopicSelector.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/model/DiagnosticsInfo.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/model/FeedbackCategory.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/model/FeedbackChannel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/model/FeedbackTopic.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/util/FeedbackComposer.kt

Comment on lines +1590 to +1591
5. Type a title and description, tap "Send Email" → your default mail client opens with a prefilled draft to `hello@github-store.org`.
6. Reopen the sheet, tap "Open as GitHub Issue" → browser opens to a pre-filled `OpenHub-Store/GitHub-Store` new-issue page with labels.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use one spelling consistently for “prefilled/pre-filled”.

Line 1590 uses “prefilled” while Line 1591 uses “pre-filled”. Please standardize to one form in this plan.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~1590-~1590: Do not mix variants of the same word (‘prefilled’ and ‘pre-filled’) within a single text.
Context: ...→ your default mail client opens with a prefilled draft to hello@github-store.org. 6. R...

(EN_WORD_COHERENCY)

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

In `@docs/superpowers/plans/2026-04-29-tweaks-feedback.md` around lines 1590 -
1591, Standardize the spelling of "prefilled/pre-filled" across the plan by
choosing one form (e.g., "prefilled") and updating both occurrences in the two
list items (Step 5: "Type a title and description, tap 'Send Email' → your
default mail client opens with a prefilled draft to `hello@github-store.org`"
and Step 6: "Reopen the sheet, tap 'Open as GitHub Issue' → browser opens to a
pre-filled `OpenHub-Store/GitHub-Store` new-issue page with labels") so both use
the same chosen spelling consistently.

Comment thread docs/superpowers/specs/2026-04-29-tweaks-feedback-design.md
Comment on lines +37 to +40
init {
viewModelScope.launch {
_state.update { it.copy(diagnostics = collectDiagnostics()) }
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard diagnostics initialization against repository/platform failures.

collectDiagnostics() is called in init without failure handling. Any exception here can crash or silently cancel init work. Wrap it with runCatching and keep diagnostics = null on failure.

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

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackViewModel.kt`
around lines 37 - 40, In the init block where viewModelScope.launch updates
_state with diagnostics, guard the call to collectDiagnostics() using
runCatching so any exception doesn’t cancel initialization; on success set
diagnostics to the collected value, and on failure set diagnostics = null (i.e.,
_state.update { it.copy(diagnostics = result.getOrNull()) } or use
runCatching(...).getOrNull()), ensuring collectDiagnostics() failures are
swallowed and diagnostics stays null rather than propagating.

Comment on lines +73 to +95
private fun send(channel: FeedbackChannel) {
val current = _state.value
if (!current.canSend) return
_state.update { it.copy(isSending = true) }
viewModelScope.launch {
var failed = false
val url = FeedbackComposer.composeUrl(current, channel)
browserHelper.openUrl(url) { error ->
failed = true
viewModelScope.launch {
_events.send(FeedbackEvent.OnSendError(error))
}
}
// Hold the disabled state briefly so the user sees the
// buttons disable and can't double-tap; long enough to
// also let any synchronous onFailure invocation arrive.
delay(250)
_state.update { it.copy(isSending = false) }
if (!failed) {
_events.send(FeedbackEvent.OnSent(channel))
resetForm()
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== BrowserHelper contract =="
fd -i "BrowserHelper.kt" .
rg -n -C4 --type=kt '\binterface\s+BrowserHelper\b|\bfun\s+openUrl\s*\('

echo
echo "== Implementations / overrides =="
rg -n -C4 --type=kt ':\s*BrowserHelper\b|override\s+fun\s+openUrl\s*\('

echo
echo "== Async dispatch around onFailure/openUrl =="
rg -n -C4 --type=kt 'openUrl\s*\(|onFailure|launch\s*\{|async\s*\{|withContext\s*\('

Repository: OpenHub-Store/GitHub-Store

Length of output: 397


🏁 Script executed:

#!/bin/bash

echo "=== BrowserHelper Interface Definition ==="
cat ./core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/utils/BrowserHelper.kt

echo
echo "=== AndroidBrowserHelper Implementation ==="
cat ./core/data/src/androidMain/kotlin/zed/rainxch/core/data/utils/AndroidBrowserHelper.kt

echo
echo "=== DesktopBrowserHelper Implementation ==="
cat ./core/data/src/jvmMain/kotlin/zed/rainxch/core/data/utils/DesktopBrowserHelper.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 2081


🏁 Script executed:

#!/bin/bash

echo "=== FeedbackViewModel Full Content ==="
cat ./feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackViewModel.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 5816


send() method lacks exception safety and doesn't handle failures from AndroidBrowserHelper.

The send() method has two reliability issues:

  1. Unhandled exceptions: If FeedbackComposer.composeUrl() or browserHelper.openUrl() throw an exception, the coroutine crashes and leaves isSending = true permanently.

  2. AndroidBrowserHelper never invokes onFailure: It only calls context.startActivity() without error handling. If the intent fails to start, the callback is never invoked, leaving the UI in a sending state indefinitely.

Wrap the send logic in try/finally and ensure isSending is reset even when exceptions occur:

💡 Suggested fix
 private fun send(channel: FeedbackChannel) {
     val current = _state.value
     if (!current.canSend) return
     _state.update { it.copy(isSending = true) }
     viewModelScope.launch {
-        var failed = false
-        val url = FeedbackComposer.composeUrl(current, channel)
-        browserHelper.openUrl(url) { error ->
-            failed = true
-            viewModelScope.launch {
-                _events.send(FeedbackEvent.OnSendError(error))
-            }
-        }
-        delay(250)
-        _state.update { it.copy(isSending = false) }
-        if (!failed) {
-            _events.send(FeedbackEvent.OnSent(channel))
-            resetForm()
-        }
+        var failureMessage: String? = null
+        try {
+            val url = FeedbackComposer.composeUrl(current, channel)
+            browserHelper.openUrl(url) { error ->
+                failureMessage = error
+            }
+            delay(250)
+            val error = failureMessage
+            if (error != null) {
+                _events.send(FeedbackEvent.OnSendError(error))
+            } else {
+                _events.send(FeedbackEvent.OnSent(channel))
+                resetForm()
+            }
+        } catch (t: Throwable) {
+            _events.send(FeedbackEvent.OnSendError(t.message ?: "Failed to open feedback channel"))
+        } finally {
+            _state.update { it.copy(isSending = false) }
+        }
     }
 }

Also harden AndroidBrowserHelper to invoke the onFailure callback when startActivity() fails or throws an exception.

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

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/feedback/FeedbackViewModel.kt`
around lines 73 - 95, The send() coroutine should be made exception-safe: wrap
the work after setting _state.update { isSending = true } in a try/finally so
_state.update { isSending = false } always runs, catch exceptions from
FeedbackComposer.composeUrl(...) and browserHelper.openUrl(...) and route them
to _events.send(FeedbackEvent.OnSendError(...)) and set a local failed flag so
the OnSent path is not taken; ensure you still call resetForm() only on success.
Also harden AndroidBrowserHelper so its startActivity/openUrl path invokes the
provided onFailure callback when startActivity throws or fails (and surface that
error back to the FeedbackViewModel via the same onFailure callback used by
browserHelper.openUrl).

@rainxchzed rainxchzed merged commit be8fe60 into main Apr 29, 2026
1 check was pending
@rainxchzed rainxchzed deleted the feat/in-app-feedback branch April 29, 2026 13:03
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