feat: finalize HTTP direct dispatch refactor and pending milestone updates
Switch API execution to direct daemon request handling, add regression coverage for non-socket HTTP dispatch, and include the remaining pending local updates across CLI/daemon/docs from the current worktree.
This commit is contained in:
+7
-2
@@ -2,7 +2,7 @@
|
|||||||
# tvctl — Living Project Map
|
# tvctl — Living Project Map
|
||||||
# This file is the single source of truth for any agent working on this project.
|
# This file is the single source of truth for any agent working on this project.
|
||||||
# Read this first. Update this when you make structural changes.
|
# Read this first. Update this when you make structural changes.
|
||||||
# Last updated: 2026-04-14
|
# Last updated: 2026-04-18
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -19,7 +19,7 @@ script and control smart TVs through a stable, brand-agnostic API.
|
|||||||
|
|
||||||
## Project Status
|
## Project Status
|
||||||
|
|
||||||
**Phase:** Milestone 5 in progress. Daemon core and CLI are complete for v1 Roku control, and the `/v1` HTTP API server is now wired to the same daemon request surface. Remaining work is automated API validation and any follow-up transport cleanup.
|
**Phase:** Milestone 6 in progress. Daemon core, CLI, and `/v1` HTTP API are complete for v1 Roku control. Remaining work is release prep (service verification, cross-compile, CI, first release).
|
||||||
**Platform v1:** Roku only (via ECP HTTP API)
|
**Platform v1:** Roku only (via ECP HTTP API)
|
||||||
**Language:** Rust
|
**Language:** Rust
|
||||||
**Crate type:** Binary (single binary distribution target)
|
**Crate type:** Binary (single binary distribution target)
|
||||||
@@ -45,6 +45,7 @@ tvctl/
|
|||||||
├── src/
|
├── src/
|
||||||
│ ├── lib.rs ← Library surface for shared modules and integration tests
|
│ ├── lib.rs ← Library surface for shared modules and integration tests
|
||||||
│ ├── main.rs ← Binary entry point and runtime bootstrap
|
│ ├── main.rs ← Binary entry point and runtime bootstrap
|
||||||
|
│ ├── logging.rs ← Tracing initialization and runtime log-filter reload
|
||||||
│ ├── cli/ ← CLI layer (clap-based scaffold)
|
│ ├── cli/ ← CLI layer (clap-based scaffold)
|
||||||
│ │ └── mod.rs
|
│ │ └── mod.rs
|
||||||
│ ├── daemon/ ← tvctld daemon core scaffolding
|
│ ├── daemon/ ← tvctld daemon core scaffolding
|
||||||
@@ -108,6 +109,10 @@ CLI (tvctl)
|
|||||||
└── /v1/... (tool builders)
|
└── /v1/... (tool builders)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
The CLI reaches the daemon over the Unix socket. The HTTP API runs in the same
|
||||||
|
process and dispatches directly into the daemon request executor instead of
|
||||||
|
looping back through the socket layer.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Adapter Trait
|
## Adapter Trait
|
||||||
|
|||||||
@@ -87,7 +87,8 @@ Fast, secure, no port conflicts. The socket is only accessible to the running us
|
|||||||
**HTTP API**
|
**HTTP API**
|
||||||
The daemon optionally exposes a local HTTP API on `127.0.0.1:7272`. This is the interface for tool
|
The daemon optionally exposes a local HTTP API on `127.0.0.1:7272`. This is the interface for tool
|
||||||
builders — Home Assistant integrations, stream deck plugins, scripts, dashboards. Loopback-only by
|
builders — Home Assistant integrations, stream deck plugins, scripts, dashboards. Loopback-only by
|
||||||
default. All responses are JSON.
|
default. HTTP handlers dispatch directly inside the daemon process rather than looping back through
|
||||||
|
the Unix socket. All responses are JSON.
|
||||||
|
|
||||||
**Adapter Layer**
|
**Adapter Layer**
|
||||||
Each TV platform is implemented as an adapter behind a common Rust trait. The adapter handles all
|
Each TV platform is implemented as an adapter behind a common Rust trait. The adapter handles all
|
||||||
@@ -404,7 +405,7 @@ roku_password = ""
|
|||||||
| `daemon.http_enabled` | `true` | Expose HTTP API |
|
| `daemon.http_enabled` | `true` | Expose HTTP API |
|
||||||
| `daemon.http_port` | `7272` | HTTP API port |
|
| `daemon.http_port` | `7272` | HTTP API port |
|
||||||
| `daemon.http_host` | `127.0.0.1` | Bind address (loopback by default) |
|
| `daemon.http_host` | `127.0.0.1` | Bind address (loopback by default) |
|
||||||
| `daemon.log_level` | `info` | debug / info / warn / error |
|
| `daemon.log_level` | `info` | Tracing filter (for example `info` or `tvctl=debug`), applied on daemon start and `tvctl config reload` |
|
||||||
| `discovery.auto_discover` | `true` | Discover devices on daemon start |
|
| `discovery.auto_discover` | `true` | Discover devices on daemon start |
|
||||||
| `discovery.interval_secs` | `300` | Re-scan interval (0 = disabled) |
|
| `discovery.interval_secs` | `300` | Re-scan interval (0 = disabled) |
|
||||||
| `discovery.timeout_secs` | `5` | Per-device timeout |
|
| `discovery.timeout_secs` | `5` | Per-device timeout |
|
||||||
|
|||||||
+3
-2
@@ -1,7 +1,7 @@
|
|||||||
# ROADMAP.md
|
# ROADMAP.md
|
||||||
# tvctl — Feature Roadmap and Milestone Tracker
|
# tvctl — Feature Roadmap and Milestone Tracker
|
||||||
# Agents: update this file as work is completed. See AGENT.md for instructions.
|
# Agents: update this file as work is completed. See AGENT.md for instructions.
|
||||||
# Last updated: 2026-04-15
|
# Last updated: 2026-04-18
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -120,7 +120,7 @@ _Goal: Ready for real use._
|
|||||||
- [ ] `tvctl daemon install` generates correct systemd unit file
|
- [ ] `tvctl daemon install` generates correct systemd unit file
|
||||||
- [x] 2026-04-15 — First-run experience: helpful output when no devices discovered yet
|
- [x] 2026-04-15 — First-run experience: helpful output when no devices discovered yet
|
||||||
- [x] 2026-04-15 — Daemon startup message with socket path and HTTP port
|
- [x] 2026-04-15 — Daemon startup message with socket path and HTTP port
|
||||||
- [ ] Log output via `tracing` (respects `log_level` config)
|
- [x] 2026-04-18 — Log output via `tracing` (respects `log_level` config)
|
||||||
- [x] 2026-04-15 — README accuracy pass (verify all examples work)
|
- [x] 2026-04-15 — README accuracy pass (verify all examples work)
|
||||||
- [x] 2026-04-15 — `cargo clippy` clean
|
- [x] 2026-04-15 — `cargo clippy` clean
|
||||||
- [x] 2026-04-15 — `cargo test` passing
|
- [x] 2026-04-15 — `cargo test` passing
|
||||||
@@ -166,6 +166,7 @@ out of scope until Milestone 6 is complete and stable.
|
|||||||
- [x] 2026-04-14 — Finish Milestone 4 with help, JSON-mode, config socket-reload, and secret-redaction polish
|
- [x] 2026-04-14 — Finish Milestone 4 with help, JSON-mode, config socket-reload, and secret-redaction polish
|
||||||
- [x] 2026-04-15 — Add HTTP API route parity and integration coverage against an isolated running daemon
|
- [x] 2026-04-15 — Add HTTP API route parity and integration coverage against an isolated running daemon
|
||||||
- [x] 2026-04-15 — Add shell completions and first-run CLI polish for Milestone 6
|
- [x] 2026-04-15 — Add shell completions and first-run CLI polish for Milestone 6
|
||||||
|
- [x] 2026-04-18 — Make runtime tracing respect `daemon.log_level` on daemon start and config reload
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
# Claude Observations Plan
|
||||||
|
|
||||||
|
Reference source: `claude-observations.txt`
|
||||||
|
|
||||||
|
This file is a fact-checked implementation plan based on the numbered observations
|
||||||
|
in `claude-observations.txt`. It reflects the current worktree state on 2026-04-18,
|
||||||
|
not just the last committed `main`.
|
||||||
|
|
||||||
|
## Fact Check Summary
|
||||||
|
|
||||||
|
| Observation | Status | Fact Check | Planned Action |
|
||||||
|
|---|---|---|---|
|
||||||
|
| 1. `format_tv_key` duplicates normalized key mapping | Confirmed | `src/daemon/mod.rs` still has a 33-arm `format_tv_key` helper and one call site in the `SendKey` action detail path. `src/adapters/mod.rs` still has a separate `parse_normalized_tv_key` table and `TvKey` keeps `#[serde(rename_all = "kebab-case")]`. | Remove the duplicate reverse table. Preferred fix: add a canonical string conversion on `TvKey` and use it from the daemon response text. |
|
||||||
|
| 2. `DeviceRegistry::remove` duplicates `find` matching logic | Confirmed | `src/daemon/registry.rs` has `find()` with inline UUID/name matching and `remove()` with separate `matches_target(...)` logic. | Refactor `remove()` to reuse `find()` or otherwise share one target-matching implementation. Delete the duplicate matcher after the refactor. |
|
||||||
|
| 3. `parse_bool` is a no-value wrapper | Confirmed | `src/daemon/config.rs` still has `fn parse_bool(...) { parse_value(...) }` and uses it in `set_value(...)`. | Delete `parse_bool` and switch the bool assignments to `parse_value(...)`. |
|
||||||
|
| 4. HTTP API loops back through the Unix socket even though it is in-process | Completed locally on 2026-04-18 | Refactored `src/api/mod.rs` to call `daemon::execute_request(...)` directly and added an in-process HTTP regression test that succeeds without starting the daemon Unix socket server. | No further change needed unless follow-up regressions are found. |
|
||||||
|
| 5. `PROJECT_MAP.md` is stale about Milestone 5 / HTTP status | Stale in current worktree | In the current worktree, `PROJECT_MAP.md` is already locally updated to `Last updated: 2026-04-18` and `Phase: Milestone 6 in progress`. This observation was valid against an older snapshot, not the current tree. | No repo change needed for this item unless those local doc edits are reverted before commit. |
|
||||||
|
| 6. Missing cleanup for `node-compile-cache/` and `phantomjs/` | Partially confirmed | Neither directory exists in the current repo tree, but `.gitignore` does not currently list them. The observation appears archive-related rather than evidence of a current tracked repo problem. | Add preventive ignore entries later if desired. No tracked-file cleanup is needed unless those directories reappear. |
|
||||||
|
| 7. `get_value()` allocates a full `BTreeMap` for one lookup | Confirmed | `src/daemon/config.rs` still implements `get_value()` as `self.entries().remove(key)`. | Rewrite `get_value()` as a direct `match` over stable keys so it no longer allocates the full map for single-key reads. |
|
||||||
|
| 8. `roku_key_paths` unsupported-key intent is unclear | Confirmed | `src/adapters/roku/mod.rs` still falls through from valid `Ok(...)` arms into a bottom branch where `Stop`, `Skip`, `Power`, `PowerOn`, and `Options` are only used to build `InvalidKey(...)`. | Add a clarifying comment above the fall-through block. No behavior change needed. |
|
||||||
|
| 9. `RefreshAppsBody` optional `clear` handling can be simplified | Cosmetic | `src/api/mod.rs` still uses `body.map(|value| value.clear).unwrap_or(false)`. | Fold into a smaller expression while touching the HTTP module for higher-priority work. Do not treat this as a standalone task. |
|
||||||
|
| 10. Rust 2024 edition note | Informational only | `Cargo.toml` already uses `edition = "2024"`. No code issue is implied. | No repo change needed now. Keep the note in mind when CI is added so the toolchain is new enough. |
|
||||||
|
|
||||||
|
## Execution Order
|
||||||
|
|
||||||
|
The future implementation order should stay low-risk first and defer the heavier
|
||||||
|
HTTP refactor until the small cleanups are merged or at least locally stable.
|
||||||
|
|
||||||
|
1. Remove duplication and low-risk cleanup
|
||||||
|
- Observation 3: delete `parse_bool`
|
||||||
|
- Observation 8: add unsupported-key comment in `roku_key_paths`
|
||||||
|
- Observation 9: simplify `RefreshAppsBody` boolean handling if already editing `src/api/mod.rs`
|
||||||
|
|
||||||
|
2. Remove duplicated matching / key-string tables
|
||||||
|
- Observation 2: unify `DeviceRegistry` target matching
|
||||||
|
- Observation 1: replace `format_tv_key` with one canonical `TvKey` string path
|
||||||
|
- Observation 7: rewrite `get_value()` to avoid `entries()` allocation
|
||||||
|
|
||||||
|
3. Perform the medium-risk HTTP architecture cleanup
|
||||||
|
- Observation 4: replace Unix-socket loopback in the HTTP API with direct `execute_request(...)`
|
||||||
|
- Add focused regression coverage around HTTP handlers after this change because it changes the transport path, not just formatting
|
||||||
|
|
||||||
|
4. Preventive hygiene
|
||||||
|
- Observation 6: add `.gitignore` entries for `node-compile-cache/` and `phantomjs/` if we want to harden against future archive/worktree leakage
|
||||||
|
|
||||||
|
## Constraints For Future Work
|
||||||
|
|
||||||
|
- Keep this work separate from the current uncommitted logging-related changes already in the worktree unless the user explicitly asks to batch them together.
|
||||||
|
- Do not assume all Claude observations are still valid against the live tree. Re-check each item immediately before implementation.
|
||||||
|
- Preserve current user-facing behavior where possible, especially for daemon/API error payloads and CLI help/output.
|
||||||
@@ -0,0 +1,716 @@
|
|||||||
|
AGENT INSTRUCTION: BELOW ARE SOME OBSERVATIONS FROM CLAUDE. ITS ONLY PARTIAL BECAUSE THE FREE PLAN LIMIT WAS REACHED. BUT READ THIS AND LEARN WHAT YOU CAN ABOUT IT. THEN IMPLEMENT SOME FIXES ETC AFTER VALIDATING THE ISSUE.
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
Now I have a thorough picture of the entire codebase. Let me compile my findings.---
|
||||||
|
|
||||||
|
Here's a thorough analysis of the project with agent instructions ready for use.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Project Health Summary
|
||||||
|
|
||||||
|
**On track**: The project is genuinely following its plan. Milestones 1–5 are complete. Milestone 6 is partially done. Architecture, data shapes, CLI surface, IPC protocol, and adapter trait are all correctly implemented. The `node-compile-cache/` and `phantomjs/` directories in the archive are clearly accidental inclusions from the working directory — they should not be in the repo.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Agent Instructions
|
||||||
|
|
||||||
|
```
|
||||||
|
You are working on the `tvctl` Rust project. Read PROJECT_MAP.md first.
|
||||||
|
Do NOT rewrite files wholesale. Make targeted, surgical changes only.
|
||||||
|
Do NOT touch anything not explicitly listed below.
|
||||||
|
Run `cargo clippy` and `cargo test` after each change and fix any new warnings before moving on.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 1. CRITICAL BUG — `format_tv_key` duplicates `parse_normalized_tv_key` in reverse
|
||||||
|
|
||||||
|
File: `src/daemon/mod.rs`, function `format_tv_key`
|
||||||
|
|
||||||
|
This function is a full 33-arm hand-written match over `TvKey` that returns the kebab-case string for each key.
|
||||||
|
The `TvKey` enum in `src/adapters/mod.rs` is already decorated with `#[serde(rename_all = "kebab-case")]`.
|
||||||
|
|
||||||
|
`format_tv_key` can never drift from the parse table without silently producing wrong values. It is
|
||||||
|
duplication by definition and contains a latent inconsistency: if a new `TvKey` variant is added, it
|
||||||
|
must be added in three places (the enum, `parse_normalized_tv_key`, AND `format_tv_key`) with no
|
||||||
|
compiler enforcement that the last one is updated.
|
||||||
|
|
||||||
|
**Fix**: Delete `format_tv_key` and replace its single call site with a serde round-trip:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// In place of: format_tv_key(&key)
|
||||||
|
// Use:
|
||||||
|
serde_json::to_value(&key)
|
||||||
|
.ok()
|
||||||
|
.and_then(|v| v.as_str().map(str::to_string))
|
||||||
|
.unwrap_or_else(|| format!("{key:?}"))
|
||||||
|
```
|
||||||
|
|
||||||
|
Or, alternatively, implement `std::fmt::Display` for `TvKey` in `src/adapters/mod.rs` using serde's
|
||||||
|
rename logic, and use that in the call site. Either approach eliminates the duplicate table.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 2. BUG — `DeviceRegistry::remove` duplicates the match logic of `DeviceRegistry::find`
|
||||||
|
|
||||||
|
File: `src/daemon/registry.rs`
|
||||||
|
|
||||||
|
`find` uses an inline closure:
|
||||||
|
```rust
|
||||||
|
self.devices.iter().find(|device| {
|
||||||
|
target_uuid.map(|uuid| device.id == uuid).unwrap_or(false)
|
||||||
|
|| device.name.to_ascii_lowercase() == normalized
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
`remove` uses a separate private `matches_target` free function with identical logic.
|
||||||
|
|
||||||
|
These two code paths will diverge if the matching rules ever change (e.g., adding prefix-match support).
|
||||||
|
|
||||||
|
**Fix**: Refactor `remove` to use `find` to locate the index first:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn remove(&mut self, target: &str) -> Option<Device> {
|
||||||
|
let id = self.find(target)?.id;
|
||||||
|
let index = self.devices.iter().position(|d| d.id == id)?;
|
||||||
|
let removed = self.devices.remove(index);
|
||||||
|
self.ensure_default();
|
||||||
|
Some(removed)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Then delete the `matches_target` free function entirely.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 3. BUG — `parse_bool` is a useless wrapper in `src/daemon/config.rs`
|
||||||
|
|
||||||
|
File: `src/daemon/config.rs`, lines ~300–303
|
||||||
|
|
||||||
|
```rust
|
||||||
|
fn parse_bool(key: &str, value: &str) -> anyhow::Result<bool> {
|
||||||
|
parse_value(key, value)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This is a zero-value indirection. `parse_value::<bool>` already exists and already handles `bool`.
|
||||||
|
`parse_bool` exists solely because someone called it before `parse_value` was made generic.
|
||||||
|
|
||||||
|
**Fix**: Delete `parse_bool`. Replace the three call sites with `parse_value::<bool>(key, value)?`
|
||||||
|
(or just `parse_value(key, value)?` — Rust will infer the type from the assignment target).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 4. ARCHITECTURE CONCERN — HTTP API routes through the Unix socket unnecessarily
|
||||||
|
|
||||||
|
File: `src/api/mod.rs`, function `send_daemon_request`
|
||||||
|
|
||||||
|
The HTTP API runs *inside the same process as the daemon*. It already holds `SharedDaemon: Arc<Mutex<Daemon>>`.
|
||||||
|
Despite this, every HTTP handler opens a new Unix socket connection back to the daemon, serializes
|
||||||
|
the request to JSON, sends it over the socket, and deserializes the response back. This is:
|
||||||
|
- An unnecessary round-trip through the OS socket layer
|
||||||
|
- A source of failure (`daemon_socket_unreachable`) for requests that should never fail
|
||||||
|
- A potential deadlock: the HTTP handler acquires the Mutex briefly to get the socket path, releases it,
|
||||||
|
then the request comes back in on a socket connection and tries to acquire the Mutex in `execute_request`.
|
||||||
|
This works today because `handle_connection` holds no lock, but it is a footgun as the code evolves.
|
||||||
|
|
||||||
|
The daemon already exposes `pub(crate) async fn execute_request(request: DaemonRequest, daemon: SharedDaemon)`.
|
||||||
|
|
||||||
|
**Fix**: Replace `send_daemon_request` in the HTTP API with a direct call to `daemon::execute_request`.
|
||||||
|
The refactor is mechanical:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// In execute_json and execute_json_value, instead of:
|
||||||
|
match send_daemon_request(daemon, &request).await { ... }
|
||||||
|
|
||||||
|
// Use:
|
||||||
|
let (response, _stop) = daemon::execute_request(request, daemon).await;
|
||||||
|
from_daemon_response::<T>(response)
|
||||||
|
```
|
||||||
|
|
||||||
|
Then delete `send_daemon_request` entirely. This eliminates ~60 lines of socket boilerplate from the
|
||||||
|
HTTP path, removes a class of failure modes, and eliminates the latent deadlock risk.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 5. DOCUMENTATION STALENESS — PROJECT_MAP.md out of date
|
||||||
|
|
||||||
|
File: `PROJECT_MAP.md`
|
||||||
|
|
||||||
|
The "What Has NOT Been Started" section still says:
|
||||||
|
```
|
||||||
|
- HTTP route handlers and request validation
|
||||||
|
```
|
||||||
|
|
||||||
|
This was completed in Milestone 5. Update that section to reflect the current state (Milestone 6 in
|
||||||
|
progress, remaining items are tracing log output, systemd unit generation, CI, cross-compile test,
|
||||||
|
and first binary release).
|
||||||
|
|
||||||
|
Also update the `## Project Status` block:
|
||||||
|
- Change "Phase: Milestone 5 in progress" to "Phase: Milestone 6 in progress"
|
||||||
|
- Update the description to reflect that the HTTP API is complete
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 6. CLEANUP — Accidental files in the archive / repo
|
||||||
|
|
||||||
|
The project archive contains two directories that must not be in the repo:
|
||||||
|
- `node-compile-cache/` — Node.js V8 bytecode cache, clearly from the developer's working environment
|
||||||
|
- `phantomjs/` — A 23 MB PhantomJS binary distribution
|
||||||
|
|
||||||
|
Neither is referenced by the Rust project in any way. Confirm these are excluded in `.gitignore` and
|
||||||
|
remove them from any committed state if they were accidentally committed. The `.gitignore` should
|
||||||
|
include at minimum:
|
||||||
|
```
|
||||||
|
node-compile-cache/
|
||||||
|
phantomjs/
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 7. MINOR — `get_value` allocates a full BTreeMap to look up one key
|
||||||
|
|
||||||
|
File: `src/daemon/config.rs`, `get_value` method
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn get_value(&self, key: &str) -> Option<String> {
|
||||||
|
self.entries().remove(key) // allocates entire map just to look up one key
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This allocates and populates a full `BTreeMap` (15 entries) just to retrieve one value.
|
||||||
|
|
||||||
|
**Fix**: Inline the lookup directly in `get_value` using a `match` on the key, identical to the
|
||||||
|
pattern already used in `set_value`. This is a micro-optimization but it also eliminates the
|
||||||
|
inconsistency where `entries()` and `set_value` are two separate parallel tables that must stay in
|
||||||
|
sync. Alternatively, make `set_value` call `entries()` for validation but that's heavier. The
|
||||||
|
simplest correct fix is to re-express `get_value` as a `match` block mirroring `set_value`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 8. MINOR — `roku_key_paths` uses mixed return styles
|
||||||
|
|
||||||
|
File: `src/adapters/roku/mod.rs`, function `roku_key_paths`
|
||||||
|
|
||||||
|
The function has two code paths:
|
||||||
|
1. The first ~25 arms `return Ok(vec!["SomeKey".to_string()])` inline
|
||||||
|
2. The last 5 arms fall through to a `let path = match { "some-string" }` followed by
|
||||||
|
`Err(TvError::InvalidKey(path.to_string()))`
|
||||||
|
|
||||||
|
This is intentional (the last 5 keys have no known Roku mapping), but the pattern is confusing:
|
||||||
|
those 5 arms look like they map to valid paths but actually the entire bottom branch returns `Err`.
|
||||||
|
|
||||||
|
**Fix**: Consolidate to make the intent obvious. The simplest approach: keep the two-style structure
|
||||||
|
but add a comment before the fall-through block making it explicit that these keys are
|
||||||
|
**intentionally unsupported** and the string values are used only for the error message:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// The following keys have no documented Roku ECP equivalent.
|
||||||
|
// The string values are used only to populate the InvalidKey error.
|
||||||
|
TvKey::Stop => "stop",
|
||||||
|
TvKey::Skip => "skip",
|
||||||
|
TvKey::Power => "power",
|
||||||
|
TvKey::PowerOn => "power-on",
|
||||||
|
TvKey::Options => "options",
|
||||||
|
```
|
||||||
|
|
||||||
|
No logic change required — just a documentation fix to prevent future maintainers from trying to
|
||||||
|
"fix" this by making it return Ok.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 9. MINOR — `RefreshAppsBody` optional body field can use `unwrap_or_default`
|
||||||
|
|
||||||
|
File: `src/api/mod.rs`
|
||||||
|
|
||||||
|
```rust
|
||||||
|
clear: body.map(|value| value.clear).unwrap_or(false),
|
||||||
|
```
|
||||||
|
|
||||||
|
`bool` implements `Default` (defaults to `false`), so this can be simplified to:
|
||||||
|
```rust
|
||||||
|
clear: body.map_or(false, |value| value.clear),
|
||||||
|
```
|
||||||
|
|
||||||
|
Or better: make `clear` default-able in the body by giving it `#[serde(default)]` (it already has
|
||||||
|
that) and unwrap the optional body using `unwrap_or_default()` on the struct. Either is fine.
|
||||||
|
This is cosmetic.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 10. NOTE — `edition = "2024"` in Cargo.toml
|
||||||
|
|
||||||
|
File: `Cargo.toml`
|
||||||
|
|
||||||
|
The crate uses `edition = "2024"`. Rust 2024 edition stabilized in Rust 1.85 (Feb 2025).
|
||||||
|
This is fine and intentional, but it explains the `if clear && let Err(...)` syntax in `daemon/mod.rs`
|
||||||
|
which is a 2024-edition `let chains` feature. No change needed — just confirm the CI target
|
||||||
|
(when it lands) pins to `rustup toolchain` >= 1.85.
|
||||||
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
Let me check the remaining files I haven't read yet — the CLI's full body and the cache/state files.Good — now I have seen everything. Let me produce the complete, final agent instructions.Now I have a complete picture. Here are the full, final agent instructions:
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Complete Agent Instructions for tvctl
|
||||||
|
|
||||||
|
```
|
||||||
|
You are working on the `tvctl` Rust project. Read PROJECT_MAP.md and ROADMAP.md first.
|
||||||
|
Do NOT rewrite files wholesale. Make targeted, surgical str_replace changes only.
|
||||||
|
Do NOT touch any code not explicitly listed in the tasks below.
|
||||||
|
After all changes, run `cargo clippy` and `cargo test` and fix any new warnings before finishing.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## VERIFIED PROJECT STATUS
|
||||||
|
|
||||||
|
Milestones 1–5 are complete. Milestone 6 is partially done. The following Milestone 6 items
|
||||||
|
remain open per ROADMAP.md:
|
||||||
|
- `tvctl daemon install` generating a correct systemd unit ← DONE (render_systemd_unit exists)
|
||||||
|
- Log output via `tracing` (respects `log_level` config) ← STILL OPEN
|
||||||
|
- Cross-compile test ← STILL OPEN
|
||||||
|
- GitHub Actions CI ← STILL OPEN
|
||||||
|
- First binary release ← STILL OPEN
|
||||||
|
|
||||||
|
**The PROJECT_MAP.md is stale.** Update it as part of these tasks (see Task 9).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 1 — CRITICAL BUG: Eliminate `format_tv_key` — it is a duplicate of `parse_normalized_tv_key` in reverse
|
||||||
|
|
||||||
|
**File:** `src/daemon/mod.rs`
|
||||||
|
|
||||||
|
`format_tv_key` is a 33-arm match over `TvKey` that reconstructs the kebab-case name for each key.
|
||||||
|
`TvKey` is already decorated with `#[serde(rename_all = "kebab-case")]`, so serde already owns
|
||||||
|
this mapping. `format_tv_key` will silently drift if any variant is added or renamed. There is no
|
||||||
|
compiler enforcement.
|
||||||
|
|
||||||
|
**Step 1:** In `src/adapters/mod.rs`, implement `std::fmt::Display` for `TvKey` using serde's
|
||||||
|
rename output as the source of truth:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
impl std::fmt::Display for TvKey {
|
||||||
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
// Delegate to serde's kebab-case serialization so the display name
|
||||||
|
// is always consistent with what the CLI and API accept as input.
|
||||||
|
let name = serde_json::to_value(self)
|
||||||
|
.ok()
|
||||||
|
.and_then(|v| v.as_str().map(str::to_string))
|
||||||
|
.unwrap_or_else(|| format!("{self:?}"));
|
||||||
|
f.write_str(&name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Place this impl block directly after the `TvKey` enum definition.
|
||||||
|
|
||||||
|
**Step 2:** In `src/daemon/mod.rs`, find the single call site of `format_tv_key`:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
let detail = format!("Sent key '{}' to {}.", format_tv_key(&key), device.name);
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace it with:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
let detail = format!("Sent key '{key}' to {}.", device.name);
|
||||||
|
```
|
||||||
|
|
||||||
|
(This works because `TvKey` now implements `Display`.)
|
||||||
|
|
||||||
|
**Step 3:** Delete the entire `format_tv_key` function from `src/daemon/mod.rs`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 2 — BUG: `DeviceRegistry::remove` duplicates the match logic of `DeviceRegistry::find`
|
||||||
|
|
||||||
|
**File:** `src/daemon/registry.rs`
|
||||||
|
|
||||||
|
`find` (line ~84) and `matches_target` (line ~322) contain identical UUID + case-insensitive name
|
||||||
|
matching logic. They will silently diverge if matching rules change.
|
||||||
|
|
||||||
|
**Step 1:** Replace the body of `remove` so it delegates to `find`:
|
||||||
|
|
||||||
|
Current:
|
||||||
|
```rust
|
||||||
|
pub fn remove(&mut self, target: &str) -> Option<Device> {
|
||||||
|
let index = self
|
||||||
|
.devices
|
||||||
|
.iter()
|
||||||
|
.position(|device| matches_target(device, target))?;
|
||||||
|
let removed = self.devices.remove(index);
|
||||||
|
self.ensure_default();
|
||||||
|
Some(removed)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace with:
|
||||||
|
```rust
|
||||||
|
pub fn remove(&mut self, target: &str) -> Option<Device> {
|
||||||
|
let id = self.find(target)?.id;
|
||||||
|
let index = self.devices.iter().position(|d| d.id == id)?;
|
||||||
|
let removed = self.devices.remove(index);
|
||||||
|
self.ensure_default();
|
||||||
|
Some(removed)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Step 2:** Delete the `matches_target` free function at the bottom of the file entirely.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 3 — BUG: `parse_bool` is a zero-value wrapper
|
||||||
|
|
||||||
|
**File:** `src/daemon/config.rs`
|
||||||
|
|
||||||
|
```rust
|
||||||
|
fn parse_bool(key: &str, value: &str) -> anyhow::Result<bool> {
|
||||||
|
parse_value(key, value)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This function does nothing that `parse_value::<bool>` does not already do.
|
||||||
|
|
||||||
|
**Step 1:** Delete the `parse_bool` function.
|
||||||
|
|
||||||
|
**Step 2:** Replace its three call sites:
|
||||||
|
- `parse_bool(key, value)?` → `parse_value(key, value)?`
|
||||||
|
|
||||||
|
The Rust type inference will resolve `parse_value` to `bool` from the assignment context in each case.
|
||||||
|
Confirm that `cargo build` passes after this change.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 4 — ARCHITECTURE BUG: HTTP API routes through the Unix socket when it could call directly
|
||||||
|
|
||||||
|
**File:** `src/api/mod.rs`
|
||||||
|
|
||||||
|
The HTTP API server runs **inside the same process** as the daemon. It holds `SharedDaemon`. Despite
|
||||||
|
this, every HTTP handler calls `send_daemon_request`, which opens a new Unix socket connection back
|
||||||
|
to the same daemon process, serializes the request to JSON, sends it over the socket, and deserializes
|
||||||
|
the response back.
|
||||||
|
|
||||||
|
Problems:
|
||||||
|
1. Unnecessary OS round-trip on every HTTP request.
|
||||||
|
2. A class of failures (`daemon_socket_unreachable`) that can never legitimately occur for in-process calls.
|
||||||
|
3. Latent deadlock risk: handler acquires the Mutex to read the socket path, releases it, then the
|
||||||
|
request re-enters via the socket handler and tries to acquire the Mutex again. Works today only
|
||||||
|
because `handle_connection` holds no lock during execution, but this is not enforced.
|
||||||
|
|
||||||
|
`daemon::execute_request` is already `pub(crate)` and accepts `(DaemonRequest, SharedDaemon)`.
|
||||||
|
|
||||||
|
**Step 1:** Replace the two implementations of `execute_json` and `execute_json_value` with versions
|
||||||
|
that call `daemon::execute_request` directly:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
async fn execute_json<T>(daemon: SharedDaemon, request: DaemonRequest) -> Response
|
||||||
|
where
|
||||||
|
T: Serialize + DeserializeOwned,
|
||||||
|
{
|
||||||
|
let (response, _) = crate::daemon::execute_request(request, daemon).await;
|
||||||
|
from_daemon_response::<T>(response)
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn execute_json_value(daemon: SharedDaemon, request: DaemonRequest) -> Response {
|
||||||
|
let (response, _) = crate::daemon::execute_request(request, daemon).await;
|
||||||
|
from_daemon_response::<Value>(response)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Step 2:** Delete the entire `send_daemon_request` async function (~65 lines).
|
||||||
|
|
||||||
|
**Step 3:** Remove the now-unused imports from `src/api/mod.rs`:
|
||||||
|
- `tokio::io::{AsyncReadExt, AsyncWriteExt}`
|
||||||
|
- `tokio::net::UnixStream`
|
||||||
|
|
||||||
|
Confirm `cargo build` passes after this change.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 5 — BUG: `PowerState` rendered with `{:?}` in human-readable output
|
||||||
|
|
||||||
|
**File:** `src/cli/mod.rs`, function `render_state`
|
||||||
|
|
||||||
|
```rust
|
||||||
|
format!("Power: {:?}", state.power),
|
||||||
|
```
|
||||||
|
|
||||||
|
`{:?}` prints `On`, `Off`, or `Unknown` (Rust variant names). This is fine but inconsistent with
|
||||||
|
the rest of the output, which is lowercase prose. Worse, it will break if `PowerState` is ever
|
||||||
|
renamed or repr-changed.
|
||||||
|
|
||||||
|
`PowerState` is already `#[serde(rename_all = "snake_case")]`, so it serializes to `"on"`, `"off"`,
|
||||||
|
`"unknown"`.
|
||||||
|
|
||||||
|
**Fix:** Implement `std::fmt::Display` for `PowerState` in `src/adapters/mod.rs`, mirroring the
|
||||||
|
serde rename:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
impl std::fmt::Display for PowerState {
|
||||||
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
match self {
|
||||||
|
PowerState::On => f.write_str("on"),
|
||||||
|
PowerState::Off => f.write_str("off"),
|
||||||
|
PowerState::Unknown => f.write_str("unknown"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Then in `render_state`, change:
|
||||||
|
```rust
|
||||||
|
format!("Power: {:?}", state.power),
|
||||||
|
```
|
||||||
|
to:
|
||||||
|
```rust
|
||||||
|
format!("Power: {}", state.power),
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 6 — BUG: Secret redaction logic is duplicated across three locations
|
||||||
|
|
||||||
|
**Files:** `src/cli/mod.rs`, `src/api/mod.rs`
|
||||||
|
|
||||||
|
The password redaction pattern appears in three places:
|
||||||
|
|
||||||
|
1. `src/cli/mod.rs` — `redact_config_value` (checks `is_secret_config_key`) — used in `config list`
|
||||||
|
and `config get`
|
||||||
|
2. `src/cli/mod.rs` — `sanitize_config_reload_result` (manually mutates `roku_password`) — used in
|
||||||
|
`config reload`
|
||||||
|
3. `src/api/mod.rs` — `get_config` (manually mutates `roku_password`) — used in `GET /v1/config`
|
||||||
|
|
||||||
|
These three will drift if new secret keys are added to the config.
|
||||||
|
|
||||||
|
**Fix:**
|
||||||
|
|
||||||
|
**Step 1:** Add a `redact_secrets` method to `TvctlConfig` in `src/daemon/config.rs`:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
impl TvctlConfig {
|
||||||
|
/// Return a copy of this config with sensitive fields replaced by "<redacted>".
|
||||||
|
pub fn redacted(&self) -> Self {
|
||||||
|
let mut copy = self.clone();
|
||||||
|
if !copy.dev.roku_password.is_empty() {
|
||||||
|
copy.dev.roku_password = "<redacted>".to_string();
|
||||||
|
}
|
||||||
|
copy
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Step 2:** In `src/api/mod.rs`, replace the manual redaction in `get_config`:
|
||||||
|
|
||||||
|
Current:
|
||||||
|
```rust
|
||||||
|
Ok(mut config) => {
|
||||||
|
if !config.dev.roku_password.is_empty() {
|
||||||
|
config.dev.roku_password = "<redacted>".to_string();
|
||||||
|
}
|
||||||
|
api_success(StatusCode::OK, config)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Replace with:
|
||||||
|
```rust
|
||||||
|
Ok(config) => api_success(StatusCode::OK, config.redacted()),
|
||||||
|
```
|
||||||
|
|
||||||
|
**Step 3:** In `src/cli/mod.rs`, replace `sanitize_config_reload_result`:
|
||||||
|
|
||||||
|
Current:
|
||||||
|
```rust
|
||||||
|
fn sanitize_config_reload_result(mut result: ConfigReloadResult) -> ConfigReloadResult {
|
||||||
|
if !result.config.dev.roku_password.is_empty() {
|
||||||
|
result.config.dev.roku_password = "<redacted>".to_string();
|
||||||
|
}
|
||||||
|
result
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Replace with:
|
||||||
|
```rust
|
||||||
|
fn sanitize_config_reload_result(mut result: ConfigReloadResult) -> ConfigReloadResult {
|
||||||
|
result.config = result.config.redacted();
|
||||||
|
result
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The existing `redact_config_value` / `is_secret_config_key` pair in the CLI can remain as-is for now
|
||||||
|
since it serves a different path (key-by-key display), but the two manual inline redactions are gone.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 7 — MINOR: `get_value` allocates a full BTreeMap to look up one key
|
||||||
|
|
||||||
|
**File:** `src/daemon/config.rs`
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn get_value(&self, key: &str) -> Option<String> {
|
||||||
|
self.entries().remove(key)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This allocates and populates a 15-entry `BTreeMap` to look up a single value. `set_value` already
|
||||||
|
has a `match` on the key — the same pattern should be used here.
|
||||||
|
|
||||||
|
**Fix:** Replace `get_value` with a match block that mirrors the structure of `set_value`:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn get_value(&self, key: &str) -> Option<String> {
|
||||||
|
match key {
|
||||||
|
"daemon.socket" => Some(self.daemon.socket.clone()),
|
||||||
|
"daemon.http_enabled" => Some(self.daemon.http_enabled.to_string()),
|
||||||
|
"daemon.http_port" => Some(self.daemon.http_port.to_string()),
|
||||||
|
"daemon.http_host" => Some(self.daemon.http_host.clone()),
|
||||||
|
"daemon.log_level" => Some(self.daemon.log_level.clone()),
|
||||||
|
"discovery.auto_discover" => Some(self.discovery.auto_discover.to_string()),
|
||||||
|
"discovery.interval_secs" => Some(self.discovery.interval_secs.to_string()),
|
||||||
|
"discovery.timeout_secs" => Some(self.discovery.timeout_secs.to_string()),
|
||||||
|
"devices.default" => Some(self.devices.default.clone()),
|
||||||
|
"remote.roku_key_mode" => Some(self.remote.roku_key_mode.clone()),
|
||||||
|
"remote.roku_press_duration_ms" => Some(self.remote.roku_press_duration_ms.to_string()),
|
||||||
|
"dev.enabled" => Some(self.dev.enabled.to_string()),
|
||||||
|
"dev.roku_username" => Some(self.dev.roku_username.clone()),
|
||||||
|
"dev.roku_password" => Some(self.dev.roku_password.clone()),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This eliminates the allocation and keeps `get_value` and `set_value` as a symmetric pair that the
|
||||||
|
compiler will force you to update together.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 8 — MINOR: `roku_key_paths` unsupported-key arms need a clarifying comment
|
||||||
|
|
||||||
|
**File:** `src/adapters/roku/mod.rs`, function `roku_key_paths`
|
||||||
|
|
||||||
|
The first ~25 arms of `roku_key_paths` `return Ok(vec!["..."])`. The last 5 arms fall through to
|
||||||
|
`let path = match { ... }` followed by `Err(TvError::InvalidKey(...))`. This is intentional but
|
||||||
|
looks like the strings in the last 5 arms are ECP key paths when they are actually just error
|
||||||
|
message content.
|
||||||
|
|
||||||
|
**Fix:** Add a comment before the fall-through block, no logic change:
|
||||||
|
|
||||||
|
Find this section in `roku_key_paths`:
|
||||||
|
```rust
|
||||||
|
TvKey::Stop => "stop",
|
||||||
|
TvKey::Skip => "skip",
|
||||||
|
TvKey::Power => "power",
|
||||||
|
TvKey::PowerOn => "power-on",
|
||||||
|
TvKey::Options => "options",
|
||||||
|
};
|
||||||
|
|
||||||
|
Err(TvError::InvalidKey(path.to_string()))
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace with:
|
||||||
|
```rust
|
||||||
|
// The following keys have no documented Roku ECP equivalent.
|
||||||
|
// These strings populate the InvalidKey error message only — they are NOT ECP paths.
|
||||||
|
TvKey::Stop => "stop",
|
||||||
|
TvKey::Skip => "skip",
|
||||||
|
TvKey::Power => "power",
|
||||||
|
TvKey::PowerOn => "power-on",
|
||||||
|
TvKey::Options => "options",
|
||||||
|
};
|
||||||
|
|
||||||
|
Err(TvError::InvalidKey(path.to_string()))
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 9 — Update PROJECT_MAP.md and clean up the archive
|
||||||
|
|
||||||
|
**Step 1:** In `PROJECT_MAP.md`:
|
||||||
|
|
||||||
|
Under `## Project Status`, change:
|
||||||
|
```
|
||||||
|
**Phase:** Milestone 5 in progress. ...
|
||||||
|
```
|
||||||
|
to:
|
||||||
|
```
|
||||||
|
**Phase:** Milestone 6 in progress. Milestones 1–5 are complete. The HTTP API is live. Remaining
|
||||||
|
Milestone 6 work: tracing log level enforcement, cross-compile test, GitHub Actions CI, and first
|
||||||
|
binary release.
|
||||||
|
**Platform v1:** Roku only (via ECP HTTP API)
|
||||||
|
**Language:** Rust
|
||||||
|
**Crate type:** Binary (single binary distribution target)
|
||||||
|
```
|
||||||
|
|
||||||
|
Under `## What Has NOT Been Started`, remove the line:
|
||||||
|
```
|
||||||
|
- HTTP route handlers and request validation
|
||||||
|
```
|
||||||
|
Replace the entire section with what actually remains:
|
||||||
|
```
|
||||||
|
## What Has NOT Been Started
|
||||||
|
|
||||||
|
- Log level enforcement via `tracing` (config key `daemon.log_level` is parsed but the subscriber
|
||||||
|
filter is not updated at runtime after config reload)
|
||||||
|
- CI/CD configuration (GitHub Actions)
|
||||||
|
- Cross-compile targets (x86_64 + aarch64)
|
||||||
|
- First binary release
|
||||||
|
```
|
||||||
|
|
||||||
|
**Step 2:** Confirm that `node-compile-cache/` and `phantomjs/` are listed in `.gitignore`.
|
||||||
|
If `.gitignore` does not exist, create it at the project root with at minimum:
|
||||||
|
```
|
||||||
|
/target
|
||||||
|
node-compile-cache/
|
||||||
|
phantomjs/
|
||||||
|
*.lock
|
||||||
|
```
|
||||||
|
Do not add `Cargo.lock` to gitignore (binary crates should commit their lockfile).
|
||||||
|
The correct ignore for Cargo lockfile in a binary crate is to NOT ignore it. Keep `Cargo.lock` tracked.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## TASK 10 — NOTE: `AppInfo.id` and `AppInfo.platform_id` are always identical for Roku (no fix needed, but document)
|
||||||
|
|
||||||
|
In `src/adapters/roku/mod.rs`, every `AppInfo` constructed in `parse_apps` and `parse_active_app`
|
||||||
|
sets both `id` and `platform_id` to the same value:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
AppInfo {
|
||||||
|
id: platform_id.to_string(),
|
||||||
|
name: name.to_string(),
|
||||||
|
version: ...,
|
||||||
|
platform_id: platform_id.to_string(),
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The design intent (per `src/adapters/mod.rs` comments) is that `id` is a "tvctl-normalized app
|
||||||
|
identifier" and `platform_id` is the "raw platform-specific app identifier." For Roku these are
|
||||||
|
the same. For a future platform (e.g., Google TV via ADB) they could differ.
|
||||||
|
|
||||||
|
**No code change needed.** Add a comment in the Roku `parse_apps` function to document the intent:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
Some(AppInfo {
|
||||||
|
// For Roku, the tvctl-normalized id and the raw ECP channel id are the same value.
|
||||||
|
// Future platforms may produce different normalized and platform-specific identifiers.
|
||||||
|
id: platform_id.to_string(),
|
||||||
|
name: name.to_string(),
|
||||||
|
version: node.attribute("version").map(ToString::to_string),
|
||||||
|
platform_id: platform_id.to_string(),
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
Apply the same comment in `parse_active_app`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## VERIFICATION CHECKLIST
|
||||||
|
|
||||||
|
After completing all tasks:
|
||||||
|
1. `cargo build` — must produce zero errors
|
||||||
|
2. `cargo clippy -- -D warnings` — must produce zero warnings
|
||||||
|
3. `cargo test` — all tests must pass
|
||||||
|
4. Manually verify that Task 4 (API direct call) did not break the HTTP integration path by
|
||||||
|
confirming `execute_request` in `daemon/mod.rs` is `pub(crate)` (it already is)
|
||||||
|
5. Verify PROJECT_MAP.md and ROADMAP.md are consistent with each other after Task 9 edits
|
||||||
|
```
|
||||||
+71
-79
@@ -9,16 +9,12 @@ use axum::{
|
|||||||
};
|
};
|
||||||
use serde::{Deserialize, Serialize, de::DeserializeOwned};
|
use serde::{Deserialize, Serialize, de::DeserializeOwned};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
use tokio::{
|
use tokio::fs;
|
||||||
fs,
|
|
||||||
io::{AsyncReadExt, AsyncWriteExt},
|
|
||||||
net::UnixStream,
|
|
||||||
};
|
|
||||||
|
|
||||||
use crate::{
|
use crate::{
|
||||||
adapters::{Device, TvKey, parse_normalized_tv_key},
|
adapters::{Device, TvKey, parse_normalized_tv_key},
|
||||||
daemon::{
|
daemon::{
|
||||||
SharedDaemon,
|
self, SharedDaemon,
|
||||||
config::TvctlConfig,
|
config::TvctlConfig,
|
||||||
ipc::{
|
ipc::{
|
||||||
AppListResult, ConfigReloadResult, DaemonRequest, DaemonResponse, DaemonStatus,
|
AppListResult, ConfigReloadResult, DaemonRequest, DaemonResponse, DaemonStatus,
|
||||||
@@ -355,17 +351,13 @@ async fn execute_json<T>(daemon: SharedDaemon, request: DaemonRequest) -> Respon
|
|||||||
where
|
where
|
||||||
T: Serialize + DeserializeOwned,
|
T: Serialize + DeserializeOwned,
|
||||||
{
|
{
|
||||||
match send_daemon_request(daemon, &request).await {
|
let response = execute_daemon_request(daemon, request).await;
|
||||||
Ok(response) => from_daemon_response::<T>(response),
|
from_daemon_response::<T>(response)
|
||||||
Err(response) => response,
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn execute_json_value(daemon: SharedDaemon, request: DaemonRequest) -> Response {
|
async fn execute_json_value(daemon: SharedDaemon, request: DaemonRequest) -> Response {
|
||||||
match send_daemon_request(daemon, &request).await {
|
let response = execute_daemon_request(daemon, request).await;
|
||||||
Ok(response) => from_daemon_response::<Value>(response),
|
from_daemon_response::<Value>(response)
|
||||||
Err(response) => response,
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn from_daemon_response<T>(response: DaemonResponse) -> Response
|
fn from_daemon_response<T>(response: DaemonResponse) -> Response
|
||||||
@@ -459,74 +451,74 @@ fn status_for_error(code: &str) -> StatusCode {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn send_daemon_request(
|
async fn execute_daemon_request(daemon: SharedDaemon, request: DaemonRequest) -> DaemonResponse {
|
||||||
daemon: SharedDaemon,
|
let (response, should_stop) = daemon::execute_request(request, daemon).await;
|
||||||
request: &DaemonRequest,
|
if should_stop {
|
||||||
) -> Result<DaemonResponse, Response> {
|
return DaemonResponse::error(
|
||||||
let socket_path = {
|
"invalid_http_request",
|
||||||
let guard = daemon.lock().await;
|
"HTTP API routes cannot trigger daemon shutdown.",
|
||||||
guard.paths.socket_file.clone()
|
Some("Use the Unix socket daemon protocol or CLI for lifecycle control.".to_string()),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
response
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
use tokio::sync::Mutex;
|
||||||
|
|
||||||
|
use super::execute_daemon_request;
|
||||||
|
use crate::daemon::{
|
||||||
|
Daemon,
|
||||||
|
cache::AppCacheStore,
|
||||||
|
config::{RuntimePaths, TvctlConfig},
|
||||||
|
discovery::DiscoveryService,
|
||||||
|
ipc::DaemonRequest,
|
||||||
|
registry::{AdapterRegistry, DeviceRegistry},
|
||||||
|
state::StateCache,
|
||||||
};
|
};
|
||||||
|
|
||||||
let mut stream = UnixStream::connect(&socket_path).await.map_err(|error| {
|
async fn build_test_daemon() -> Arc<Mutex<Daemon>> {
|
||||||
api_error(
|
let temp_dir = tempfile::tempdir().expect("temp dir should exist");
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
let root = temp_dir.path();
|
||||||
"daemon_socket_unreachable",
|
let data_dir = root.join("data");
|
||||||
format!(
|
std::fs::create_dir_all(&data_dir).expect("data dir should exist");
|
||||||
"Unable to reach tvctld at {}: {error}",
|
|
||||||
socket_path.display()
|
|
||||||
),
|
|
||||||
Some(
|
|
||||||
"Check whether the daemon socket is writable and the daemon is running."
|
|
||||||
.to_string(),
|
|
||||||
),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
|
|
||||||
let bytes = serde_json::to_vec(request).map_err(|error| {
|
let paths = RuntimePaths {
|
||||||
api_error(
|
config_file: root.join("config.toml"),
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
data_dir: data_dir.clone(),
|
||||||
"request_encode_failed",
|
devices_file: data_dir.join("devices.json"),
|
||||||
format!("Failed to encode daemon request: {error}"),
|
cache_dir: data_dir.join("cache"),
|
||||||
Some("Ensure the HTTP API and daemon are from the same build.".to_string()),
|
active_socket_file: data_dir.join("active_socket"),
|
||||||
)
|
socket_file: root.join("tvctl.sock"),
|
||||||
})?;
|
};
|
||||||
stream.write_all(&bytes).await.map_err(|error| {
|
let config = TvctlConfig::default();
|
||||||
api_error(
|
let adapters = AdapterRegistry::from_config(&config);
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
let registry = DeviceRegistry::load(paths.devices_file.clone())
|
||||||
"daemon_write_failed",
|
.await
|
||||||
format!("Failed to write request to tvctld: {error}"),
|
.expect("registry should load");
|
||||||
Some("Retry the request after checking the daemon state.".to_string()),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
stream.shutdown().await.map_err(|error| {
|
|
||||||
api_error(
|
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
|
||||||
"daemon_shutdown_failed",
|
|
||||||
format!("Failed to finish the daemon request: {error}"),
|
|
||||||
Some("Retry the request after restarting the daemon.".to_string()),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
|
|
||||||
let mut response_bytes = Vec::new();
|
Arc::new(Mutex::new(Daemon {
|
||||||
stream
|
config,
|
||||||
.read_to_end(&mut response_bytes)
|
paths: paths.clone(),
|
||||||
.await
|
registry,
|
||||||
.map_err(|error| {
|
app_cache: AppCacheStore::new(paths.cache_dir.clone()),
|
||||||
api_error(
|
state_cache: StateCache::default(),
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
adapters: adapters.clone(),
|
||||||
"daemon_read_failed",
|
discovery: DiscoveryService::new(adapters),
|
||||||
format!("Failed to read the daemon response: {error}"),
|
}))
|
||||||
Some("Retry the request after restarting the daemon.".to_string()),
|
}
|
||||||
)
|
|
||||||
})?;
|
|
||||||
|
|
||||||
serde_json::from_slice::<DaemonResponse>(&response_bytes).map_err(|error| {
|
#[tokio::test]
|
||||||
api_error(
|
async fn http_dispatch_rejects_shutdown_requests() {
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
let daemon = build_test_daemon().await;
|
||||||
"daemon_response_invalid",
|
let response = execute_daemon_request(daemon, DaemonRequest::Shutdown).await;
|
||||||
format!("Failed to decode the daemon response: {error}"),
|
|
||||||
Some("Ensure the HTTP API and daemon are from the same build.".to_string()),
|
let error = response
|
||||||
)
|
.error
|
||||||
})
|
.expect("shutdown should be rejected via HTTP");
|
||||||
|
assert_eq!(error.code, "invalid_http_request");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+7
-3
@@ -311,8 +311,8 @@ struct ServiceUninstallResult {
|
|||||||
pub async fn run() -> Result<(), CliError> {
|
pub async fn run() -> Result<(), CliError> {
|
||||||
let cli = Cli::parse();
|
let cli = Cli::parse();
|
||||||
let Some(command) = cli.command.clone() else {
|
let Some(command) = cli.command.clone() else {
|
||||||
let mut cmd = Cli::command();
|
let mut command = build_cli_command();
|
||||||
cmd.print_long_help().map_err(|error| {
|
command.print_long_help().map_err(|error| {
|
||||||
CliError::new(
|
CliError::new(
|
||||||
format!("Failed to render CLI help: {error}"),
|
format!("Failed to render CLI help: {error}"),
|
||||||
"Retry the command or inspect the terminal output.",
|
"Retry the command or inspect the terminal output.",
|
||||||
@@ -338,7 +338,7 @@ pub async fn run() -> Result<(), CliError> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn handle_completion_command(shell: CompletionShell) -> Result<(), CliError> {
|
fn handle_completion_command(shell: CompletionShell) -> Result<(), CliError> {
|
||||||
let mut command = Cli::command();
|
let mut command = build_cli_command();
|
||||||
match shell {
|
match shell {
|
||||||
CompletionShell::Bash => print_completions(Shell::Bash, &mut command),
|
CompletionShell::Bash => print_completions(Shell::Bash, &mut command),
|
||||||
CompletionShell::Zsh => print_completions(Shell::Zsh, &mut command),
|
CompletionShell::Zsh => print_completions(Shell::Zsh, &mut command),
|
||||||
@@ -356,6 +356,10 @@ fn print_completions<G: Generator>(generator: G, command: &mut clap::Command) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn build_cli_command() -> clap::Command {
|
||||||
|
Cli::command()
|
||||||
|
}
|
||||||
|
|
||||||
async fn handle_daemon_command(cli: &Cli, command: DaemonCommand) -> Result<(), CliError> {
|
async fn handle_daemon_command(cli: &Cli, command: DaemonCommand) -> Result<(), CliError> {
|
||||||
match command {
|
match command {
|
||||||
DaemonCommand::Start => daemon_start(cli).await,
|
DaemonCommand::Start => daemon_start(cli).await,
|
||||||
|
|||||||
+26
-1
@@ -7,6 +7,7 @@ use std::{
|
|||||||
use anyhow::bail;
|
use anyhow::bail;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use tokio::fs;
|
use tokio::fs;
|
||||||
|
use tracing_subscriber::EnvFilter;
|
||||||
|
|
||||||
/// The complete daemon configuration loaded from TOML.
|
/// The complete daemon configuration loaded from TOML.
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
|
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
|
||||||
@@ -93,7 +94,10 @@ impl TvctlConfig {
|
|||||||
"daemon.http_enabled" => self.daemon.http_enabled = parse_bool(key, value)?,
|
"daemon.http_enabled" => self.daemon.http_enabled = parse_bool(key, value)?,
|
||||||
"daemon.http_port" => self.daemon.http_port = parse_value(key, value)?,
|
"daemon.http_port" => self.daemon.http_port = parse_value(key, value)?,
|
||||||
"daemon.http_host" => self.daemon.http_host = value.to_string(),
|
"daemon.http_host" => self.daemon.http_host = value.to_string(),
|
||||||
"daemon.log_level" => self.daemon.log_level = value.to_string(),
|
"daemon.log_level" => {
|
||||||
|
validate_log_level(value)?;
|
||||||
|
self.daemon.log_level = value.to_string();
|
||||||
|
}
|
||||||
"discovery.auto_discover" => self.discovery.auto_discover = parse_bool(key, value)?,
|
"discovery.auto_discover" => self.discovery.auto_discover = parse_bool(key, value)?,
|
||||||
"discovery.interval_secs" => self.discovery.interval_secs = parse_value(key, value)?,
|
"discovery.interval_secs" => self.discovery.interval_secs = parse_value(key, value)?,
|
||||||
"discovery.timeout_secs" => self.discovery.timeout_secs = parse_value(key, value)?,
|
"discovery.timeout_secs" => self.discovery.timeout_secs = parse_value(key, value)?,
|
||||||
@@ -311,6 +315,13 @@ where
|
|||||||
.map_err(|error| anyhow::anyhow!("invalid value '{value}' for {key}: {error}"))
|
.map_err(|error| anyhow::anyhow!("invalid value '{value}' for {key}: {error}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn validate_log_level(value: &str) -> anyhow::Result<()> {
|
||||||
|
EnvFilter::try_new(value).map_err(|error| {
|
||||||
|
anyhow::anyhow!("invalid value '{value}' for daemon.log_level: {error}")
|
||||||
|
})?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -348,4 +359,18 @@ mod tests {
|
|||||||
.expect("config should load");
|
.expect("config should load");
|
||||||
assert_eq!(loaded, config);
|
assert_eq!(loaded, config);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_invalid_log_level_values() {
|
||||||
|
let mut config = TvctlConfig::default();
|
||||||
|
let error = config
|
||||||
|
.set_value("daemon.log_level", "[")
|
||||||
|
.expect_err("invalid log level should fail");
|
||||||
|
assert!(
|
||||||
|
error
|
||||||
|
.to_string()
|
||||||
|
.contains("invalid value '[' for daemon.log_level"),
|
||||||
|
"unexpected error: {error}"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+20
-7
@@ -36,6 +36,7 @@ use std::os::unix::fs::PermissionsExt;
|
|||||||
|
|
||||||
use crate::adapters::{Device, TvKey};
|
use crate::adapters::{Device, TvKey};
|
||||||
use crate::api;
|
use crate::api;
|
||||||
|
use crate::logging;
|
||||||
|
|
||||||
pub type SharedDaemon = Arc<Mutex<Daemon>>;
|
pub type SharedDaemon = Arc<Mutex<Daemon>>;
|
||||||
|
|
||||||
@@ -62,6 +63,7 @@ impl Daemon {
|
|||||||
/// Load the daemon's persisted state and adapter registry.
|
/// Load the daemon's persisted state and adapter registry.
|
||||||
pub async fn load() -> anyhow::Result<Self> {
|
pub async fn load() -> anyhow::Result<Self> {
|
||||||
let config = TvctlConfig::load().await?;
|
let config = TvctlConfig::load().await?;
|
||||||
|
logging::apply_log_level(&config.daemon.log_level).map_err(anyhow::Error::msg)?;
|
||||||
let mut paths = RuntimePaths::detect();
|
let mut paths = RuntimePaths::detect();
|
||||||
if !config.daemon.socket.is_empty() {
|
if !config.daemon.socket.is_empty() {
|
||||||
paths.socket_file = PathBuf::from(&config.daemon.socket);
|
paths.socket_file = PathBuf::from(&config.daemon.socket);
|
||||||
@@ -783,16 +785,26 @@ pub(crate) async fn execute_request(
|
|||||||
DaemonRequest::ReloadConfig => {
|
DaemonRequest::ReloadConfig => {
|
||||||
let mut guard = daemon.lock().await;
|
let mut guard = daemon.lock().await;
|
||||||
match TvctlConfig::load_from_path(&guard.paths.config_file).await {
|
match TvctlConfig::load_from_path(&guard.paths.config_file).await {
|
||||||
Ok(config) => {
|
Ok(config) => match apply_runtime_config(&mut guard, config) {
|
||||||
let restart_required = apply_runtime_config(&mut guard, config);
|
Ok(restart_required) => (
|
||||||
(
|
|
||||||
DaemonResponse::success(ConfigReloadResult {
|
DaemonResponse::success(ConfigReloadResult {
|
||||||
config: guard.config.clone(),
|
config: guard.config.clone(),
|
||||||
restart_required,
|
restart_required,
|
||||||
}),
|
}),
|
||||||
false,
|
false,
|
||||||
)
|
),
|
||||||
}
|
Err(error) => (
|
||||||
|
DaemonResponse::error(
|
||||||
|
"config_reload_failed",
|
||||||
|
format!("Failed to apply reloaded config: {error}"),
|
||||||
|
Some(
|
||||||
|
"Inspect ~/.config/tvctl/config.toml and fix invalid values."
|
||||||
|
.to_string(),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
false,
|
||||||
|
),
|
||||||
|
},
|
||||||
Err(error) => (
|
Err(error) => (
|
||||||
DaemonResponse::error(
|
DaemonResponse::error(
|
||||||
"config_reload_failed",
|
"config_reload_failed",
|
||||||
@@ -943,8 +955,9 @@ fn discovery_interval(interval_secs: u64) -> Option<time::Interval> {
|
|||||||
Some(interval)
|
Some(interval)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn apply_runtime_config(daemon: &mut Daemon, config: TvctlConfig) -> Vec<String> {
|
fn apply_runtime_config(daemon: &mut Daemon, config: TvctlConfig) -> anyhow::Result<Vec<String>> {
|
||||||
let old_config = daemon.config.clone();
|
let old_config = daemon.config.clone();
|
||||||
|
logging::apply_log_level(&config.daemon.log_level).map_err(anyhow::Error::msg)?;
|
||||||
daemon.adapters = AdapterRegistry::from_config(&config);
|
daemon.adapters = AdapterRegistry::from_config(&config);
|
||||||
daemon.discovery = DiscoveryService::new(daemon.adapters.clone());
|
daemon.discovery = DiscoveryService::new(daemon.adapters.clone());
|
||||||
|
|
||||||
@@ -972,5 +985,5 @@ fn apply_runtime_config(daemon: &mut Daemon, config: TvctlConfig) -> Vec<String>
|
|||||||
}
|
}
|
||||||
|
|
||||||
daemon.config = config;
|
daemon.config = config;
|
||||||
restart_required
|
Ok(restart_required)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -294,7 +294,7 @@ impl AdapterRegistry {
|
|||||||
F: for<'a> FnOnce(
|
F: for<'a> FnOnce(
|
||||||
&'a RokuAdapter,
|
&'a RokuAdapter,
|
||||||
) -> std::pin::Pin<
|
) -> std::pin::Pin<
|
||||||
Box<dyn std::future::Future<Output = crate::adapters::Result<T>> + 'a>,
|
Box<dyn std::future::Future<Output = crate::adapters::Result<T>> + Send + 'a>,
|
||||||
>,
|
>,
|
||||||
{
|
{
|
||||||
match platform {
|
match platform {
|
||||||
@@ -309,7 +309,7 @@ impl AdapterRegistry {
|
|||||||
&'a RokuAdapter,
|
&'a RokuAdapter,
|
||||||
&'a Device,
|
&'a Device,
|
||||||
) -> std::pin::Pin<
|
) -> std::pin::Pin<
|
||||||
Box<dyn std::future::Future<Output = crate::adapters::Result<T>> + 'a>,
|
Box<dyn std::future::Future<Output = crate::adapters::Result<T>> + Send + 'a>,
|
||||||
>,
|
>,
|
||||||
{
|
{
|
||||||
match device.platform.as_str() {
|
match device.platform.as_str() {
|
||||||
|
|||||||
@@ -4,3 +4,4 @@ pub mod adapters;
|
|||||||
pub mod api;
|
pub mod api;
|
||||||
pub mod cli;
|
pub mod cli;
|
||||||
pub mod daemon;
|
pub mod daemon;
|
||||||
|
pub mod logging;
|
||||||
|
|||||||
@@ -0,0 +1,45 @@
|
|||||||
|
use std::sync::OnceLock;
|
||||||
|
|
||||||
|
use tracing_subscriber::{
|
||||||
|
EnvFilter, Registry, fmt, layer::SubscriberExt, reload, util::SubscriberInitExt,
|
||||||
|
};
|
||||||
|
|
||||||
|
type FilterReloadHandle = reload::Handle<EnvFilter, Registry>;
|
||||||
|
|
||||||
|
static FILTER_RELOAD_HANDLE: OnceLock<FilterReloadHandle> = OnceLock::new();
|
||||||
|
|
||||||
|
/// Initialize global tracing with a runtime-reloadable filter.
|
||||||
|
pub fn init(default_level: &str) -> anyhow::Result<()> {
|
||||||
|
let filter = if std::env::var("RUST_LOG").is_ok() {
|
||||||
|
EnvFilter::try_from_default_env().map_err(|error| {
|
||||||
|
anyhow::anyhow!("failed to parse RUST_LOG env filter '{default_level}': {error}")
|
||||||
|
})?
|
||||||
|
} else {
|
||||||
|
parse_filter(default_level).map_err(anyhow::Error::msg)?
|
||||||
|
};
|
||||||
|
|
||||||
|
let (filter_layer, handle) = reload::Layer::new(filter);
|
||||||
|
let _ = FILTER_RELOAD_HANDLE.set(handle);
|
||||||
|
|
||||||
|
tracing_subscriber::registry()
|
||||||
|
.with(filter_layer)
|
||||||
|
.with(fmt::layer().with_target(false).compact())
|
||||||
|
.try_init()
|
||||||
|
.map_err(|error| anyhow::anyhow!("failed to initialize tracing: {error}"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Apply a new process log-level filter at runtime.
|
||||||
|
pub fn apply_log_level(level: &str) -> Result<(), String> {
|
||||||
|
let filter = parse_filter(level)?;
|
||||||
|
let handle = FILTER_RELOAD_HANDLE
|
||||||
|
.get()
|
||||||
|
.ok_or_else(|| "logging subsystem is not initialized".to_string())?;
|
||||||
|
handle
|
||||||
|
.modify(|current_filter| *current_filter = filter)
|
||||||
|
.map_err(|error| format!("failed to apply log filter: {error}"))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_filter(value: &str) -> Result<EnvFilter, String> {
|
||||||
|
EnvFilter::try_new(value)
|
||||||
|
.map_err(|error| format!("invalid value '{value}' for daemon.log_level: {error}"))
|
||||||
|
}
|
||||||
+4
-8
@@ -1,14 +1,10 @@
|
|||||||
/// Launch the tvctl binary entry point.
|
/// Launch the tvctl binary entry point.
|
||||||
#[tokio::main]
|
#[tokio::main]
|
||||||
async fn main() {
|
async fn main() {
|
||||||
tracing_subscriber::fmt()
|
if let Err(error) = tvctl::logging::init("info") {
|
||||||
.with_env_filter(
|
eprintln!("Failed to initialize logging: {error}");
|
||||||
tracing_subscriber::EnvFilter::from_default_env()
|
std::process::exit(1);
|
||||||
.add_directive(tracing::Level::INFO.into()),
|
}
|
||||||
)
|
|
||||||
.with_target(false)
|
|
||||||
.compact()
|
|
||||||
.init();
|
|
||||||
|
|
||||||
if let Err(error) = tvctl::cli::run().await {
|
if let Err(error) = tvctl::cli::run().await {
|
||||||
eprintln!("{error}");
|
eprintln!("{error}");
|
||||||
|
|||||||
+176
-2
@@ -1,6 +1,8 @@
|
|||||||
use std::{
|
use std::{
|
||||||
net::TcpListener,
|
net::TcpListener,
|
||||||
|
path::PathBuf,
|
||||||
process::{Child, Command, Stdio},
|
process::{Child, Command, Stdio},
|
||||||
|
sync::Arc,
|
||||||
time::Duration,
|
time::Duration,
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -8,8 +10,18 @@ use chrono::Utc;
|
|||||||
use reqwest::Client;
|
use reqwest::Client;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
use tempfile::TempDir;
|
use tempfile::TempDir;
|
||||||
use tokio::time::sleep;
|
use tokio::{net::TcpListener as TokioTcpListener, sync::Mutex, task::JoinHandle, time::sleep};
|
||||||
use tvctl::{adapters::Device, daemon::config::TvctlConfig};
|
use tvctl::{
|
||||||
|
adapters::Device,
|
||||||
|
daemon::{
|
||||||
|
Daemon, SharedDaemon,
|
||||||
|
cache::AppCacheStore,
|
||||||
|
config::{DaemonConfig, DevConfig, DiscoveryConfig, RuntimePaths, TvctlConfig},
|
||||||
|
discovery::DiscoveryService,
|
||||||
|
registry::{AdapterRegistry, DeviceRegistry},
|
||||||
|
state::StateCache,
|
||||||
|
},
|
||||||
|
};
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
struct TestDaemon {
|
struct TestDaemon {
|
||||||
@@ -19,6 +31,14 @@ struct TestDaemon {
|
|||||||
device: Device,
|
device: Device,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct InProcessApi {
|
||||||
|
_temp_dir: TempDir,
|
||||||
|
server: JoinHandle<()>,
|
||||||
|
base_url: String,
|
||||||
|
device: Device,
|
||||||
|
socket_path: PathBuf,
|
||||||
|
}
|
||||||
|
|
||||||
impl TestDaemon {
|
impl TestDaemon {
|
||||||
async fn start() -> Self {
|
async fn start() -> Self {
|
||||||
let temp_dir = tempfile::tempdir().expect("temp dir should exist");
|
let temp_dir = tempfile::tempdir().expect("temp dir should exist");
|
||||||
@@ -131,6 +151,139 @@ impl Drop for TestDaemon {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl InProcessApi {
|
||||||
|
async fn start() -> Self {
|
||||||
|
let temp_dir = tempfile::tempdir().expect("temp dir should exist");
|
||||||
|
let root = temp_dir.path();
|
||||||
|
let config_home = root.join("config");
|
||||||
|
let data_home = root.join("data");
|
||||||
|
let runtime_dir = root.join("runtime");
|
||||||
|
std::fs::create_dir_all(&config_home).expect("config dir should exist");
|
||||||
|
std::fs::create_dir_all(&data_home).expect("data dir should exist");
|
||||||
|
std::fs::create_dir_all(&runtime_dir).expect("runtime dir should exist");
|
||||||
|
|
||||||
|
let listener = TokioTcpListener::bind("127.0.0.1:0")
|
||||||
|
.await
|
||||||
|
.expect("ephemeral http port should bind");
|
||||||
|
let port = listener
|
||||||
|
.local_addr()
|
||||||
|
.expect("listener addr should exist")
|
||||||
|
.port();
|
||||||
|
let socket_path = runtime_dir.join("tvctl.sock");
|
||||||
|
let config = TvctlConfig {
|
||||||
|
daemon: DaemonConfig {
|
||||||
|
socket: socket_path.display().to_string(),
|
||||||
|
http_enabled: true,
|
||||||
|
http_port: port,
|
||||||
|
http_host: "127.0.0.1".to_string(),
|
||||||
|
log_level: "info".to_string(),
|
||||||
|
},
|
||||||
|
discovery: DiscoveryConfig {
|
||||||
|
auto_discover: false,
|
||||||
|
interval_secs: 300,
|
||||||
|
timeout_secs: 1,
|
||||||
|
},
|
||||||
|
dev: DevConfig {
|
||||||
|
enabled: true,
|
||||||
|
roku_username: "rokudev".to_string(),
|
||||||
|
roku_password: "secret".to_string(),
|
||||||
|
},
|
||||||
|
..TvctlConfig::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let config_path = config_home.join("tvctl/config.toml");
|
||||||
|
let data_dir = data_home.join("tvctl");
|
||||||
|
config
|
||||||
|
.save_to_path(&config_path)
|
||||||
|
.await
|
||||||
|
.expect("config should save");
|
||||||
|
|
||||||
|
let device = Device {
|
||||||
|
id: Uuid::new_v4(),
|
||||||
|
name: "In Process Roku".to_string(),
|
||||||
|
original_name: "In Process Roku".to_string(),
|
||||||
|
platform: "roku".to_string(),
|
||||||
|
address: "127.0.0.3".parse().expect("loopback should parse"),
|
||||||
|
port: 8060,
|
||||||
|
is_default: true,
|
||||||
|
discovered_at: Utc::now(),
|
||||||
|
last_seen: Utc::now(),
|
||||||
|
};
|
||||||
|
std::fs::create_dir_all(&data_dir).expect("data dir should exist");
|
||||||
|
std::fs::write(
|
||||||
|
data_dir.join("devices.json"),
|
||||||
|
serde_json::to_vec_pretty(&vec![device.clone()]).expect("device should encode"),
|
||||||
|
)
|
||||||
|
.expect("devices file should write");
|
||||||
|
|
||||||
|
let paths = RuntimePaths {
|
||||||
|
config_file: config_path,
|
||||||
|
data_dir: data_dir.clone(),
|
||||||
|
devices_file: data_dir.join("devices.json"),
|
||||||
|
cache_dir: data_dir.join("cache"),
|
||||||
|
active_socket_file: data_dir.join("active_socket"),
|
||||||
|
socket_file: socket_path.clone(),
|
||||||
|
};
|
||||||
|
let adapters = AdapterRegistry::from_config(&config);
|
||||||
|
let mut registry = DeviceRegistry::load(paths.devices_file.clone())
|
||||||
|
.await
|
||||||
|
.expect("registry should load");
|
||||||
|
if !config.devices.default.is_empty() {
|
||||||
|
let _ = registry.set_default(&config.devices.default);
|
||||||
|
} else {
|
||||||
|
registry.ensure_default();
|
||||||
|
}
|
||||||
|
let daemon: SharedDaemon = Arc::new(Mutex::new(Daemon {
|
||||||
|
config,
|
||||||
|
paths: paths.clone(),
|
||||||
|
registry,
|
||||||
|
app_cache: AppCacheStore::new(paths.cache_dir.clone()),
|
||||||
|
state_cache: StateCache::default(),
|
||||||
|
adapters: adapters.clone(),
|
||||||
|
discovery: DiscoveryService::new(adapters),
|
||||||
|
}));
|
||||||
|
|
||||||
|
let app = tvctl::api::router(daemon);
|
||||||
|
let server = tokio::spawn(async move {
|
||||||
|
axum::serve(listener, app)
|
||||||
|
.await
|
||||||
|
.expect("in-process api should serve");
|
||||||
|
});
|
||||||
|
|
||||||
|
let instance = Self {
|
||||||
|
_temp_dir: temp_dir,
|
||||||
|
server,
|
||||||
|
base_url: format!("http://127.0.0.1:{port}/v1"),
|
||||||
|
device,
|
||||||
|
socket_path,
|
||||||
|
};
|
||||||
|
instance.wait_until_ready().await;
|
||||||
|
instance
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn wait_until_ready(&self) {
|
||||||
|
let client = Client::new();
|
||||||
|
for _ in 0..40 {
|
||||||
|
if let Ok(response) = client
|
||||||
|
.get(format!("{}/daemon/status", self.base_url))
|
||||||
|
.send()
|
||||||
|
.await
|
||||||
|
&& response.status().is_success()
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
sleep(Duration::from_millis(100)).await;
|
||||||
|
}
|
||||||
|
panic!("in-process HTTP API did not become ready");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Drop for InProcessApi {
|
||||||
|
fn drop(&mut self) {
|
||||||
|
self.server.abort();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn pick_unused_port() -> u16 {
|
fn pick_unused_port() -> u16 {
|
||||||
let listener = TcpListener::bind("127.0.0.1:0").expect("ephemeral port should bind");
|
let listener = TcpListener::bind("127.0.0.1:0").expect("ephemeral port should bind");
|
||||||
let port = listener
|
let port = listener
|
||||||
@@ -223,3 +376,24 @@ async fn http_api_exposes_core_routes_and_config_patch() {
|
|||||||
let reload_json: Value = reload.json().await.expect("reload json should parse");
|
let reload_json: Value = reload.json().await.expect("reload json should parse");
|
||||||
assert_eq!(reload_json["ok"], true);
|
assert_eq!(reload_json["ok"], true);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn http_api_routes_requests_without_unix_socket_loopback() {
|
||||||
|
let api = InProcessApi::start().await;
|
||||||
|
assert!(
|
||||||
|
!api.socket_path.exists(),
|
||||||
|
"test should not start a daemon unix socket listener"
|
||||||
|
);
|
||||||
|
|
||||||
|
let client = Client::new();
|
||||||
|
let devices = client
|
||||||
|
.get(format!("{}/devices", api.base_url))
|
||||||
|
.send()
|
||||||
|
.await
|
||||||
|
.expect("devices should respond");
|
||||||
|
assert!(devices.status().is_success());
|
||||||
|
|
||||||
|
let devices_json: Value = devices.json().await.expect("devices json should parse");
|
||||||
|
assert_eq!(devices_json["ok"], true);
|
||||||
|
assert_eq!(devices_json["data"][0]["id"], api.device.id.to_string());
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user