docs(cr-2026-04-22): add release-blocker issue tracker at docs/issues/

Captures the 12 items from docs/code-review-2026-04-22.md that
must land before v0.1 ships. One markdown file per issue with:
severity, path:line, problem description, acceptance criteria,
fix scope, and dependency graph.

Split by severity:
- 3 CRITICAL: live-session race, migration atomicity, transcript-
  profile FK
- 9 MAJOR: monolith refactor, channel-fatality, capture worker
  join, runtime capabilities, macOS App Nap, decoder error prop,
  LLM prompt preflight, keystore thread-safety, hotkey device
  filter

README.md indexes them with a fix-order dependency graph and a
fish-shell script for bulk-converting to GitHub issues once `gh`
CLI is installed and authed. Deferred step by user decision —
markdown tracker is authoritative until then.
This commit is contained in:
2026-04-22 09:46:08 +01:00
parent fd24b81a5f
commit 592b894790
13 changed files with 383 additions and 0 deletions

64
docs/issues/README.md Normal file
View File

@@ -0,0 +1,64 @@
---
name: Release-blockers index
description: Open issues that must land before v0.1 ships, derived from the 2026-04-22 code review
type: index
tags: [issues, release-blockers]
---
# Release-blockers
Issues here must land before Kon v0.1 ships. Each is sourced from
`docs/code-review-2026-04-22.md`. When `gh` CLI is available, these
should be mirrored as real GitHub issues on `jakejars/kon`.
## CRITICAL (3)
| # | File | Area | Fix scope |
|---|---|---|---|
| RB-01 | [c1-live-session-race.md](c1-live-session-race.md) | `src-tauri/commands/live.rs` | large |
| RB-02 | [c3-migrations-atomicity.md](c3-migrations-atomicity.md) | `crates/storage/migrations.rs` | medium |
| RB-03 | [c4-transcript-profile-fk.md](c4-transcript-profile-fk.md) | `crates/storage/migrations.rs` + `database.rs` | large |
## MAJOR (9)
| # | File | Area | Fix scope |
|---|---|---|---|
| RB-04 | [run-live-session-monolith.md](run-live-session-monolith.md) | `src-tauri/commands/live.rs` | large |
| RB-05 | [poll-inference-channel-fatality.md](poll-inference-channel-fatality.md) | `src-tauri/commands/live.rs` | medium |
| RB-06 | [native-capture-worker-join.md](native-capture-worker-join.md) | `src-tauri/commands/audio.rs` | medium |
| RB-07 | [runtime-capabilities-accelerators.md](runtime-capabilities-accelerators.md) | `src-tauri/commands/models.rs` | medium |
| RB-08 | [power-assertion-macos-objc2.md](power-assertion-macos-objc2.md) | `src-tauri/commands/power.rs` | medium |
| RB-09 | [decoder-partial-audio-on-error.md](decoder-partial-audio-on-error.md) | `crates/audio/decode.rs` | medium |
| RB-10 | [llm-prompt-preflight.md](llm-prompt-preflight.md) | `crates/llm/lib.rs` | medium |
| RB-11 | [keystore-thread-safety.md](keystore-thread-safety.md) | `crates/cloud-providers/keystore.rs` | medium |
| RB-12 | [hotkey-linux-device-filter.md](hotkey-linux-device-filter.md) | `crates/hotkey/linux.rs` | small |
## Dependencies
```
RB-01 (live session race)
└── blocked by RB-04 (run_live_session monolith refactor)
RB-03 (transcript-profile FK)
└── coupled with RB-02 (migrations atomicity)
— a v9 migration adding the FK constraint must be transactional
```
## How to convert to GitHub issues
Once `gh` CLI is installed and authed (`sudo dnf install -y gh && gh auth login`):
```fish
for file in docs/issues/rb-*.md c1-*.md c3-*.md c4-*.md run-*.md poll-*.md \
native-*.md runtime-*.md power-*.md decoder-*.md llm-*.md \
keystore-*.md hotkey-*.md
set -l title (head -1 "$file" | sed 's/^# //')
gh issue create --repo jakejars/kon --title "$title" --body-file "$file" \
--label release-blocker
end
```
Issue labels to create first (`gh label create`):
- `release-blocker` — colour `#d73a4a`
- `critical` — colour `#b60205`
- `major` — colour `#d93f0b`

View File

@@ -0,0 +1,29 @@
# RB-01 CRITICAL: racy single-session guard in live.rs
**Severity:** CRITICAL
**Path:** `src-tauri/src/commands/live.rs:193-338`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md#c1--racy-single-session-guard-in-livers)
**Labels:** release-blocker, critical, concurrency
## Problem
`start_live_transcription_session` checks `running` is `None` before multiple `await`s and only stores the handle at the end. `stop_live_transcription_session` removes `running` before awaiting the worker join. Two overlapping IPC calls can:
- Admit a second live session (start sees `running == None`, awaits, another start fires in the gap, both proceed)
- Expose an empty slot while the first session is still shutting down (stop removes the handle, awaits, a fresh start runs against the incoherent state)
This breaks the file's core invariant that only one microphone/live session exists at a time.
## Acceptance
- Hold the session-slot lock (or a semaphore) across the async boundary so no two `start`/`stop` IPC calls can interleave.
- Regression test: fire two `start_live_transcription_session` IPC calls concurrently; exactly one must succeed and the other must error cleanly.
- Regression test: during an in-flight `stop`, a concurrent `start` must block until the previous session's worker has fully joined.
## Fix scope
Large. Will likely require the `run_live_session` monolith refactor (RB-04) to land first so the state machine is small enough to reason about under the lock discipline.
## Dependencies
- **Blocked by:** RB-04 (`run_live_session` monolith refactor)

View File

@@ -0,0 +1,24 @@
# RB-02 CRITICAL: multi-statement migrations can half-apply
**Severity:** CRITICAL
**Path:** `crates/storage/src/migrations.rs:263-299`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md#c3--multi-statement-migrations-can-half-apply)
**Labels:** release-blocker, critical, data-integrity, storage
## Problem
`run_migrations` executes each statement individually and only records the schema version after the full migration succeeds. If a multi-statement migration (v5, v6, v8 — any containing more than one `CREATE` / `ALTER` / `UPDATE`) fails mid-run, or the process is killed between statements, the schema can end up partially changed while still appearing unapplied. The next startup replays the same migration against the mutated database, which can fail in confusing ways or corrupt data further.
## Acceptance
- Every migration runs inside a single `BEGIN` / `COMMIT` transaction.
- The version row update happens inside the same transaction — atomic success or no change.
- Regression test: a migration that panics partway through leaves the database at the previous schema version with no partial changes visible on restart.
## Fix scope
Medium. Wrap each migration in `pool.begin()` / `tx.commit()`. The version update and the migration statements all execute on the same `Transaction` handle. Needs careful review of any migration that uses implicit commits (SQLite `VACUUM`, `REINDEX`, `ATTACH` — none of which Kon currently uses, but the review pattern should guard against future additions).
## Dependencies
- Coupled with RB-03 (any v9 migration adding the transcript-profile FK must itself be transactional — this fix is a prerequisite).

View File

@@ -0,0 +1,26 @@
# RB-03 CRITICAL: transcript provenance can reference deleted profiles
**Severity:** CRITICAL
**Path:** `crates/storage/src/migrations.rs:208-216`, `crates/storage/src/database.rs:61-89`, `:697-708`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md#c4--transcript-provenance-can-reference-deleted-profiles)
**Labels:** release-blocker, critical, data-integrity, storage
## Problem
v8 migration adds `transcripts.profile_id` but without a `FOREIGN KEY` constraint. `insert_transcript` accepts any `profile_id` string without validation. `delete_profile` doesn't guard against existing transcript references. The combined result: persisted transcripts can keep orphaned profile IDs indefinitely, breaking provenance integrity.
## Acceptance
- A v9 migration adds `FOREIGN KEY (profile_id) REFERENCES profiles(id) ON DELETE RESTRICT` (or `ON DELETE SET NULL` if soft-orphaning is preferred — decide during the fix).
- The migration reconciles existing orphans: either backfill with `DEFAULT_PROFILE_ID`, or null them, per the chosen FK semantic.
- `insert_transcript` passes the FK check — no behaviour change on the happy path.
- `delete_profile` returns a meaningful error when transcripts reference the profile being deleted (or cascades to null, matching the FK semantic).
- Regression tests: (a) delete_profile with transcript references behaves per the chosen semantic; (b) insert_transcript with a non-existent profile_id errors; (c) existing orphans are reconciled on first migration to v9.
## Fix scope
Large. FK constraint design decision + migration + reconciliation + `database.rs` updates + tests.
## Dependencies
- **Blocked by:** RB-02 (migrations atomicity — the v9 migration must be transactional).

View File

@@ -0,0 +1,29 @@
# RB-09 MAJOR: decoder returns partial audio on read/decode errors
**Severity:** MAJOR
**Path:** `crates/audio/src/decode.rs:58-79`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, audio, data-integrity
## Problem
`decode_audio_file`:
- Breaks the read loop on packet-read errors (truncated / corrupt inputs)
- Counts and skips per-packet decoder errors
- Still returns `Ok` if any samples were produced before the break
A corrupt or truncated input file is silently accepted as partial audio. Callers have no way to distinguish "file decoded cleanly" from "file was bad and we handed you half of it".
## Acceptance
- Propagate read and decode errors to the caller (return `Err`) — match the pattern used in `read_wav` (fixed in the 2026-04-22 quick-wins batch, commit `b665754`).
- Optional: expose a `decode_audio_file_best_effort` variant if anyone genuinely wants the partial-audio-on-error behaviour. Today no caller needs it.
- Regression tests: (a) truncated MP3; (b) corrupted FLAC; (c) valid file continues to decode successfully.
## Fix scope
Medium. Error-propagation pattern is the same as the `read_wav` fix, but the symphonia packet-loop has several skip branches to audit.
## Dependencies
- None — standalone fix.

View File

@@ -0,0 +1,26 @@
# RB-12 MAJOR: hotkey device filtering hard-codes KEY_A / KEY_R
**Severity:** MAJOR
**Path:** `crates/hotkey/src/linux.rs:236-241`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, hotkey, correctness
## Problem
`try_attach_device` claims to check whether an input device supports the configured hotkey's key, but the implementation tests for hard-coded `KEY_A` or `KEY_R` instead of consulting the actual `HotkeyCombo` that was configured. Hotkeys bound to any other key (which is most of them) can be silently skipped even when the device supports them.
This is a correctness bug in a user-facing feature. A user who binds Kon to `Ctrl+Shift+D` and sees "no hotkey fires" has no obvious path to diagnose it.
## Acceptance
- Device attachment consults the actual configured `HotkeyCombo.trigger` key code.
- Regression test: `try_attach_device` called with a mock device that supports `KEY_D` attaches when the configured hotkey's trigger is `D`, does not attach when the trigger is a key the device doesn't support.
- Manual verification: bind `Ctrl+Shift+D` in Settings, confirm it fires in a running Kon.
## Fix scope
Small. Replace the hard-coded constants with a lookup from the passed-in `HotkeyCombo`.
## Dependencies
- None — standalone fix.

View File

@@ -0,0 +1,29 @@
# RB-11 MAJOR: keystore::store_api_key is a thread-unsafe safe API
**Severity:** MAJOR
**Path:** `crates/cloud-providers/src/keystore.rs:6-18`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, unsafe-api, cloud
## Problem
`store_api_key` is declared as a safe `pub fn`. Its implementation relies on `std::env::set_var`, which is documented as Undefined Behaviour outside single-threaded initialisation. The file's module comment acknowledges the precondition but the function signature does not enforce it — any caller can invoke it from any thread, and the compiler won't object.
## Acceptance
Choose one:
1. **Use an OS keychain backend** (e.g. `keyring` crate) so there is no `set_var` involvement. Preferred — actually secret-safe, cross-platform.
2. **Use a process-global `OnceLock` or `Mutex<HashMap>`** inside the module instead of `set_var`. Removes the UB, trades persistence.
3. **Mark `store_api_key` as `unsafe`** and document the "call once before threads spawn" contract at the signature level. Ugly but honest.
Whichever path, update the signature and doc comments to match the safety properties actually provided.
## Fix scope
Medium. Option 1 is the right long-term answer but adds a dep and platform-specific auth prompts (macOS Keychain asks the user on first access). Option 2 is fastest. Option 3 is cosmetic.
## Dependencies
- None — standalone fix.
- Coupled with future BYO LLM endpoint work (storing API keys safely is a prerequisite).

View File

@@ -0,0 +1,24 @@
# RB-10 MAJOR: LLM prompts not preflighted against context window
**Severity:** MAJOR
**Path:** `crates/llm/src/lib.rs:143-166`, `:317-321`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, llm
## Problem
`generate` tokenises and batches the full prompt at runtime. `context_window_size` hard-caps context at 8192 tokens. Long transcripts (a 30-minute dictation session is easily 40006000 tokens after segment joining) reach inference with prompts already bigger than the available context — causing late runtime failure instead of a controlled early-exit path.
## Acceptance
- Before inference begins, the prompt token count is compared against the available context window (minus the expected response budget).
- Oversized prompts either (a) surface a typed error the caller can handle gracefully, or (b) are truncated with a logged warning — decide during the fix.
- Regression test: synthesise a transcript whose tokenised form exceeds 8192 tokens, assert the chosen behaviour (early error or truncated input).
## Fix scope
Medium. Tokeniser access is already on the LLM path; the check is cheap. Decision work is in what to do when a prompt is too long (fail hard vs truncate).
## Dependencies
- None — standalone fix.

View File

@@ -0,0 +1,24 @@
# RB-06 MAJOR: native capture worker is detached, can outlive stop/start
**Severity:** MAJOR
**Path:** `src-tauri/src/commands/audio.rs:46-228`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, concurrency, audio
## Problem
`start_native_capture` and `stop_native_capture` coordinate through a channel but never retain the spawned worker handle. A previous capture can still be flushing / appending after `stop_native_capture` clears `all_samples` and before a new `start_native_capture` takes it — output can be truncated or contaminated with cross-session samples.
## Acceptance
- Store the worker's `JoinHandle` in the native capture state.
- `stop_native_capture` awaits the handle before returning — start/stop/start is fully serialised.
- Regression test: rapid start → stop → start sequence produces two distinct samples vectors with no cross-session leakage.
## Fix scope
Medium. Requires adding `JoinHandle` storage and making the stop path `await` cleanly — probably needs a small refactor of the native capture state struct.
## Dependencies
- Independent of other items, though the fix pattern (retain handles, join on stop) mirrors what RB-04 will do for the live-session worker.

View File

@@ -0,0 +1,24 @@
# RB-05 MAJOR: poll_inference treats IPC listener loss as session-fatal
**Severity:** MAJOR
**Path:** `src-tauri/src/commands/live.rs:721-813`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, ipc-lifecycle
## Problem
`result_channel.send(...)` propagates with `?`, so closing the listening frontend or reloading the webview terminates the whole live session — even when capture and inference are healthy. Tauri channel-lifecycle events are not transcription failures and should not kill the worker.
## Acceptance
- Channel-send errors log a warning and continue the session (if recoverable) or terminate gracefully (if the session was going to end anyway).
- The distinction between "transcription failed" and "no listener to report to" is explicit in the error handling.
- Regression test: simulate channel close mid-session, assert the worker keeps capturing and produces a valid WAV file.
## Fix scope
Medium. Isolated to `poll_inference` and its error handling; interacts with RB-04 (monolith refactor) since that restructures the same function family.
## Dependencies
- **Related:** RB-04.

View File

@@ -0,0 +1,27 @@
# RB-08 MAJOR: PowerAssertion is a non-functional stub on macOS
**Severity:** MAJOR (macOS only)
**Path:** `src-tauri/src/commands/power.rs:41-121`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md), originally deferred during A.1 #9
**Labels:** release-blocker, major, macos, platform
## Problem
`begin_activity` always returns `Err` on macOS, so `PowerAssertion::begin` converts to `None` and the guard never acquires an `NSProcessInfo beginActivityWithOptions:reason:` assertion. Live recording and LLM cleanup therefore run without App Nap protection on the one platform where it matters.
The stub was deliberate (A.1 #9 acceptance concession — untestable on Linux without a macOS build host). Re-flagged here so it is not forgotten before the first macOS ship.
## Acceptance
- `objc2` + `objc2-foundation` deps added to the kon crate, gated `cfg(target_os = "macos")`.
- `begin_activity` calls `[NSProcessInfo processInfo] beginActivityWithOptions:(NSActivityUserInitiated | NSActivityLatencyCritical) reason:reason]` and retains the returned activity handle.
- `end_activity` calls `endActivity:` on the retained handle.
- Manual-test on a real macOS box: 10-minute background live session completes without throttling; `pmset -g assertions` shows Kon's activity during capture.
## Fix scope
Medium. Dep addition + FFI glue + manual verification. Can be done from Linux with `cargo check --target=aarch64-apple-darwin` for compile validation, but runtime behaviour needs a macOS machine.
## Dependencies
- **Hard blocker:** before first macOS build/ship.

View File

@@ -0,0 +1,27 @@
# RB-04 MAJOR: run_live_session is a 200+ line multi-responsibility monolith
**Severity:** MAJOR
**Path:** `src-tauri/src/commands/live.rs:349-579`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, refactor, concurrency
## Problem
`run_live_session` owns mic startup, runtime error draining, resampling, progressive WAV persistence, overload dropping, inference scheduling, and shutdown/finalisation in one 200-line function. The state machine is spread across mutable locals — hard to audit, hard to reason about under concurrency, and already contributing to lifecycle bugs nearby (RB-01, RB-05, RB-06).
## Acceptance
- Split into focused types / functions: capture setup, streaming state, inference scheduler, WAV writer lifecycle, shutdown handler.
- Each function ≤ 30 lines, single responsibility.
- State machine explicit — not implicit in the interleaved mutable locals.
- Lock discipline documented: what must be held when, across what `await` boundaries.
- Existing behaviour preserved — 193 workspace lib tests still green; manual dogfood smoke test of a 30-second live dictation.
## Fix scope
Large. Probably one dedicated session.
## Dependencies
- **Unblocks:** RB-01 (live session race fix becomes tractable once the state machine is small).
- **Related:** RB-05, RB-06 (both lifecycle bugs in the same function).

View File

@@ -0,0 +1,30 @@
# RB-07 MAJOR: get_runtime_capabilities advertises wrong accelerators
**Severity:** MAJOR
**Path:** `src-tauri/src/commands/models.rs:435-489`
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
**Labels:** release-blocker, major, ui-integrity
## Problem
The IPC response hard-codes `accelerators = ["cpu", "vulkan"]` and `supports_gpu = true` for Whisper, even when:
- `detect_active_compute_device` would report `metal` on macOS (via MoltenVK).
- The binary was compiled without the `whisper` feature — in which case Whisper `supports_gpu` is meaningless because there is no Whisper backend at all.
The frontend uses this response to render Settings toggles (GPU selection, active-device badge, feature availability). Wrong values mean wrong UI states on exactly the builds this function is meant to describe.
## Acceptance
- `accelerators` is derived from actual build configuration and runtime probe, not hard-coded.
- On macOS, `accelerators` includes `"metal"` when the Metal loader resolves.
- On whisper-disabled builds, Whisper entries advertise `supports_gpu = false` (or the engine is omitted from the response entirely).
- Regression tests cover each platform variant via cfg-gated test cases.
## Fix scope
Medium. The detection helpers already exist (`detect_active_compute_device`, `vulkan_loader_available`); this is about wiring their output into the RuntimeCapabilities struct honestly.
## Dependencies
- None — standalone fix.