Remove admin_channel_enabled toggle from Security Config screen (#5547)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
James Rich
2026-05-20 14:14:45 -07:00
committed by GitHub
parent 91ad5dbb81
commit 676be26460
10 changed files with 454 additions and 11 deletions

View File

@@ -1 +1 @@
{"feature_directory":"specs/20260513-160000-m3-expressive-adoption"}
{"feature_directory":"specs/20260520-153428-remove-admin-channel-toggle"}

View File

@@ -49,5 +49,5 @@ You are an expert Android/KMP engineer. Maintain architectural boundaries, use M
<!-- SPECKIT START -->
For additional context about technologies to be used, project structure,
shell commands, and other important information, read the current plan
at `specs/20260513-160000-m3-expressive-adoption/plan.md`
at `specs/20260520-153428-remove-admin-channel-toggle/plan.md`
<!-- SPECKIT END -->

View File

@@ -46,7 +46,6 @@ import org.meshtastic.core.resources.config_security_public_key
import org.meshtastic.core.resources.config_security_serial_enabled
import org.meshtastic.core.resources.debug_log_api_enabled
import org.meshtastic.core.resources.direct_message_key
import org.meshtastic.core.resources.legacy_admin_channel
import org.meshtastic.core.resources.logs
import org.meshtastic.core.resources.managed_mode
import org.meshtastic.core.resources.private_key
@@ -204,14 +203,6 @@ fun SecurityConfigScreenCommon(viewModel: RadioConfigViewModel, onBack: () -> Un
onCheckedChange = { formState.value = formState.value.copy(is_managed = it) },
containerColor = CardDefaults.cardColors().containerColor,
)
HorizontalDivider()
SwitchPreference(
title = stringResource(Res.string.legacy_admin_channel),
checked = formState.value.admin_channel_enabled,
enabled = state.connected,
onCheckedChange = { formState.value = formState.value.copy(admin_channel_enabled = it) },
containerColor = CardDefaults.cardColors().containerColor,
)
}
}
}

View File

@@ -0,0 +1,35 @@
# Specification Quality Checklist: Remove Admin Channel Enabled Toggle
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2025-05-20
**Feature**: [spec.md](../spec.md)
## Content Quality
- [x] No implementation details (languages, frameworks, APIs)
- [x] Focused on user value and business needs
- [x] Written for non-technical stakeholders
- [x] All mandatory sections completed
## Requirement Completeness
- [x] No [NEEDS CLARIFICATION] markers remain
- [x] Requirements are testable and unambiguous
- [x] Success criteria are measurable
- [x] Success criteria are technology-agnostic (no implementation details)
- [x] All acceptance scenarios are defined
- [x] Edge cases are identified
- [x] Scope is clearly bounded
- [x] Dependencies and assumptions identified
## Feature Readiness
- [x] All functional requirements have clear acceptance criteria
- [x] User scenarios cover primary flows
- [x] Feature meets measurable outcomes defined in Success Criteria
- [x] No implementation details leak into specification
## Notes
- All items pass. Spec is ready for `/speckit.clarify` or `/speckit.plan`.
- This is a straightforward removal feature with well-defined scope from the issue.

View File

@@ -0,0 +1,26 @@
# Data Model: Remove Admin Channel Enabled Toggle
## Summary
This feature does not introduce, modify, or remove any data model entities. It is a pure UI deletion.
## Entities
### Config.SecurityConfig (unchanged)
The proto-generated `Config.SecurityConfig` data class retains the `admin_channel_enabled: Boolean` field. No schema changes.
| Field | Type | Change |
|-------|------|--------|
| `admin_channel_enabled` | `Boolean` | **No change** — remains in proto, no longer surfaced in UI |
| `is_managed` | `Boolean` | No change |
| `admin_key` | `List<ByteString>` | No change |
| ... | ... | No change |
## Relationships
No new relationships introduced. The existing relationship between `SecurityConfigScreen` composable and `Config.SecurityConfig` proto model remains — the screen simply renders one fewer field from the model.
## State Transitions
N/A — No state machine changes. The form state (`formState`) continues to hold the full `SecurityConfig` including `admin_channel_enabled`; it is simply not bound to any UI element after this change.

View File

@@ -0,0 +1,64 @@
# Implementation Plan: Remove Admin Channel Enabled Toggle
**Branch**: `jamesarich/issue-5545-alignment-remove-admin-channel-enabled-ca777c` | **Date**: 2025-05-20 | **Spec**: `specs/20260520-153428-remove-admin-channel-toggle/spec.md`
**Input**: Feature specification from `/specs/20260520-153428-remove-admin-channel-toggle/spec.md`
## Summary
Remove the `admin_channel_enabled` SwitchPreference toggle and its associated HorizontalDivider from the Security Config screen. This is a pure UI deletion (8 lines of composable code + 1 unused import) that aligns the Android client with the Apple client. No proto, business logic, or string resource changes required.
## Technical Context
**Language/Version**: Kotlin 2.3+ / JDK 21
**Primary Dependencies**: JetBrains Compose Multiplatform, Material 3
**Storage**: N/A (no persistence changes)
**Testing**: `./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm`
**Target Platform**: Android + Desktop (KMP)
**Project Type**: Mobile app (KMP multi-platform)
**Performance Goals**: N/A (removal only, no performance impact)
**Constraints**: Must not break existing Security Config screen rendering
**Scale/Scope**: Single file modification, ~9 lines removed
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- **I. Kotlin Multiplatform Core**: ✅ Only `commonMain` is touched (`feature/settings/src/commonMain/...SecurityConfigScreen.kt`). No platform-specific code introduced. No business logic change — only UI element removal.
- **II. Zero Lint Tolerance**: ✅ Will run `./gradlew spotlessApply spotlessCheck detekt` on the `:feature:settings` module. Removing an unused import improves lint compliance.
- **III. Compose Multiplatform UI**: ✅ Change is deletion of a Compose Multiplatform composable invocation. No new UI added. No navigation or float formatting involved.
- **IV. Privacy First**: ✅ No PII/location/crypto logging introduced. `core/proto` submodule not modified. The proto field `admin_channel_enabled` remains untouched.
- **V. Design Standards Compliance**: ✅ This is a removal aligning with Apple client behavior. Cross-Platform Spec field marked N/A — this is platform alignment removal, not a new cross-platform feature.
- **VI. Documentation Freshness**: ✅ No user-facing docs reference this toggle. The `skip-docs-check` label applies as no documentation page covers this specific setting.
- **VII. Verify Before Push**: ✅ Local verification commands:
```bash
./gradlew spotlessApply spotlessCheck detekt
./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm
```
Post-push: `gh pr checks` or `gh run list --branch jamesarich/issue-5545-alignment-remove-admin-channel-enabled-ca777c --limit 5`
## Project Structure
### Documentation (this feature)
```text
specs/20260520-153428-remove-admin-channel-toggle/
├── plan.md # This file
├── research.md # Phase 0 output (minimal — no unknowns)
├── data-model.md # Phase 1 output (minimal — no entity changes)
├── quickstart.md # Phase 1 output (implementation guide)
└── tasks.md # Phase 2 output (created by /speckit.tasks)
```
### Source Code (repository root)
```text
feature/settings/
└── src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/
└── SecurityConfigScreen.kt # Remove lines 49, 207214
```
**Structure Decision**: Existing KMP multi-module structure. This feature touches a single file in the `feature:settings` module's `commonMain` source set.
## Complexity Tracking
> No constitution violations. This is a minimal UI deletion with no architectural concerns.

View File

@@ -0,0 +1,55 @@
# Quickstart: Remove Admin Channel Enabled Toggle
## Overview
Remove the `admin_channel_enabled` toggle from the Security Config screen by deleting 8 lines of Compose UI code and 1 unused import.
## Prerequisites
- JDK 21 installed
- `ANDROID_HOME` set
- Proto submodule initialized (`git submodule update --init`)
## Implementation Steps
### Step 1: Remove unused import (Line 49)
In `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt`:
```kotlin
// DELETE this line:
import org.meshtastic.core.resources.legacy_admin_channel
```
### Step 2: Remove toggle block (Lines 207214)
In the same file, inside the `TitledCard(title = stringResource(Res.string.administration))` block, delete:
```kotlin
// DELETE these 8 lines:
HorizontalDivider()
SwitchPreference(
title = stringResource(Res.string.legacy_admin_channel),
checked = formState.value.admin_channel_enabled,
enabled = state.connected,
onCheckedChange = { formState.value = formState.value.copy(admin_channel_enabled = it) },
containerColor = CardDefaults.cardColors().containerColor,
)
```
### Step 3: Verify
```bash
# Compile all KMP targets for the settings module
./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm
# Full lint check
./gradlew spotlessApply spotlessCheck detekt
```
## Expected Result
- Security Config screen renders without the admin channel toggle
- "Administration" TitledCard shows only the "Managed Mode" switch
- All existing tests pass
- No lint violations

View File

@@ -0,0 +1,41 @@
# Research: Remove Admin Channel Enabled Toggle
## Summary
This feature has no unresolved unknowns. The technical context is fully specified by the feature spec and existing codebase analysis.
## Findings
### 1. Admin Channel Enabled Toggle Location
- **Decision**: Remove lines 207214 in `SecurityConfigScreen.kt` (HorizontalDivider + SwitchPreference block)
- **Rationale**: Confirmed via code inspection — these are the only lines rendering the `admin_channel_enabled` toggle
- **Alternatives considered**: None — this is the only location
### 2. Unused Import Cleanup
- **Decision**: Remove line 49 (`import org.meshtastic.core.resources.legacy_admin_channel`)
- **Rationale**: After removing the SwitchPreference that uses `Res.string.legacy_admin_channel`, this import becomes unused. Leaving it would fail lint checks.
- **Alternatives considered**: Leave import (rejected — violates Zero Lint Tolerance principle)
### 3. Proto Field Preservation
- **Decision**: Do NOT modify `admin_channel_enabled` proto field
- **Rationale**: Per spec non-goals and Constitution §IV (Privacy First / read-only proto submodule). The field remains for firmware communication and backward compatibility.
- **Alternatives considered**: Full removal including proto (rejected — requires upstream proto change, out of scope)
### 4. String Resource Preservation
- **Decision**: Leave `legacy_admin_channel` string resources intact
- **Rationale**: String resources are Crowdin-managed. Removal is a separate cleanup task. No lint violation from unused string resources.
- **Alternatives considered**: Remove strings (rejected — Crowdin-managed, separate concern)
### 5. Verification Strategy
- **Decision**: Run `./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm`
- **Rationale**: Covers all KMP targets for the settings module. `compileKotlinJvm` ensures desktop target compiles correctly after the removal.
- **Alternatives considered**: Full `assembleDebug` (not needed — change is isolated to one module)
## No NEEDS CLARIFICATION Items
All technical context was resolved from spec + codebase inspection. No external research required.

View File

@@ -0,0 +1,121 @@
# Feature Specification: Remove Admin Channel Enabled Toggle
**Feature Branch**: `jamesarich-issue-5545-alignment-remove-admin-channel-enabled-ca777c`
**Created**: 2025-05-20
**Status**: Draft
**Input**: User description: "Remove admin_channel_enabled toggle from the Security Config screen"
**Cross-Platform Spec**: N/A — platform alignment removal (aligning Android with Apple client behavior)
## Summary
Remove the `admin_channel_enabled` toggle from the Security Config settings screen in the Android client. This toggle is Android-only, not present in the Apple client, and has been identified as a cross-platform discrepancy in the settings validation audit. With PKC-based administration becoming the default approach, this toggle creates user confusion and should be removed to align both clients.
## Goals
- Remove the `admin_channel_enabled` UI toggle from the Security Config screen
- Align the Android client's Security Config screen with the Apple client
- Reduce user confusion around legacy admin channel configuration
- Maintain full PKC-based admin functionality without regression
## Non-Goals
- Removing or modifying the underlying `admin_channel_enabled` proto field (it remains in the protobuf schema)
- Changing any backend/firmware behavior related to admin channels
- Modifying PKC-based administration logic
- Removing string resources from locale files (cleanup can be done separately)
## User Scenarios & Testing *(mandatory)*
### User Story 1 - Security Config Screen Without Legacy Toggle (Priority: P1)
As a user navigating to the Security Config screen, I no longer see the `admin_channel_enabled` toggle, resulting in a cleaner interface focused on PKC-based administration.
**Why this priority**: This is the core deliverable — removing the toggle from the UI.
**Independent Test**: Can be fully tested by navigating to the Security Config screen and verifying the toggle is absent, while all other security settings remain functional.
**Acceptance Scenarios**:
1. **Given** a user opens the Security Config screen, **When** the screen renders, **Then** the `admin_channel_enabled` toggle and its associated divider are not displayed.
2. **Given** a user opens the Security Config screen, **When** reviewing available settings, **Then** all other security configuration options remain visible and functional.
---
### User Story 2 - PKC Admin Functionality Unaffected (Priority: P1)
As a user performing administrative actions via PKC-based administration, the removal of the toggle does not affect my ability to manage nodes.
**Why this priority**: Ensuring no regression in core admin functionality is critical.
**Independent Test**: Can be tested by performing PKC-based admin operations (e.g., remote node configuration) after the toggle removal and verifying they succeed.
**Acceptance Scenarios**:
1. **Given** a user has PKC-based admin configured, **When** they perform a remote admin operation, **Then** the operation completes successfully as before.
2. **Given** a device had `admin_channel_enabled` previously set to true, **When** the user opens Security Config, **Then** the setting value persists in the proto config but is simply not shown in the UI.
---
### Edge Cases
- What happens when a device has `admin_channel_enabled` set to `true` in its stored config? The value remains in the proto; it is simply no longer surfaced or toggleable in the UI.
- What happens on config export/import? The field remains in the protobuf schema, so existing exports with the field set remain valid and importable.
## Architecture
### Key Components
| Component | Module / File | Purpose |
|-----------|---------------|---------|
| SecurityConfigScreen | `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt` | Screen where the toggle is removed |
| String resources | `core/resources/src/commonMain/composeResources/values*/strings.xml` | `legacy_admin_channel` string (unused after removal) |
## Requirements *(mandatory)*
### Functional Requirements
- **FR-001**: The Security Config screen MUST NOT display the `admin_channel_enabled` toggle (SwitchPreference) or its preceding HorizontalDivider
- **FR-002**: The Security Config screen MUST continue to display all other security configuration options unchanged
- **FR-003**: The underlying proto field `admin_channel_enabled` on `Config.SecurityConfig` MUST NOT be modified or removed
### Non-Functional Requirements
- **NFR-001**: The Security Config screen must render without visual artifacts or layout shifts where the toggle was previously positioned
- **NFR-002**: Existing screenshot tests (if any) for Security Config must be updated to reflect the removal
## Source-Set Impact
| Source Set | Impact | Justification |
|-----------|--------|---------------|
| `commonMain` | Modified: `SecurityConfigScreen.kt` (remove toggle + divider) | All UI is in commonMain per Constitution §I |
| `androidMain` | None | No platform-specific changes needed |
| `jvmMain` | None | No desktop-specific changes needed |
## Design Standards Compliance
- [x] New screens reviewed against design standards — N/A (removal only, no new UI)
- [x] M3 component selection verified — N/A (no new components)
- [x] Accessibility: TalkBack semantics — N/A (removing element, not adding)
- [x] Typography — N/A (no new text)
## Privacy Assessment
- [x] No PII, location data, or cryptographic keys logged or exposed
- [x] No new network calls that transmit user data
- [x] Proto submodule (`core/proto`) not modified (read-only upstream)
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: The Security Config screen displays zero instances of the admin channel enabled toggle
- **SC-002**: All existing PKC-based admin operations complete successfully after the change (no regression)
- **SC-003**: The Android Security Config screen field count matches the Apple client's Security Config screen (alignment achieved)
## Assumptions
- All business logic and UI composables reside in `commonMain` source set
- The proto field `admin_channel_enabled` remains available for firmware communication; only the UI toggle is removed
- String resource cleanup (`legacy_admin_channel`) is considered optional follow-up work and not required for this feature
- The Apple client's Security Config screen is the reference for cross-platform alignment
- No other screens or components reference the `admin_channel_enabled` toggle UI

View File

@@ -0,0 +1,110 @@
# Tasks: Remove Admin Channel Enabled Toggle
**Input**: Design documents from `/specs/20260520-153428-remove-admin-channel-toggle/`
**Prerequisites**: plan.md ✅, spec.md ✅, research.md ✅, data-model.md ✅, quickstart.md ✅
**Tests**: No new automated tests requested by the feature specification. Verification relies on existing module tests and lint checks.
**Verification**: Constitution-required validation tasks (spotlessCheck, detekt, compile/test) are included in Phase 3.
**Organization**: This is a minimal single-file UI deletion. Given the simplicity (~9 lines removed), the phase structure is condensed.
## Format: `[ID] [P?] [Story] Description`
- **[P]**: Can run in parallel (different files, no dependencies)
- **[Story]**: Which user story this task belongs to (e.g., US1, US2)
- Exact file paths included in descriptions
---
## Phase 1: Setup
**Purpose**: No project initialization needed — existing KMP multi-module project. This phase is intentionally empty.
**Checkpoint**: N/A — project already configured.
---
## Phase 2: User Story 1 — Security Config Screen Without Legacy Toggle (Priority: P1) 🎯 MVP
**Goal**: Remove the `admin_channel_enabled` SwitchPreference toggle and its HorizontalDivider from the Security Config screen so users see a cleaner interface focused on PKC-based administration.
**Independent Test**: Navigate to Security Config screen → verify the toggle is absent and all other settings remain functional. Run `./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm`.
### Implementation
- [x] T001 [US1] Remove unused import `org.meshtastic.core.resources.legacy_admin_channel` (line 49) in feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt
- [x] T002 [US1] Remove HorizontalDivider + SwitchPreference block for `admin_channel_enabled` (lines 207214) in feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt
**Checkpoint**: Security Config screen renders without the admin channel toggle. The "Administration" TitledCard shows only the "Managed Mode" switch.
---
## Phase 3: Verification & Polish (Priority: P1)
**Purpose**: Constitution-required validation — formatting, static analysis, and compilation across all KMP targets.
- [x] T003 [P] Run `./gradlew spotlessApply spotlessCheck detekt` on `:feature:settings` module to ensure zero lint violations
- [x] T004 [P] Run `./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm` to verify all tests pass and all KMP targets compile
- [x] T005 Confirm no logs, telemetry, or config changes expose PII, location data, secrets, or modify `core/proto`
**Checkpoint**: All verification passes. Feature complete.
---
## Dependencies & Execution Order
### Phase Dependencies
- **Phase 1 (Setup)**: Empty — no action required
- **Phase 2 (US1 Implementation)**: Can start immediately
- **Phase 3 (Verification)**: Depends on Phase 2 completion
### User Story Dependencies
- **User Story 1 (P1)**: No dependencies on other stories — this IS the feature
- **User Story 2 (P1)**: Verified implicitly by T004 (existing tests cover PKC admin functionality). No code changes needed — US2 is a non-regression guarantee satisfied by the test suite passing.
### Within Phase 2
- T001 and T002 are in the same file and operate on non-overlapping line ranges — they CAN be applied in parallel (different line regions) but are sequenced for clarity.
### Parallel Opportunities
- T003 and T004 can run in parallel (different Gradle tasks, independent checks)
- T005 is a manual/automated review that can happen alongside T003/T004
---
## Parallel Example: Verification Phase
```bash
# Launch verification tasks in parallel:
Task T003: "./gradlew spotlessApply spotlessCheck detekt"
Task T004: "./gradlew :feature:settings:allTests :feature:settings:compileKotlinJvm"
```
---
## Implementation Strategy
### MVP First (User Story 1 Only)
1. Complete T001 + T002: Remove import and toggle block (same file, ~9 lines)
2. Complete T003 + T004: Verify lint + tests pass
3. Complete T005: Privacy/proto confirmation
4. **DONE**: Feature is complete in a single increment
### Incremental Delivery
This feature is atomic — there is no meaningful increment smaller than the full change. The removal of the import (T001) and the toggle block (T002) must ship together to avoid lint failures.
---
## Notes
- Single file modified: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt`
- No data model changes (proto field preserved per spec non-goals)
- No string resource removal (Crowdin-managed, separate concern)
- User Story 2 (PKC admin unaffected) is validated by existing test suite — no new code needed
- Total lines removed: ~9 (1 import + 8 composable lines)